| Message ID | 1309889871-6267-2-git-send-email-lcapitulino@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 07/05/2011 01:17 PM, Luiz Capitulino wrote: > We need to track the VM status so that QMP can report it to clients. > > This commit adds the VMStatus type and related functions. The > vm_status_set() function is used to keep track of the current > VM status. > > The current statuses are: Which states are terminal, and what's the proper procedure to recover from non-terminal states? Is it cont or system_reset followed by cont? > > - debug: guest is running under gdb > - inmigrate: guest is paused waiting for an incoming migration > - postmigrate: guest is paused following a successful migration > - internal-error: Fatal internal error that prevents further guest > execution > - load-state-error: guest is paused following a failed 'loadvm' > - io-error: the last IOP has failed and the device is configured > to pause on I/O errors > - watchdog-error: the watchdog action is configured to pause and > has been triggered > - paused: guest has been paused via the 'stop' command > - prelaunch: QEMU was started with -S and guest has not started > - running: guest is actively running > - shutdown: guest is shut down (and -no-shutdown is in use) Regards, Anthony Liguori > > Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com> > --- > gdbstub.c | 4 ++++ > hw/ide/core.c | 1 + > hw/scsi-disk.c | 1 + > hw/virtio-blk.c | 1 + > hw/watchdog.c | 1 + > kvm-all.c | 1 + > migration.c | 3 +++ > monitor.c | 5 ++++- > sysemu.h | 19 +++++++++++++++++++ > vl.c | 37 +++++++++++++++++++++++++++++++++++++ > 10 files changed, 72 insertions(+), 1 deletions(-) > > diff --git a/gdbstub.c b/gdbstub.c > index c085a5a..61b700a 100644 > --- a/gdbstub.c > +++ b/gdbstub.c > @@ -2358,6 +2358,7 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...) > s->state = RS_SYSCALL; > #ifndef CONFIG_USER_ONLY > vm_stop(VMSTOP_DEBUG); > + vm_status_set(VMST_DEBUG); > #endif > s->state = RS_IDLE; > va_start(va, fmt); > @@ -2432,6 +2433,7 @@ static void gdb_read_byte(GDBState *s, int ch) > /* when the CPU is running, we cannot do anything except stop > it when receiving a char */ > vm_stop(VMSTOP_USER); > + vm_status_set(VMST_DEBUG); > } else > #endif > { > @@ -2694,6 +2696,7 @@ static void gdb_chr_event(void *opaque, int event) > switch (event) { > case CHR_EVENT_OPENED: > vm_stop(VMSTOP_USER); > + vm_status_set(VMST_DEBUG); > gdb_has_xml = 0; > break; > default: > @@ -2735,6 +2738,7 @@ static void gdb_sigterm_handler(int signal) > { > if (vm_running) { > vm_stop(VMSTOP_USER); > + vm_status_set(VMST_DEBUG); > } > } > #endif > diff --git a/hw/ide/core.c b/hw/ide/core.c > index ca17a43..bf9df41 100644 > --- a/hw/ide/core.c > +++ b/hw/ide/core.c > @@ -523,6 +523,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int op) > s->bus->error_status = op; > bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); > vm_stop(VMSTOP_DISKFULL); > + vm_status_set(VMST_IOERROR); > } else { > if (op& BM_STATUS_DMA_RETRY) { > dma_buf_commit(s, 0); > diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c > index a8c7372..66037fd 100644 > --- a/hw/scsi-disk.c > +++ b/hw/scsi-disk.c > @@ -216,6 +216,7 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type) > > bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); > vm_stop(VMSTOP_DISKFULL); > + vm_status_set(VMST_IOERROR); > } else { > if (type == SCSI_REQ_STATUS_RETRY_READ) { > scsi_req_data(&r->req, 0); > diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c > index 91e0394..bf70200 100644 > --- a/hw/virtio-blk.c > +++ b/hw/virtio-blk.c > @@ -79,6 +79,7 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, > s->rq = req; > bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); > vm_stop(VMSTOP_DISKFULL); > + vm_status_set(VMST_IOERROR); > } else { > virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR); > bdrv_mon_event(s->bs, BDRV_ACTION_REPORT, is_read); > diff --git a/hw/watchdog.c b/hw/watchdog.c > index 1c900a1..d130cbb 100644 > --- a/hw/watchdog.c > +++ b/hw/watchdog.c > @@ -133,6 +133,7 @@ void watchdog_perform_action(void) > case WDT_PAUSE: /* same as 'stop' command in monitor */ > watchdog_mon_event("pause"); > vm_stop(VMSTOP_WATCHDOG); > + vm_status_set(VMST_WATCHDOG); > break; > > case WDT_DEBUG: > diff --git a/kvm-all.c b/kvm-all.c > index cbc2532..aee9e0a 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -1015,6 +1015,7 @@ int kvm_cpu_exec(CPUState *env) > if (ret< 0) { > cpu_dump_state(env, stderr, fprintf, CPU_DUMP_CODE); > vm_stop(VMSTOP_PANIC); > + vm_status_set(VMST_INTERROR); > } > > env->exit_request = 0; > diff --git a/migration.c b/migration.c > index af3a1f2..674792f 100644 > --- a/migration.c > +++ b/migration.c > @@ -394,6 +394,9 @@ void migrate_fd_put_ready(void *opaque) > } > state = MIG_STATE_ERROR; > } > + if (state == MIG_STATE_COMPLETED) { > + vm_status_set(VMST_POSTMIGRATE); > + } > s->state = state; > notifier_list_notify(&migration_state_notifiers); > } > diff --git a/monitor.c b/monitor.c > index 67ceb46..1cb3191 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -1258,6 +1258,7 @@ static void do_singlestep(Monitor *mon, const QDict *qdict) > static int do_stop(Monitor *mon, const QDict *qdict, QObject **ret_data) > { > vm_stop(VMSTOP_USER); > + vm_status_set(VMST_PAUSED); > return 0; > } > > @@ -2782,7 +2783,9 @@ static void do_loadvm(Monitor *mon, const QDict *qdict) > > vm_stop(VMSTOP_LOADVM); > > - if (load_vmstate(name) == 0&& saved_vm_running) { > + if (load_vmstate(name)< 0) { > + vm_status_set(VMST_LOADERROR); > + } else if (saved_vm_running) { > vm_start(); > } > } > diff --git a/sysemu.h b/sysemu.h > index d3013f5..7308ac5 100644 > --- a/sysemu.h > +++ b/sysemu.h > @@ -77,6 +77,25 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f); > void qemu_savevm_state_cancel(Monitor *mon, QEMUFile *f); > int qemu_loadvm_state(QEMUFile *f); > > +typedef enum { > + VMST_NOSTATUS, > + VMST_DEBUG, > + VMST_INMIGRATE, > + VMST_POSTMIGRATE, > + VMST_INTERROR, > + VMST_IOERROR, > + VMST_LOADERROR, > + VMST_PAUSED, > + VMST_PRELAUNCH, > + VMST_RUNNING, > + VMST_SHUTDOWN, > + VMST_WATCHDOG, > + VMST_MAX, > +} VMStatus; > + > +void vm_status_set(VMStatus s); > +const char *vm_status_get_name(void); > + > /* SLIRP */ > void do_info_slirp(Monitor *mon); > > diff --git a/vl.c b/vl.c > index fcd7395..fb7208f 100644 > --- a/vl.c > +++ b/vl.c > @@ -309,6 +309,37 @@ static int default_driver_check(QemuOpts *opts, void *opaque) > } > > /***********************************************************/ > +/* VMStatus */ > + > +static const char *const vm_status_name[VMST_MAX] = { > + [VMST_DEBUG] = "debug", > + [VMST_INMIGRATE] = "inmigrate", > + [VMST_POSTMIGRATE] = "postmigrate", > + [VMST_INTERROR] = "internal-error", > + [VMST_IOERROR] = "io-error", > + [VMST_LOADERROR] = "load-state-error", > + [VMST_PAUSED] = "paused", > + [VMST_PRELAUNCH] = "prelaunch", > + [VMST_RUNNING] = "running", > + [VMST_SHUTDOWN] = "shutdown", > + [VMST_WATCHDOG] = "watchdog-error", > +}; > + > +static VMStatus qemu_vm_status = VMST_NOSTATUS; > + > +void vm_status_set(VMStatus s) > +{ > + assert(s> VMST_NOSTATUS&& s< VMST_MAX); > + qemu_vm_status = s; > +} > + > +const char *vm_status_get_name(void) > +{ > + assert(qemu_vm_status> VMST_NOSTATUS&& qemu_vm_status< VMST_MAX); > + return vm_status_name[qemu_vm_status]; > +} > + > +/***********************************************************/ > /* real time host monotonic timer */ > > /***********************************************************/ > @@ -1150,6 +1181,7 @@ void vm_start(void) > vm_state_notify(1, 0); > resume_all_vcpus(); > monitor_protocol_event(QEVENT_RESUME, NULL); > + vm_status_set(VMST_RUNNING); > } > } > > @@ -1392,12 +1424,14 @@ static void main_loop(void) > > if (qemu_debug_requested()) { > vm_stop(VMSTOP_DEBUG); > + vm_status_set(VMST_DEBUG); > } > if (qemu_shutdown_requested()) { > qemu_kill_report(); > monitor_protocol_event(QEVENT_SHUTDOWN, NULL); > if (no_shutdown) { > vm_stop(VMSTOP_SHUTDOWN); > + vm_status_set(VMST_SHUTDOWN); > no_shutdown = 0; > } else > break; > @@ -2476,6 +2510,7 @@ int main(int argc, char **argv, char **envp) > break; > case QEMU_OPTION_S: > autostart = 0; > + vm_status_set(VMST_PRELAUNCH); > break; > case QEMU_OPTION_k: > keyboard_layout = optarg; > @@ -3307,11 +3342,13 @@ int main(int argc, char **argv, char **envp) > qemu_system_reset(VMRESET_SILENT); > if (loadvm) { > if (load_vmstate(loadvm)< 0) { > + vm_status_set(VMST_LOADERROR); > autostart = 0; > } > } > > if (incoming) { > + vm_status_set(VMST_INMIGRATE); > int ret = qemu_start_incoming_migration(incoming); > if (ret< 0) { > fprintf(stderr, "Migration failed. Exit code %s(%d), exiting.\n",
On Tue, 05 Jul 2011 13:33:07 -0500 Anthony Liguori <anthony@codemonkey.ws> wrote: > On 07/05/2011 01:17 PM, Luiz Capitulino wrote: > > We need to track the VM status so that QMP can report it to clients. > > > > This commit adds the VMStatus type and related functions. The > > vm_status_set() function is used to keep track of the current > > VM status. > > > > The current statuses are: > > Which states are terminal, and what's the proper procedure to recover > from non-terminal states? Is it cont or system_reset followed by cont? Can I add that to the next patch (which also adds the QMP doc) or do you prefer it in this patch? > > > > > - debug: guest is running under gdb > > - inmigrate: guest is paused waiting for an incoming migration > > - postmigrate: guest is paused following a successful migration > > - internal-error: Fatal internal error that prevents further guest > > execution > > - load-state-error: guest is paused following a failed 'loadvm' > > - io-error: the last IOP has failed and the device is configured > > to pause on I/O errors > > - watchdog-error: the watchdog action is configured to pause and > > has been triggered > > - paused: guest has been paused via the 'stop' command > > - prelaunch: QEMU was started with -S and guest has not started > > - running: guest is actively running > > - shutdown: guest is shut down (and -no-shutdown is in use) > > Regards, > > Anthony Liguori > > > > > Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com> > > --- > > gdbstub.c | 4 ++++ > > hw/ide/core.c | 1 + > > hw/scsi-disk.c | 1 + > > hw/virtio-blk.c | 1 + > > hw/watchdog.c | 1 + > > kvm-all.c | 1 + > > migration.c | 3 +++ > > monitor.c | 5 ++++- > > sysemu.h | 19 +++++++++++++++++++ > > vl.c | 37 +++++++++++++++++++++++++++++++++++++ > > 10 files changed, 72 insertions(+), 1 deletions(-) > > > > diff --git a/gdbstub.c b/gdbstub.c > > index c085a5a..61b700a 100644 > > --- a/gdbstub.c > > +++ b/gdbstub.c > > @@ -2358,6 +2358,7 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...) > > s->state = RS_SYSCALL; > > #ifndef CONFIG_USER_ONLY > > vm_stop(VMSTOP_DEBUG); > > + vm_status_set(VMST_DEBUG); > > #endif > > s->state = RS_IDLE; > > va_start(va, fmt); > > @@ -2432,6 +2433,7 @@ static void gdb_read_byte(GDBState *s, int ch) > > /* when the CPU is running, we cannot do anything except stop > > it when receiving a char */ > > vm_stop(VMSTOP_USER); > > + vm_status_set(VMST_DEBUG); > > } else > > #endif > > { > > @@ -2694,6 +2696,7 @@ static void gdb_chr_event(void *opaque, int event) > > switch (event) { > > case CHR_EVENT_OPENED: > > vm_stop(VMSTOP_USER); > > + vm_status_set(VMST_DEBUG); > > gdb_has_xml = 0; > > break; > > default: > > @@ -2735,6 +2738,7 @@ static void gdb_sigterm_handler(int signal) > > { > > if (vm_running) { > > vm_stop(VMSTOP_USER); > > + vm_status_set(VMST_DEBUG); > > } > > } > > #endif > > diff --git a/hw/ide/core.c b/hw/ide/core.c > > index ca17a43..bf9df41 100644 > > --- a/hw/ide/core.c > > +++ b/hw/ide/core.c > > @@ -523,6 +523,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int op) > > s->bus->error_status = op; > > bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); > > vm_stop(VMSTOP_DISKFULL); > > + vm_status_set(VMST_IOERROR); > > } else { > > if (op& BM_STATUS_DMA_RETRY) { > > dma_buf_commit(s, 0); > > diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c > > index a8c7372..66037fd 100644 > > --- a/hw/scsi-disk.c > > +++ b/hw/scsi-disk.c > > @@ -216,6 +216,7 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type) > > > > bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); > > vm_stop(VMSTOP_DISKFULL); > > + vm_status_set(VMST_IOERROR); > > } else { > > if (type == SCSI_REQ_STATUS_RETRY_READ) { > > scsi_req_data(&r->req, 0); > > diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c > > index 91e0394..bf70200 100644 > > --- a/hw/virtio-blk.c > > +++ b/hw/virtio-blk.c > > @@ -79,6 +79,7 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, > > s->rq = req; > > bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); > > vm_stop(VMSTOP_DISKFULL); > > + vm_status_set(VMST_IOERROR); > > } else { > > virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR); > > bdrv_mon_event(s->bs, BDRV_ACTION_REPORT, is_read); > > diff --git a/hw/watchdog.c b/hw/watchdog.c > > index 1c900a1..d130cbb 100644 > > --- a/hw/watchdog.c > > +++ b/hw/watchdog.c > > @@ -133,6 +133,7 @@ void watchdog_perform_action(void) > > case WDT_PAUSE: /* same as 'stop' command in monitor */ > > watchdog_mon_event("pause"); > > vm_stop(VMSTOP_WATCHDOG); > > + vm_status_set(VMST_WATCHDOG); > > break; > > > > case WDT_DEBUG: > > diff --git a/kvm-all.c b/kvm-all.c > > index cbc2532..aee9e0a 100644 > > --- a/kvm-all.c > > +++ b/kvm-all.c > > @@ -1015,6 +1015,7 @@ int kvm_cpu_exec(CPUState *env) > > if (ret< 0) { > > cpu_dump_state(env, stderr, fprintf, CPU_DUMP_CODE); > > vm_stop(VMSTOP_PANIC); > > + vm_status_set(VMST_INTERROR); > > } > > > > env->exit_request = 0; > > diff --git a/migration.c b/migration.c > > index af3a1f2..674792f 100644 > > --- a/migration.c > > +++ b/migration.c > > @@ -394,6 +394,9 @@ void migrate_fd_put_ready(void *opaque) > > } > > state = MIG_STATE_ERROR; > > } > > + if (state == MIG_STATE_COMPLETED) { > > + vm_status_set(VMST_POSTMIGRATE); > > + } > > s->state = state; > > notifier_list_notify(&migration_state_notifiers); > > } > > diff --git a/monitor.c b/monitor.c > > index 67ceb46..1cb3191 100644 > > --- a/monitor.c > > +++ b/monitor.c > > @@ -1258,6 +1258,7 @@ static void do_singlestep(Monitor *mon, const QDict *qdict) > > static int do_stop(Monitor *mon, const QDict *qdict, QObject **ret_data) > > { > > vm_stop(VMSTOP_USER); > > + vm_status_set(VMST_PAUSED); > > return 0; > > } > > > > @@ -2782,7 +2783,9 @@ static void do_loadvm(Monitor *mon, const QDict *qdict) > > > > vm_stop(VMSTOP_LOADVM); > > > > - if (load_vmstate(name) == 0&& saved_vm_running) { > > + if (load_vmstate(name)< 0) { > > + vm_status_set(VMST_LOADERROR); > > + } else if (saved_vm_running) { > > vm_start(); > > } > > } > > diff --git a/sysemu.h b/sysemu.h > > index d3013f5..7308ac5 100644 > > --- a/sysemu.h > > +++ b/sysemu.h > > @@ -77,6 +77,25 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f); > > void qemu_savevm_state_cancel(Monitor *mon, QEMUFile *f); > > int qemu_loadvm_state(QEMUFile *f); > > > > +typedef enum { > > + VMST_NOSTATUS, > > + VMST_DEBUG, > > + VMST_INMIGRATE, > > + VMST_POSTMIGRATE, > > + VMST_INTERROR, > > + VMST_IOERROR, > > + VMST_LOADERROR, > > + VMST_PAUSED, > > + VMST_PRELAUNCH, > > + VMST_RUNNING, > > + VMST_SHUTDOWN, > > + VMST_WATCHDOG, > > + VMST_MAX, > > +} VMStatus; > > + > > +void vm_status_set(VMStatus s); > > +const char *vm_status_get_name(void); > > + > > /* SLIRP */ > > void do_info_slirp(Monitor *mon); > > > > diff --git a/vl.c b/vl.c > > index fcd7395..fb7208f 100644 > > --- a/vl.c > > +++ b/vl.c > > @@ -309,6 +309,37 @@ static int default_driver_check(QemuOpts *opts, void *opaque) > > } > > > > /***********************************************************/ > > +/* VMStatus */ > > + > > +static const char *const vm_status_name[VMST_MAX] = { > > + [VMST_DEBUG] = "debug", > > + [VMST_INMIGRATE] = "inmigrate", > > + [VMST_POSTMIGRATE] = "postmigrate", > > + [VMST_INTERROR] = "internal-error", > > + [VMST_IOERROR] = "io-error", > > + [VMST_LOADERROR] = "load-state-error", > > + [VMST_PAUSED] = "paused", > > + [VMST_PRELAUNCH] = "prelaunch", > > + [VMST_RUNNING] = "running", > > + [VMST_SHUTDOWN] = "shutdown", > > + [VMST_WATCHDOG] = "watchdog-error", > > +}; > > + > > +static VMStatus qemu_vm_status = VMST_NOSTATUS; > > + > > +void vm_status_set(VMStatus s) > > +{ > > + assert(s> VMST_NOSTATUS&& s< VMST_MAX); > > + qemu_vm_status = s; > > +} > > + > > +const char *vm_status_get_name(void) > > +{ > > + assert(qemu_vm_status> VMST_NOSTATUS&& qemu_vm_status< VMST_MAX); > > + return vm_status_name[qemu_vm_status]; > > +} > > + > > +/***********************************************************/ > > /* real time host monotonic timer */ > > > > /***********************************************************/ > > @@ -1150,6 +1181,7 @@ void vm_start(void) > > vm_state_notify(1, 0); > > resume_all_vcpus(); > > monitor_protocol_event(QEVENT_RESUME, NULL); > > + vm_status_set(VMST_RUNNING); > > } > > } > > > > @@ -1392,12 +1424,14 @@ static void main_loop(void) > > > > if (qemu_debug_requested()) { > > vm_stop(VMSTOP_DEBUG); > > + vm_status_set(VMST_DEBUG); > > } > > if (qemu_shutdown_requested()) { > > qemu_kill_report(); > > monitor_protocol_event(QEVENT_SHUTDOWN, NULL); > > if (no_shutdown) { > > vm_stop(VMSTOP_SHUTDOWN); > > + vm_status_set(VMST_SHUTDOWN); > > no_shutdown = 0; > > } else > > break; > > @@ -2476,6 +2510,7 @@ int main(int argc, char **argv, char **envp) > > break; > > case QEMU_OPTION_S: > > autostart = 0; > > + vm_status_set(VMST_PRELAUNCH); > > break; > > case QEMU_OPTION_k: > > keyboard_layout = optarg; > > @@ -3307,11 +3342,13 @@ int main(int argc, char **argv, char **envp) > > qemu_system_reset(VMRESET_SILENT); > > if (loadvm) { > > if (load_vmstate(loadvm)< 0) { > > + vm_status_set(VMST_LOADERROR); > > autostart = 0; > > } > > } > > > > if (incoming) { > > + vm_status_set(VMST_INMIGRATE); > > int ret = qemu_start_incoming_migration(incoming); > > if (ret< 0) { > > fprintf(stderr, "Migration failed. Exit code %s(%d), exiting.\n", >
On Tue, 05 Jul 2011 13:58:34 -0500 Anthony Liguori <anthony@codemonkey.ws> wrote: > On 07/05/2011 01:51 PM, Luiz Capitulino wrote: > > On Tue, 05 Jul 2011 13:33:07 -0500 > > Anthony Liguori<anthony@codemonkey.ws> wrote: > > > >> On 07/05/2011 01:17 PM, Luiz Capitulino wrote: > >>> We need to track the VM status so that QMP can report it to clients. > >>> > >>> This commit adds the VMStatus type and related functions. The > >>> vm_status_set() function is used to keep track of the current > >>> VM status. > >>> > >>> The current statuses are: > >> > >> Which states are terminal, and what's the proper procedure to recover > >> from non-terminal states? Is it cont or system_reset followed by cont? > > > > Can I add that to the next patch (which also adds the QMP doc) or do you > > prefer it in this patch? > > It can be a separate patch, I was more curious about whether you had > thought through this for each of the states so that the states we were > introducing were well defined. I haven't to be honest. Adding it as documentation will be a good exercise though. Will do that for v2, but if you have comments about specific states, please, let me know now. > > For instance, it's not clear to me what the semantics of > 'load-state-error' are and how it's different from prelaunch. > > I'm quite surprised that 'load-state-error' wouldn't be a terminal error. Me too. I thought that starting (but not running) the VM after a failed load_vmstate() was a feature to allow debugging. I've never thought that it were possible to do 'cont' in this case (only badness can happen when doing this, no?). There are two solutions: 1. Just drop load-state-error 2. Doesn't allow a VM that failed a load_vmstate() call to be put to run, unless another call to load_vmstate() succeeds > > Regards, > > Anthony Liguori > > > > >> > >>> > >>> - debug: guest is running under gdb > >>> - inmigrate: guest is paused waiting for an incoming migration > >>> - postmigrate: guest is paused following a successful migration > >>> - internal-error: Fatal internal error that prevents further guest > >>> execution > >>> - load-state-error: guest is paused following a failed 'loadvm' > >>> - io-error: the last IOP has failed and the device is configured > >>> to pause on I/O errors > >>> - watchdog-error: the watchdog action is configured to pause and > >>> has been triggered > >>> - paused: guest has been paused via the 'stop' command > >>> - prelaunch: QEMU was started with -S and guest has not started > >>> - running: guest is actively running > >>> - shutdown: guest is shut down (and -no-shutdown is in use) > >> > >> Regards, > >> > >> Anthony Liguori > >> > >>> > >>> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com> > >>> --- > >>> gdbstub.c | 4 ++++ > >>> hw/ide/core.c | 1 + > >>> hw/scsi-disk.c | 1 + > >>> hw/virtio-blk.c | 1 + > >>> hw/watchdog.c | 1 + > >>> kvm-all.c | 1 + > >>> migration.c | 3 +++ > >>> monitor.c | 5 ++++- > >>> sysemu.h | 19 +++++++++++++++++++ > >>> vl.c | 37 +++++++++++++++++++++++++++++++++++++ > >>> 10 files changed, 72 insertions(+), 1 deletions(-) > >>> > >>> diff --git a/gdbstub.c b/gdbstub.c > >>> index c085a5a..61b700a 100644 > >>> --- a/gdbstub.c > >>> +++ b/gdbstub.c > >>> @@ -2358,6 +2358,7 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...) > >>> s->state = RS_SYSCALL; > >>> #ifndef CONFIG_USER_ONLY > >>> vm_stop(VMSTOP_DEBUG); > >>> + vm_status_set(VMST_DEBUG); > >>> #endif > >>> s->state = RS_IDLE; > >>> va_start(va, fmt); > >>> @@ -2432,6 +2433,7 @@ static void gdb_read_byte(GDBState *s, int ch) > >>> /* when the CPU is running, we cannot do anything except stop > >>> it when receiving a char */ > >>> vm_stop(VMSTOP_USER); > >>> + vm_status_set(VMST_DEBUG); > >>> } else > >>> #endif > >>> { > >>> @@ -2694,6 +2696,7 @@ static void gdb_chr_event(void *opaque, int event) > >>> switch (event) { > >>> case CHR_EVENT_OPENED: > >>> vm_stop(VMSTOP_USER); > >>> + vm_status_set(VMST_DEBUG); > >>> gdb_has_xml = 0; > >>> break; > >>> default: > >>> @@ -2735,6 +2738,7 @@ static void gdb_sigterm_handler(int signal) > >>> { > >>> if (vm_running) { > >>> vm_stop(VMSTOP_USER); > >>> + vm_status_set(VMST_DEBUG); > >>> } > >>> } > >>> #endif > >>> diff --git a/hw/ide/core.c b/hw/ide/core.c > >>> index ca17a43..bf9df41 100644 > >>> --- a/hw/ide/core.c > >>> +++ b/hw/ide/core.c > >>> @@ -523,6 +523,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int op) > >>> s->bus->error_status = op; > >>> bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); > >>> vm_stop(VMSTOP_DISKFULL); > >>> + vm_status_set(VMST_IOERROR); > >>> } else { > >>> if (op& BM_STATUS_DMA_RETRY) { > >>> dma_buf_commit(s, 0); > >>> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c > >>> index a8c7372..66037fd 100644 > >>> --- a/hw/scsi-disk.c > >>> +++ b/hw/scsi-disk.c > >>> @@ -216,6 +216,7 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type) > >>> > >>> bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); > >>> vm_stop(VMSTOP_DISKFULL); > >>> + vm_status_set(VMST_IOERROR); > >>> } else { > >>> if (type == SCSI_REQ_STATUS_RETRY_READ) { > >>> scsi_req_data(&r->req, 0); > >>> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c > >>> index 91e0394..bf70200 100644 > >>> --- a/hw/virtio-blk.c > >>> +++ b/hw/virtio-blk.c > >>> @@ -79,6 +79,7 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, > >>> s->rq = req; > >>> bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); > >>> vm_stop(VMSTOP_DISKFULL); > >>> + vm_status_set(VMST_IOERROR); > >>> } else { > >>> virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR); > >>> bdrv_mon_event(s->bs, BDRV_ACTION_REPORT, is_read); > >>> diff --git a/hw/watchdog.c b/hw/watchdog.c > >>> index 1c900a1..d130cbb 100644 > >>> --- a/hw/watchdog.c > >>> +++ b/hw/watchdog.c > >>> @@ -133,6 +133,7 @@ void watchdog_perform_action(void) > >>> case WDT_PAUSE: /* same as 'stop' command in monitor */ > >>> watchdog_mon_event("pause"); > >>> vm_stop(VMSTOP_WATCHDOG); > >>> + vm_status_set(VMST_WATCHDOG); > >>> break; > >>> > >>> case WDT_DEBUG: > >>> diff --git a/kvm-all.c b/kvm-all.c > >>> index cbc2532..aee9e0a 100644 > >>> --- a/kvm-all.c > >>> +++ b/kvm-all.c > >>> @@ -1015,6 +1015,7 @@ int kvm_cpu_exec(CPUState *env) > >>> if (ret< 0) { > >>> cpu_dump_state(env, stderr, fprintf, CPU_DUMP_CODE); > >>> vm_stop(VMSTOP_PANIC); > >>> + vm_status_set(VMST_INTERROR); > >>> } > >>> > >>> env->exit_request = 0; > >>> diff --git a/migration.c b/migration.c > >>> index af3a1f2..674792f 100644 > >>> --- a/migration.c > >>> +++ b/migration.c > >>> @@ -394,6 +394,9 @@ void migrate_fd_put_ready(void *opaque) > >>> } > >>> state = MIG_STATE_ERROR; > >>> } > >>> + if (state == MIG_STATE_COMPLETED) { > >>> + vm_status_set(VMST_POSTMIGRATE); > >>> + } > >>> s->state = state; > >>> notifier_list_notify(&migration_state_notifiers); > >>> } > >>> diff --git a/monitor.c b/monitor.c > >>> index 67ceb46..1cb3191 100644 > >>> --- a/monitor.c > >>> +++ b/monitor.c > >>> @@ -1258,6 +1258,7 @@ static void do_singlestep(Monitor *mon, const QDict *qdict) > >>> static int do_stop(Monitor *mon, const QDict *qdict, QObject **ret_data) > >>> { > >>> vm_stop(VMSTOP_USER); > >>> + vm_status_set(VMST_PAUSED); > >>> return 0; > >>> } > >>> > >>> @@ -2782,7 +2783,9 @@ static void do_loadvm(Monitor *mon, const QDict *qdict) > >>> > >>> vm_stop(VMSTOP_LOADVM); > >>> > >>> - if (load_vmstate(name) == 0&& saved_vm_running) { > >>> + if (load_vmstate(name)< 0) { > >>> + vm_status_set(VMST_LOADERROR); > >>> + } else if (saved_vm_running) { > >>> vm_start(); > >>> } > >>> } > >>> diff --git a/sysemu.h b/sysemu.h > >>> index d3013f5..7308ac5 100644 > >>> --- a/sysemu.h > >>> +++ b/sysemu.h > >>> @@ -77,6 +77,25 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f); > >>> void qemu_savevm_state_cancel(Monitor *mon, QEMUFile *f); > >>> int qemu_loadvm_state(QEMUFile *f); > >>> > >>> +typedef enum { > >>> + VMST_NOSTATUS, > >>> + VMST_DEBUG, > >>> + VMST_INMIGRATE, > >>> + VMST_POSTMIGRATE, > >>> + VMST_INTERROR, > >>> + VMST_IOERROR, > >>> + VMST_LOADERROR, > >>> + VMST_PAUSED, > >>> + VMST_PRELAUNCH, > >>> + VMST_RUNNING, > >>> + VMST_SHUTDOWN, > >>> + VMST_WATCHDOG, > >>> + VMST_MAX, > >>> +} VMStatus; > >>> + > >>> +void vm_status_set(VMStatus s); > >>> +const char *vm_status_get_name(void); > >>> + > >>> /* SLIRP */ > >>> void do_info_slirp(Monitor *mon); > >>> > >>> diff --git a/vl.c b/vl.c > >>> index fcd7395..fb7208f 100644 > >>> --- a/vl.c > >>> +++ b/vl.c > >>> @@ -309,6 +309,37 @@ static int default_driver_check(QemuOpts *opts, void *opaque) > >>> } > >>> > >>> /***********************************************************/ > >>> +/* VMStatus */ > >>> + > >>> +static const char *const vm_status_name[VMST_MAX] = { > >>> + [VMST_DEBUG] = "debug", > >>> + [VMST_INMIGRATE] = "inmigrate", > >>> + [VMST_POSTMIGRATE] = "postmigrate", > >>> + [VMST_INTERROR] = "internal-error", > >>> + [VMST_IOERROR] = "io-error", > >>> + [VMST_LOADERROR] = "load-state-error", > >>> + [VMST_PAUSED] = "paused", > >>> + [VMST_PRELAUNCH] = "prelaunch", > >>> + [VMST_RUNNING] = "running", > >>> + [VMST_SHUTDOWN] = "shutdown", > >>> + [VMST_WATCHDOG] = "watchdog-error", > >>> +}; > >>> + > >>> +static VMStatus qemu_vm_status = VMST_NOSTATUS; > >>> + > >>> +void vm_status_set(VMStatus s) > >>> +{ > >>> + assert(s> VMST_NOSTATUS&& s< VMST_MAX); > >>> + qemu_vm_status = s; > >>> +} > >>> + > >>> +const char *vm_status_get_name(void) > >>> +{ > >>> + assert(qemu_vm_status> VMST_NOSTATUS&& qemu_vm_status< VMST_MAX); > >>> + return vm_status_name[qemu_vm_status]; > >>> +} > >>> + > >>> +/***********************************************************/ > >>> /* real time host monotonic timer */ > >>> > >>> /***********************************************************/ > >>> @@ -1150,6 +1181,7 @@ void vm_start(void) > >>> vm_state_notify(1, 0); > >>> resume_all_vcpus(); > >>> monitor_protocol_event(QEVENT_RESUME, NULL); > >>> + vm_status_set(VMST_RUNNING); > >>> } > >>> } > >>> > >>> @@ -1392,12 +1424,14 @@ static void main_loop(void) > >>> > >>> if (qemu_debug_requested()) { > >>> vm_stop(VMSTOP_DEBUG); > >>> + vm_status_set(VMST_DEBUG); > >>> } > >>> if (qemu_shutdown_requested()) { > >>> qemu_kill_report(); > >>> monitor_protocol_event(QEVENT_SHUTDOWN, NULL); > >>> if (no_shutdown) { > >>> vm_stop(VMSTOP_SHUTDOWN); > >>> + vm_status_set(VMST_SHUTDOWN); > >>> no_shutdown = 0; > >>> } else > >>> break; > >>> @@ -2476,6 +2510,7 @@ int main(int argc, char **argv, char **envp) > >>> break; > >>> case QEMU_OPTION_S: > >>> autostart = 0; > >>> + vm_status_set(VMST_PRELAUNCH); > >>> break; > >>> case QEMU_OPTION_k: > >>> keyboard_layout = optarg; > >>> @@ -3307,11 +3342,13 @@ int main(int argc, char **argv, char **envp) > >>> qemu_system_reset(VMRESET_SILENT); > >>> if (loadvm) { > >>> if (load_vmstate(loadvm)< 0) { > >>> + vm_status_set(VMST_LOADERROR); > >>> autostart = 0; > >>> } > >>> } > >>> > >>> if (incoming) { > >>> + vm_status_set(VMST_INMIGRATE); > >>> int ret = qemu_start_incoming_migration(incoming); > >>> if (ret< 0) { > >>> fprintf(stderr, "Migration failed. Exit code %s(%d), exiting.\n", > >> > > >
Luiz Capitulino <lcapitulino@redhat.com> writes: > We need to track the VM status so that QMP can report it to clients. > > This commit adds the VMStatus type and related functions. The > vm_status_set() function is used to keep track of the current > VM status. > > The current statuses are: Nitpicking about names, bear with me. > - debug: guest is running under gdb > - inmigrate: guest is paused waiting for an incoming migration incoming-migration? > - postmigrate: guest is paused following a successful migration post-migrate? > - internal-error: Fatal internal error that prevents further guest > execution > - load-state-error: guest is paused following a failed 'loadvm' Less than obvious. If you like concrete, name it loadvm-failed. If you like abstract, name it restore-vm-failed. > - io-error: the last IOP has failed and the device is configured > to pause on I/O errors > - watchdog-error: the watchdog action is configured to pause and > has been triggered Sounds like the watchdog suffered an error. watchdog-fired? > - paused: guest has been paused via the 'stop' command stop-command? > - prelaunch: QEMU was started with -S and guest has not started unstarted? > - running: guest is actively running > - shutdown: guest is shut down (and -no-shutdown is in use) > > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> > --- > gdbstub.c | 4 ++++ > hw/ide/core.c | 1 + > hw/scsi-disk.c | 1 + > hw/virtio-blk.c | 1 + > hw/watchdog.c | 1 + > kvm-all.c | 1 + > migration.c | 3 +++ > monitor.c | 5 ++++- > sysemu.h | 19 +++++++++++++++++++ > vl.c | 37 +++++++++++++++++++++++++++++++++++++ > 10 files changed, 72 insertions(+), 1 deletions(-) > > diff --git a/gdbstub.c b/gdbstub.c > index c085a5a..61b700a 100644 > --- a/gdbstub.c > +++ b/gdbstub.c > @@ -2358,6 +2358,7 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...) > s->state = RS_SYSCALL; > #ifndef CONFIG_USER_ONLY > vm_stop(VMSTOP_DEBUG); > + vm_status_set(VMST_DEBUG); > #endif > s->state = RS_IDLE; > va_start(va, fmt); > @@ -2432,6 +2433,7 @@ static void gdb_read_byte(GDBState *s, int ch) > /* when the CPU is running, we cannot do anything except stop > it when receiving a char */ > vm_stop(VMSTOP_USER); > + vm_status_set(VMST_DEBUG); > } else > #endif > { > @@ -2694,6 +2696,7 @@ static void gdb_chr_event(void *opaque, int event) > switch (event) { > case CHR_EVENT_OPENED: > vm_stop(VMSTOP_USER); > + vm_status_set(VMST_DEBUG); > gdb_has_xml = 0; > break; > default: Previous hunk has VMST_DEBUG with VMST_DEBUG. Odd. > @@ -2735,6 +2738,7 @@ static void gdb_sigterm_handler(int signal) > { > if (vm_running) { > vm_stop(VMSTOP_USER); > + vm_status_set(VMST_DEBUG); > } > } > #endif > diff --git a/hw/ide/core.c b/hw/ide/core.c > index ca17a43..bf9df41 100644 > --- a/hw/ide/core.c > +++ b/hw/ide/core.c > @@ -523,6 +523,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int op) > s->bus->error_status = op; > bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); > vm_stop(VMSTOP_DISKFULL); > + vm_status_set(VMST_IOERROR); > } else { > if (op & BM_STATUS_DMA_RETRY) { > dma_buf_commit(s, 0); > diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c > index a8c7372..66037fd 100644 > --- a/hw/scsi-disk.c > +++ b/hw/scsi-disk.c > @@ -216,6 +216,7 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type) > > bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); > vm_stop(VMSTOP_DISKFULL); > + vm_status_set(VMST_IOERROR); > } else { > if (type == SCSI_REQ_STATUS_RETRY_READ) { > scsi_req_data(&r->req, 0); > diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c > index 91e0394..bf70200 100644 > --- a/hw/virtio-blk.c > +++ b/hw/virtio-blk.c > @@ -79,6 +79,7 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, > s->rq = req; > bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); > vm_stop(VMSTOP_DISKFULL); > + vm_status_set(VMST_IOERROR); > } else { > virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR); > bdrv_mon_event(s->bs, BDRV_ACTION_REPORT, is_read); > diff --git a/hw/watchdog.c b/hw/watchdog.c > index 1c900a1..d130cbb 100644 > --- a/hw/watchdog.c > +++ b/hw/watchdog.c > @@ -133,6 +133,7 @@ void watchdog_perform_action(void) > case WDT_PAUSE: /* same as 'stop' command in monitor */ > watchdog_mon_event("pause"); > vm_stop(VMSTOP_WATCHDOG); > + vm_status_set(VMST_WATCHDOG); > break; > > case WDT_DEBUG: > diff --git a/kvm-all.c b/kvm-all.c > index cbc2532..aee9e0a 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -1015,6 +1015,7 @@ int kvm_cpu_exec(CPUState *env) > if (ret < 0) { > cpu_dump_state(env, stderr, fprintf, CPU_DUMP_CODE); > vm_stop(VMSTOP_PANIC); > + vm_status_set(VMST_INTERROR); > } > > env->exit_request = 0; > diff --git a/migration.c b/migration.c > index af3a1f2..674792f 100644 > --- a/migration.c > +++ b/migration.c > @@ -394,6 +394,9 @@ void migrate_fd_put_ready(void *opaque) > } > state = MIG_STATE_ERROR; > } > + if (state == MIG_STATE_COMPLETED) { > + vm_status_set(VMST_POSTMIGRATE); > + } > s->state = state; > notifier_list_notify(&migration_state_notifiers); > } > diff --git a/monitor.c b/monitor.c > index 67ceb46..1cb3191 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -1258,6 +1258,7 @@ static void do_singlestep(Monitor *mon, const QDict *qdict) > static int do_stop(Monitor *mon, const QDict *qdict, QObject **ret_data) > { > vm_stop(VMSTOP_USER); > + vm_status_set(VMST_PAUSED); > return 0; > } > > @@ -2782,7 +2783,9 @@ static void do_loadvm(Monitor *mon, const QDict *qdict) > > vm_stop(VMSTOP_LOADVM); > > - if (load_vmstate(name) == 0 && saved_vm_running) { > + if (load_vmstate(name) < 0) { > + vm_status_set(VMST_LOADERROR); > + } else if (saved_vm_running) { > vm_start(); > } > } > diff --git a/sysemu.h b/sysemu.h > index d3013f5..7308ac5 100644 > --- a/sysemu.h > +++ b/sysemu.h > @@ -77,6 +77,25 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f); > void qemu_savevm_state_cancel(Monitor *mon, QEMUFile *f); > int qemu_loadvm_state(QEMUFile *f); > > +typedef enum { > + VMST_NOSTATUS, > + VMST_DEBUG, > + VMST_INMIGRATE, > + VMST_POSTMIGRATE, > + VMST_INTERROR, > + VMST_IOERROR, > + VMST_LOADERROR, > + VMST_PAUSED, > + VMST_PRELAUNCH, > + VMST_RUNNING, > + VMST_SHUTDOWN, > + VMST_WATCHDOG, > + VMST_MAX, > +} VMStatus; How are these related to the VMSTOP_*? Why do we need a separate enumeration? > + > +void vm_status_set(VMStatus s); > +const char *vm_status_get_name(void); > + > /* SLIRP */ > void do_info_slirp(Monitor *mon); > > diff --git a/vl.c b/vl.c > index fcd7395..fb7208f 100644 > --- a/vl.c > +++ b/vl.c > @@ -309,6 +309,37 @@ static int default_driver_check(QemuOpts *opts, void *opaque) > } > > /***********************************************************/ > +/* VMStatus */ > + > +static const char *const vm_status_name[VMST_MAX] = { > + [VMST_DEBUG] = "debug", > + [VMST_INMIGRATE] = "inmigrate", > + [VMST_POSTMIGRATE] = "postmigrate", > + [VMST_INTERROR] = "internal-error", > + [VMST_IOERROR] = "io-error", > + [VMST_LOADERROR] = "load-state-error", > + [VMST_PAUSED] = "paused", > + [VMST_PRELAUNCH] = "prelaunch", > + [VMST_RUNNING] = "running", > + [VMST_SHUTDOWN] = "shutdown", > + [VMST_WATCHDOG] = "watchdog-error", > +}; > + > +static VMStatus qemu_vm_status = VMST_NOSTATUS; > + > +void vm_status_set(VMStatus s) > +{ > + assert(s > VMST_NOSTATUS && s < VMST_MAX); > + qemu_vm_status = s; > +} > + > +const char *vm_status_get_name(void) > +{ > + assert(qemu_vm_status > VMST_NOSTATUS && qemu_vm_status < VMST_MAX); > + return vm_status_name[qemu_vm_status]; > +} > + Couldn't find a better place than the ol' vl.c rubbish dump ;) [...]
On Tue, 12 Jul 2011 09:28:05 +0200 Markus Armbruster <armbru@redhat.com> wrote: > Luiz Capitulino <lcapitulino@redhat.com> writes: > > > We need to track the VM status so that QMP can report it to clients. > > > > This commit adds the VMStatus type and related functions. The > > vm_status_set() function is used to keep track of the current > > VM status. > > > > The current statuses are: > > Nitpicking about names, bear with me. > > > - debug: guest is running under gdb > > - inmigrate: guest is paused waiting for an incoming migration > > incoming-migration? > > > - postmigrate: guest is paused following a successful migration > > post-migrate? > > > - internal-error: Fatal internal error that prevents further guest > > execution > > - load-state-error: guest is paused following a failed 'loadvm' > > Less than obvious. If you like concrete, name it loadvm-failed. If you > like abstract, name it restore-vm-failed. Ok for your suggestions above. > > > - io-error: the last IOP has failed and the device is configured > > to pause on I/O errors > > - watchdog-error: the watchdog action is configured to pause and > > has been triggered > > Sounds like the watchdog suffered an error. watchdog-fired? Maybe watchdog-paused. > > > - paused: guest has been paused via the 'stop' command > > stop-command? I prefer 'paused', it communicates better the state we're in. > > > - prelaunch: QEMU was started with -S and guest has not started > > unstarted? Looks the same to me. > > > - running: guest is actively running > > - shutdown: guest is shut down (and -no-shutdown is in use) > > > > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> > > --- > > gdbstub.c | 4 ++++ > > hw/ide/core.c | 1 + > > hw/scsi-disk.c | 1 + > > hw/virtio-blk.c | 1 + > > hw/watchdog.c | 1 + > > kvm-all.c | 1 + > > migration.c | 3 +++ > > monitor.c | 5 ++++- > > sysemu.h | 19 +++++++++++++++++++ > > vl.c | 37 +++++++++++++++++++++++++++++++++++++ > > 10 files changed, 72 insertions(+), 1 deletions(-) > > > > diff --git a/gdbstub.c b/gdbstub.c > > index c085a5a..61b700a 100644 > > --- a/gdbstub.c > > +++ b/gdbstub.c > > @@ -2358,6 +2358,7 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...) > > s->state = RS_SYSCALL; > > #ifndef CONFIG_USER_ONLY > > vm_stop(VMSTOP_DEBUG); > > + vm_status_set(VMST_DEBUG); > > #endif > > s->state = RS_IDLE; > > va_start(va, fmt); > > @@ -2432,6 +2433,7 @@ static void gdb_read_byte(GDBState *s, int ch) > > /* when the CPU is running, we cannot do anything except stop > > it when receiving a char */ > > vm_stop(VMSTOP_USER); > > + vm_status_set(VMST_DEBUG); > > } else > > #endif > > { > > @@ -2694,6 +2696,7 @@ static void gdb_chr_event(void *opaque, int event) > > switch (event) { > > case CHR_EVENT_OPENED: > > vm_stop(VMSTOP_USER); > > + vm_status_set(VMST_DEBUG); > > gdb_has_xml = 0; > > break; > > default: > > Previous hunk has VMST_DEBUG with VMST_DEBUG. Odd. > > > @@ -2735,6 +2738,7 @@ static void gdb_sigterm_handler(int signal) > > { > > if (vm_running) { > > vm_stop(VMSTOP_USER); > > + vm_status_set(VMST_DEBUG); > > } > > } > > #endif > > diff --git a/hw/ide/core.c b/hw/ide/core.c > > index ca17a43..bf9df41 100644 > > --- a/hw/ide/core.c > > +++ b/hw/ide/core.c > > @@ -523,6 +523,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int op) > > s->bus->error_status = op; > > bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); > > vm_stop(VMSTOP_DISKFULL); > > + vm_status_set(VMST_IOERROR); > > } else { > > if (op & BM_STATUS_DMA_RETRY) { > > dma_buf_commit(s, 0); > > diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c > > index a8c7372..66037fd 100644 > > --- a/hw/scsi-disk.c > > +++ b/hw/scsi-disk.c > > @@ -216,6 +216,7 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type) > > > > bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); > > vm_stop(VMSTOP_DISKFULL); > > + vm_status_set(VMST_IOERROR); > > } else { > > if (type == SCSI_REQ_STATUS_RETRY_READ) { > > scsi_req_data(&r->req, 0); > > diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c > > index 91e0394..bf70200 100644 > > --- a/hw/virtio-blk.c > > +++ b/hw/virtio-blk.c > > @@ -79,6 +79,7 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, > > s->rq = req; > > bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); > > vm_stop(VMSTOP_DISKFULL); > > + vm_status_set(VMST_IOERROR); > > } else { > > virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR); > > bdrv_mon_event(s->bs, BDRV_ACTION_REPORT, is_read); > > diff --git a/hw/watchdog.c b/hw/watchdog.c > > index 1c900a1..d130cbb 100644 > > --- a/hw/watchdog.c > > +++ b/hw/watchdog.c > > @@ -133,6 +133,7 @@ void watchdog_perform_action(void) > > case WDT_PAUSE: /* same as 'stop' command in monitor */ > > watchdog_mon_event("pause"); > > vm_stop(VMSTOP_WATCHDOG); > > + vm_status_set(VMST_WATCHDOG); > > break; > > > > case WDT_DEBUG: > > diff --git a/kvm-all.c b/kvm-all.c > > index cbc2532..aee9e0a 100644 > > --- a/kvm-all.c > > +++ b/kvm-all.c > > @@ -1015,6 +1015,7 @@ int kvm_cpu_exec(CPUState *env) > > if (ret < 0) { > > cpu_dump_state(env, stderr, fprintf, CPU_DUMP_CODE); > > vm_stop(VMSTOP_PANIC); > > + vm_status_set(VMST_INTERROR); > > } > > > > env->exit_request = 0; > > diff --git a/migration.c b/migration.c > > index af3a1f2..674792f 100644 > > --- a/migration.c > > +++ b/migration.c > > @@ -394,6 +394,9 @@ void migrate_fd_put_ready(void *opaque) > > } > > state = MIG_STATE_ERROR; > > } > > + if (state == MIG_STATE_COMPLETED) { > > + vm_status_set(VMST_POSTMIGRATE); > > + } > > s->state = state; > > notifier_list_notify(&migration_state_notifiers); > > } > > diff --git a/monitor.c b/monitor.c > > index 67ceb46..1cb3191 100644 > > --- a/monitor.c > > +++ b/monitor.c > > @@ -1258,6 +1258,7 @@ static void do_singlestep(Monitor *mon, const QDict *qdict) > > static int do_stop(Monitor *mon, const QDict *qdict, QObject **ret_data) > > { > > vm_stop(VMSTOP_USER); > > + vm_status_set(VMST_PAUSED); > > return 0; > > } > > > > @@ -2782,7 +2783,9 @@ static void do_loadvm(Monitor *mon, const QDict *qdict) > > > > vm_stop(VMSTOP_LOADVM); > > > > - if (load_vmstate(name) == 0 && saved_vm_running) { > > + if (load_vmstate(name) < 0) { > > + vm_status_set(VMST_LOADERROR); > > + } else if (saved_vm_running) { > > vm_start(); > > } > > } > > diff --git a/sysemu.h b/sysemu.h > > index d3013f5..7308ac5 100644 > > --- a/sysemu.h > > +++ b/sysemu.h > > @@ -77,6 +77,25 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f); > > void qemu_savevm_state_cancel(Monitor *mon, QEMUFile *f); > > int qemu_loadvm_state(QEMUFile *f); > > > > +typedef enum { > > + VMST_NOSTATUS, > > + VMST_DEBUG, > > + VMST_INMIGRATE, > > + VMST_POSTMIGRATE, > > + VMST_INTERROR, > > + VMST_IOERROR, > > + VMST_LOADERROR, > > + VMST_PAUSED, > > + VMST_PRELAUNCH, > > + VMST_RUNNING, > > + VMST_SHUTDOWN, > > + VMST_WATCHDOG, > > + VMST_MAX, > > +} VMStatus; > > How are these related to the VMSTOP_*? > > Why do we need a separate enumeration? > > > + > > +void vm_status_set(VMStatus s); > > +const char *vm_status_get_name(void); > > + > > /* SLIRP */ > > void do_info_slirp(Monitor *mon); > > > > diff --git a/vl.c b/vl.c > > index fcd7395..fb7208f 100644 > > --- a/vl.c > > +++ b/vl.c > > @@ -309,6 +309,37 @@ static int default_driver_check(QemuOpts *opts, void *opaque) > > } > > > > /***********************************************************/ > > +/* VMStatus */ > > + > > +static const char *const vm_status_name[VMST_MAX] = { > > + [VMST_DEBUG] = "debug", > > + [VMST_INMIGRATE] = "inmigrate", > > + [VMST_POSTMIGRATE] = "postmigrate", > > + [VMST_INTERROR] = "internal-error", > > + [VMST_IOERROR] = "io-error", > > + [VMST_LOADERROR] = "load-state-error", > > + [VMST_PAUSED] = "paused", > > + [VMST_PRELAUNCH] = "prelaunch", > > + [VMST_RUNNING] = "running", > > + [VMST_SHUTDOWN] = "shutdown", > > + [VMST_WATCHDOG] = "watchdog-error", > > +}; > > + > > +static VMStatus qemu_vm_status = VMST_NOSTATUS; > > + > > +void vm_status_set(VMStatus s) > > +{ > > + assert(s > VMST_NOSTATUS && s < VMST_MAX); > > + qemu_vm_status = s; > > +} > > + > > +const char *vm_status_get_name(void) > > +{ > > + assert(qemu_vm_status > VMST_NOSTATUS && qemu_vm_status < VMST_MAX); > > + return vm_status_name[qemu_vm_status]; > > +} > > + > > Couldn't find a better place than the ol' vl.c rubbish dump ;) > > [...] >
Am 12.07.2011 16:25, schrieb Luiz Capitulino: > On Tue, 12 Jul 2011 09:28:05 +0200 > Markus Armbruster <armbru@redhat.com> wrote: > >> Luiz Capitulino <lcapitulino@redhat.com> writes: >> >>> We need to track the VM status so that QMP can report it to clients. >>> >>> This commit adds the VMStatus type and related functions. The >>> vm_status_set() function is used to keep track of the current >>> VM status. >>> >>> The current statuses are: >> >> Nitpicking about names, bear with me. >> >>> - debug: guest is running under gdb >>> - inmigrate: guest is paused waiting for an incoming migration >> >> incoming-migration? >> >>> - postmigrate: guest is paused following a successful migration >> >> post-migrate? >> >>> - internal-error: Fatal internal error that prevents further guest >>> execution >>> - load-state-error: guest is paused following a failed 'loadvm' >> >> Less than obvious. If you like concrete, name it loadvm-failed. If you >> like abstract, name it restore-vm-failed. > > Ok for your suggestions above. > >> >>> - io-error: the last IOP has failed and the device is configured >>> to pause on I/O errors >>> - watchdog-error: the watchdog action is configured to pause and >>> has been triggered >> >> Sounds like the watchdog suffered an error. watchdog-fired? > > Maybe watchdog-paused. > >> >>> - paused: guest has been paused via the 'stop' command >> >> stop-command? > > I prefer 'paused', it communicates better the state we're in. > >> >>> - prelaunch: QEMU was started with -S and guest has not started >> >> unstarted? > > Looks the same to me. > >> >>> - running: guest is actively running >>> - shutdown: guest is shut down (and -no-shutdown is in use) >>> >>> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> >>> --- >>> gdbstub.c | 4 ++++ >>> hw/ide/core.c | 1 + >>> hw/scsi-disk.c | 1 + >>> hw/virtio-blk.c | 1 + >>> hw/watchdog.c | 1 + >>> kvm-all.c | 1 + >>> migration.c | 3 +++ >>> monitor.c | 5 ++++- >>> sysemu.h | 19 +++++++++++++++++++ >>> vl.c | 37 +++++++++++++++++++++++++++++++++++++ >>> 10 files changed, 72 insertions(+), 1 deletions(-) >>> >>> diff --git a/gdbstub.c b/gdbstub.c >>> index c085a5a..61b700a 100644 >>> --- a/gdbstub.c >>> +++ b/gdbstub.c >>> @@ -2358,6 +2358,7 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...) >>> s->state = RS_SYSCALL; >>> #ifndef CONFIG_USER_ONLY >>> vm_stop(VMSTOP_DEBUG); >>> + vm_status_set(VMST_DEBUG); >>> #endif >>> s->state = RS_IDLE; >>> va_start(va, fmt); >>> @@ -2432,6 +2433,7 @@ static void gdb_read_byte(GDBState *s, int ch) >>> /* when the CPU is running, we cannot do anything except stop >>> it when receiving a char */ >>> vm_stop(VMSTOP_USER); >>> + vm_status_set(VMST_DEBUG); >>> } else >>> #endif >>> { >>> @@ -2694,6 +2696,7 @@ static void gdb_chr_event(void *opaque, int event) >>> switch (event) { >>> case CHR_EVENT_OPENED: >>> vm_stop(VMSTOP_USER); >>> + vm_status_set(VMST_DEBUG); >>> gdb_has_xml = 0; >>> break; >>> default: >> >> Previous hunk has VMST_DEBUG with VMST_DEBUG. Odd. >> >>> @@ -2735,6 +2738,7 @@ static void gdb_sigterm_handler(int signal) >>> { >>> if (vm_running) { >>> vm_stop(VMSTOP_USER); >>> + vm_status_set(VMST_DEBUG); >>> } >>> } >>> #endif >>> diff --git a/hw/ide/core.c b/hw/ide/core.c >>> index ca17a43..bf9df41 100644 >>> --- a/hw/ide/core.c >>> +++ b/hw/ide/core.c >>> @@ -523,6 +523,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int op) >>> s->bus->error_status = op; >>> bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); >>> vm_stop(VMSTOP_DISKFULL); >>> + vm_status_set(VMST_IOERROR); >>> } else { >>> if (op & BM_STATUS_DMA_RETRY) { >>> dma_buf_commit(s, 0); >>> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c >>> index a8c7372..66037fd 100644 >>> --- a/hw/scsi-disk.c >>> +++ b/hw/scsi-disk.c >>> @@ -216,6 +216,7 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type) >>> >>> bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); >>> vm_stop(VMSTOP_DISKFULL); >>> + vm_status_set(VMST_IOERROR); >>> } else { >>> if (type == SCSI_REQ_STATUS_RETRY_READ) { >>> scsi_req_data(&r->req, 0); >>> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c >>> index 91e0394..bf70200 100644 >>> --- a/hw/virtio-blk.c >>> +++ b/hw/virtio-blk.c >>> @@ -79,6 +79,7 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, >>> s->rq = req; >>> bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); >>> vm_stop(VMSTOP_DISKFULL); >>> + vm_status_set(VMST_IOERROR); >>> } else { >>> virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR); >>> bdrv_mon_event(s->bs, BDRV_ACTION_REPORT, is_read); >>> diff --git a/hw/watchdog.c b/hw/watchdog.c >>> index 1c900a1..d130cbb 100644 >>> --- a/hw/watchdog.c >>> +++ b/hw/watchdog.c >>> @@ -133,6 +133,7 @@ void watchdog_perform_action(void) >>> case WDT_PAUSE: /* same as 'stop' command in monitor */ >>> watchdog_mon_event("pause"); >>> vm_stop(VMSTOP_WATCHDOG); >>> + vm_status_set(VMST_WATCHDOG); >>> break; >>> >>> case WDT_DEBUG: >>> diff --git a/kvm-all.c b/kvm-all.c >>> index cbc2532..aee9e0a 100644 >>> --- a/kvm-all.c >>> +++ b/kvm-all.c >>> @@ -1015,6 +1015,7 @@ int kvm_cpu_exec(CPUState *env) >>> if (ret < 0) { >>> cpu_dump_state(env, stderr, fprintf, CPU_DUMP_CODE); >>> vm_stop(VMSTOP_PANIC); >>> + vm_status_set(VMST_INTERROR); >>> } >>> >>> env->exit_request = 0; >>> diff --git a/migration.c b/migration.c >>> index af3a1f2..674792f 100644 >>> --- a/migration.c >>> +++ b/migration.c >>> @@ -394,6 +394,9 @@ void migrate_fd_put_ready(void *opaque) >>> } >>> state = MIG_STATE_ERROR; >>> } >>> + if (state == MIG_STATE_COMPLETED) { >>> + vm_status_set(VMST_POSTMIGRATE); >>> + } >>> s->state = state; >>> notifier_list_notify(&migration_state_notifiers); >>> } >>> diff --git a/monitor.c b/monitor.c >>> index 67ceb46..1cb3191 100644 >>> --- a/monitor.c >>> +++ b/monitor.c >>> @@ -1258,6 +1258,7 @@ static void do_singlestep(Monitor *mon, const QDict *qdict) >>> static int do_stop(Monitor *mon, const QDict *qdict, QObject **ret_data) >>> { >>> vm_stop(VMSTOP_USER); >>> + vm_status_set(VMST_PAUSED); >>> return 0; >>> } >>> >>> @@ -2782,7 +2783,9 @@ static void do_loadvm(Monitor *mon, const QDict *qdict) >>> >>> vm_stop(VMSTOP_LOADVM); >>> >>> - if (load_vmstate(name) == 0 && saved_vm_running) { >>> + if (load_vmstate(name) < 0) { >>> + vm_status_set(VMST_LOADERROR); >>> + } else if (saved_vm_running) { >>> vm_start(); >>> } >>> } >>> diff --git a/sysemu.h b/sysemu.h >>> index d3013f5..7308ac5 100644 >>> --- a/sysemu.h >>> +++ b/sysemu.h >>> @@ -77,6 +77,25 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f); >>> void qemu_savevm_state_cancel(Monitor *mon, QEMUFile *f); >>> int qemu_loadvm_state(QEMUFile *f); >>> >>> +typedef enum { >>> + VMST_NOSTATUS, >>> + VMST_DEBUG, >>> + VMST_INMIGRATE, >>> + VMST_POSTMIGRATE, >>> + VMST_INTERROR, >>> + VMST_IOERROR, >>> + VMST_LOADERROR, >>> + VMST_PAUSED, >>> + VMST_PRELAUNCH, >>> + VMST_RUNNING, >>> + VMST_SHUTDOWN, >>> + VMST_WATCHDOG, >>> + VMST_MAX, >>> +} VMStatus; >> >> How are these related to the VMSTOP_*? >> >> Why do we need a separate enumeration? Luiz, what about this part? For me, this was the most important question. We already have VMSTOP_*, and every caller of vm_stop() should change the status (most of the vm_status_set() calls come immediately after a vm_stop() call), so it would appear logical that vm_stop(), which already gets a reason, sets the status. Probably we would need a few additional reasons for vm_stop(), but keeping two separate status values for almost the same thing looks suspicious. Kevin
On Tue, 12 Jul 2011 16:51:03 +0200 Kevin Wolf <kwolf@redhat.com> wrote: > Am 12.07.2011 16:25, schrieb Luiz Capitulino: > > On Tue, 12 Jul 2011 09:28:05 +0200 > > Markus Armbruster <armbru@redhat.com> wrote: > > > >> Luiz Capitulino <lcapitulino@redhat.com> writes: > >> > >>> We need to track the VM status so that QMP can report it to clients. > >>> > >>> This commit adds the VMStatus type and related functions. The > >>> vm_status_set() function is used to keep track of the current > >>> VM status. > >>> > >>> The current statuses are: > >> > >> Nitpicking about names, bear with me. > >> > >>> - debug: guest is running under gdb > >>> - inmigrate: guest is paused waiting for an incoming migration > >> > >> incoming-migration? > >> > >>> - postmigrate: guest is paused following a successful migration > >> > >> post-migrate? > >> > >>> - internal-error: Fatal internal error that prevents further guest > >>> execution > >>> - load-state-error: guest is paused following a failed 'loadvm' > >> > >> Less than obvious. If you like concrete, name it loadvm-failed. If you > >> like abstract, name it restore-vm-failed. > > > > Ok for your suggestions above. > > > >> > >>> - io-error: the last IOP has failed and the device is configured > >>> to pause on I/O errors > >>> - watchdog-error: the watchdog action is configured to pause and > >>> has been triggered > >> > >> Sounds like the watchdog suffered an error. watchdog-fired? > > > > Maybe watchdog-paused. > > > >> > >>> - paused: guest has been paused via the 'stop' command > >> > >> stop-command? > > > > I prefer 'paused', it communicates better the state we're in. > > > >> > >>> - prelaunch: QEMU was started with -S and guest has not started > >> > >> unstarted? > > > > Looks the same to me. > > > >> > >>> - running: guest is actively running > >>> - shutdown: guest is shut down (and -no-shutdown is in use) > >>> > >>> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> > >>> --- > >>> gdbstub.c | 4 ++++ > >>> hw/ide/core.c | 1 + > >>> hw/scsi-disk.c | 1 + > >>> hw/virtio-blk.c | 1 + > >>> hw/watchdog.c | 1 + > >>> kvm-all.c | 1 + > >>> migration.c | 3 +++ > >>> monitor.c | 5 ++++- > >>> sysemu.h | 19 +++++++++++++++++++ > >>> vl.c | 37 +++++++++++++++++++++++++++++++++++++ > >>> 10 files changed, 72 insertions(+), 1 deletions(-) > >>> > >>> diff --git a/gdbstub.c b/gdbstub.c > >>> index c085a5a..61b700a 100644 > >>> --- a/gdbstub.c > >>> +++ b/gdbstub.c > >>> @@ -2358,6 +2358,7 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...) > >>> s->state = RS_SYSCALL; > >>> #ifndef CONFIG_USER_ONLY > >>> vm_stop(VMSTOP_DEBUG); > >>> + vm_status_set(VMST_DEBUG); > >>> #endif > >>> s->state = RS_IDLE; > >>> va_start(va, fmt); > >>> @@ -2432,6 +2433,7 @@ static void gdb_read_byte(GDBState *s, int ch) > >>> /* when the CPU is running, we cannot do anything except stop > >>> it when receiving a char */ > >>> vm_stop(VMSTOP_USER); > >>> + vm_status_set(VMST_DEBUG); > >>> } else > >>> #endif > >>> { > >>> @@ -2694,6 +2696,7 @@ static void gdb_chr_event(void *opaque, int event) > >>> switch (event) { > >>> case CHR_EVENT_OPENED: > >>> vm_stop(VMSTOP_USER); > >>> + vm_status_set(VMST_DEBUG); > >>> gdb_has_xml = 0; > >>> break; > >>> default: > >> > >> Previous hunk has VMST_DEBUG with VMST_DEBUG. Odd. > >> > >>> @@ -2735,6 +2738,7 @@ static void gdb_sigterm_handler(int signal) > >>> { > >>> if (vm_running) { > >>> vm_stop(VMSTOP_USER); > >>> + vm_status_set(VMST_DEBUG); > >>> } > >>> } > >>> #endif > >>> diff --git a/hw/ide/core.c b/hw/ide/core.c > >>> index ca17a43..bf9df41 100644 > >>> --- a/hw/ide/core.c > >>> +++ b/hw/ide/core.c > >>> @@ -523,6 +523,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int op) > >>> s->bus->error_status = op; > >>> bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); > >>> vm_stop(VMSTOP_DISKFULL); > >>> + vm_status_set(VMST_IOERROR); > >>> } else { > >>> if (op & BM_STATUS_DMA_RETRY) { > >>> dma_buf_commit(s, 0); > >>> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c > >>> index a8c7372..66037fd 100644 > >>> --- a/hw/scsi-disk.c > >>> +++ b/hw/scsi-disk.c > >>> @@ -216,6 +216,7 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type) > >>> > >>> bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); > >>> vm_stop(VMSTOP_DISKFULL); > >>> + vm_status_set(VMST_IOERROR); > >>> } else { > >>> if (type == SCSI_REQ_STATUS_RETRY_READ) { > >>> scsi_req_data(&r->req, 0); > >>> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c > >>> index 91e0394..bf70200 100644 > >>> --- a/hw/virtio-blk.c > >>> +++ b/hw/virtio-blk.c > >>> @@ -79,6 +79,7 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, > >>> s->rq = req; > >>> bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); > >>> vm_stop(VMSTOP_DISKFULL); > >>> + vm_status_set(VMST_IOERROR); > >>> } else { > >>> virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR); > >>> bdrv_mon_event(s->bs, BDRV_ACTION_REPORT, is_read); > >>> diff --git a/hw/watchdog.c b/hw/watchdog.c > >>> index 1c900a1..d130cbb 100644 > >>> --- a/hw/watchdog.c > >>> +++ b/hw/watchdog.c > >>> @@ -133,6 +133,7 @@ void watchdog_perform_action(void) > >>> case WDT_PAUSE: /* same as 'stop' command in monitor */ > >>> watchdog_mon_event("pause"); > >>> vm_stop(VMSTOP_WATCHDOG); > >>> + vm_status_set(VMST_WATCHDOG); > >>> break; > >>> > >>> case WDT_DEBUG: > >>> diff --git a/kvm-all.c b/kvm-all.c > >>> index cbc2532..aee9e0a 100644 > >>> --- a/kvm-all.c > >>> +++ b/kvm-all.c > >>> @@ -1015,6 +1015,7 @@ int kvm_cpu_exec(CPUState *env) > >>> if (ret < 0) { > >>> cpu_dump_state(env, stderr, fprintf, CPU_DUMP_CODE); > >>> vm_stop(VMSTOP_PANIC); > >>> + vm_status_set(VMST_INTERROR); > >>> } > >>> > >>> env->exit_request = 0; > >>> diff --git a/migration.c b/migration.c > >>> index af3a1f2..674792f 100644 > >>> --- a/migration.c > >>> +++ b/migration.c > >>> @@ -394,6 +394,9 @@ void migrate_fd_put_ready(void *opaque) > >>> } > >>> state = MIG_STATE_ERROR; > >>> } > >>> + if (state == MIG_STATE_COMPLETED) { > >>> + vm_status_set(VMST_POSTMIGRATE); > >>> + } > >>> s->state = state; > >>> notifier_list_notify(&migration_state_notifiers); > >>> } > >>> diff --git a/monitor.c b/monitor.c > >>> index 67ceb46..1cb3191 100644 > >>> --- a/monitor.c > >>> +++ b/monitor.c > >>> @@ -1258,6 +1258,7 @@ static void do_singlestep(Monitor *mon, const QDict *qdict) > >>> static int do_stop(Monitor *mon, const QDict *qdict, QObject **ret_data) > >>> { > >>> vm_stop(VMSTOP_USER); > >>> + vm_status_set(VMST_PAUSED); > >>> return 0; > >>> } > >>> > >>> @@ -2782,7 +2783,9 @@ static void do_loadvm(Monitor *mon, const QDict *qdict) > >>> > >>> vm_stop(VMSTOP_LOADVM); > >>> > >>> - if (load_vmstate(name) == 0 && saved_vm_running) { > >>> + if (load_vmstate(name) < 0) { > >>> + vm_status_set(VMST_LOADERROR); > >>> + } else if (saved_vm_running) { > >>> vm_start(); > >>> } > >>> } > >>> diff --git a/sysemu.h b/sysemu.h > >>> index d3013f5..7308ac5 100644 > >>> --- a/sysemu.h > >>> +++ b/sysemu.h > >>> @@ -77,6 +77,25 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f); > >>> void qemu_savevm_state_cancel(Monitor *mon, QEMUFile *f); > >>> int qemu_loadvm_state(QEMUFile *f); > >>> > >>> +typedef enum { > >>> + VMST_NOSTATUS, > >>> + VMST_DEBUG, > >>> + VMST_INMIGRATE, > >>> + VMST_POSTMIGRATE, > >>> + VMST_INTERROR, > >>> + VMST_IOERROR, > >>> + VMST_LOADERROR, > >>> + VMST_PAUSED, > >>> + VMST_PRELAUNCH, > >>> + VMST_RUNNING, > >>> + VMST_SHUTDOWN, > >>> + VMST_WATCHDOG, > >>> + VMST_MAX, > >>> +} VMStatus; > >> > >> How are these related to the VMSTOP_*? > >> > >> Why do we need a separate enumeration? > > Luiz, what about this part? For me, this was the most important > question. We already have VMSTOP_*, and every caller of vm_stop() should > change the status (most of the vm_status_set() calls come immediately > after a vm_stop() call), so it would appear logical that vm_stop(), > which already gets a reason, sets the status. > > Probably we would need a few additional reasons for vm_stop(), but > keeping two separate status values for almost the same thing looks > suspicious. Well, that's how I was doing it but I had a conversation with Anthony and he was against using vm_stop() for this, because (IIRC) they are different things.
On Tue, 12 Jul 2011 12:12:31 -0300 Luiz Capitulino <lcapitulino@redhat.com> wrote: > On Tue, 12 Jul 2011 16:51:03 +0200 > Kevin Wolf <kwolf@redhat.com> wrote: > > > Am 12.07.2011 16:25, schrieb Luiz Capitulino: > > > On Tue, 12 Jul 2011 09:28:05 +0200 > > > Markus Armbruster <armbru@redhat.com> wrote: > > > > > >> Luiz Capitulino <lcapitulino@redhat.com> writes: > > >> > > >>> We need to track the VM status so that QMP can report it to clients. > > >>> > > >>> This commit adds the VMStatus type and related functions. The > > >>> vm_status_set() function is used to keep track of the current > > >>> VM status. > > >>> > > >>> The current statuses are: > > >> > > >> Nitpicking about names, bear with me. > > >> > > >>> - debug: guest is running under gdb > > >>> - inmigrate: guest is paused waiting for an incoming migration > > >> > > >> incoming-migration? > > >> > > >>> - postmigrate: guest is paused following a successful migration > > >> > > >> post-migrate? > > >> > > >>> - internal-error: Fatal internal error that prevents further guest > > >>> execution > > >>> - load-state-error: guest is paused following a failed 'loadvm' > > >> > > >> Less than obvious. If you like concrete, name it loadvm-failed. If you > > >> like abstract, name it restore-vm-failed. > > > > > > Ok for your suggestions above. > > > > > >> > > >>> - io-error: the last IOP has failed and the device is configured > > >>> to pause on I/O errors > > >>> - watchdog-error: the watchdog action is configured to pause and > > >>> has been triggered > > >> > > >> Sounds like the watchdog suffered an error. watchdog-fired? > > > > > > Maybe watchdog-paused. > > > > > >> > > >>> - paused: guest has been paused via the 'stop' command > > >> > > >> stop-command? > > > > > > I prefer 'paused', it communicates better the state we're in. > > > > > >> > > >>> - prelaunch: QEMU was started with -S and guest has not started > > >> > > >> unstarted? > > > > > > Looks the same to me. > > > > > >> > > >>> - running: guest is actively running > > >>> - shutdown: guest is shut down (and -no-shutdown is in use) > > >>> > > >>> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> > > >>> --- > > >>> gdbstub.c | 4 ++++ > > >>> hw/ide/core.c | 1 + > > >>> hw/scsi-disk.c | 1 + > > >>> hw/virtio-blk.c | 1 + > > >>> hw/watchdog.c | 1 + > > >>> kvm-all.c | 1 + > > >>> migration.c | 3 +++ > > >>> monitor.c | 5 ++++- > > >>> sysemu.h | 19 +++++++++++++++++++ > > >>> vl.c | 37 +++++++++++++++++++++++++++++++++++++ > > >>> 10 files changed, 72 insertions(+), 1 deletions(-) > > >>> > > >>> diff --git a/gdbstub.c b/gdbstub.c > > >>> index c085a5a..61b700a 100644 > > >>> --- a/gdbstub.c > > >>> +++ b/gdbstub.c > > >>> @@ -2358,6 +2358,7 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...) > > >>> s->state = RS_SYSCALL; > > >>> #ifndef CONFIG_USER_ONLY > > >>> vm_stop(VMSTOP_DEBUG); > > >>> + vm_status_set(VMST_DEBUG); > > >>> #endif > > >>> s->state = RS_IDLE; > > >>> va_start(va, fmt); > > >>> @@ -2432,6 +2433,7 @@ static void gdb_read_byte(GDBState *s, int ch) > > >>> /* when the CPU is running, we cannot do anything except stop > > >>> it when receiving a char */ > > >>> vm_stop(VMSTOP_USER); > > >>> + vm_status_set(VMST_DEBUG); > > >>> } else > > >>> #endif > > >>> { > > >>> @@ -2694,6 +2696,7 @@ static void gdb_chr_event(void *opaque, int event) > > >>> switch (event) { > > >>> case CHR_EVENT_OPENED: > > >>> vm_stop(VMSTOP_USER); > > >>> + vm_status_set(VMST_DEBUG); > > >>> gdb_has_xml = 0; > > >>> break; > > >>> default: > > >> > > >> Previous hunk has VMST_DEBUG with VMST_DEBUG. Odd. > > >> > > >>> @@ -2735,6 +2738,7 @@ static void gdb_sigterm_handler(int signal) > > >>> { > > >>> if (vm_running) { > > >>> vm_stop(VMSTOP_USER); > > >>> + vm_status_set(VMST_DEBUG); > > >>> } > > >>> } > > >>> #endif > > >>> diff --git a/hw/ide/core.c b/hw/ide/core.c > > >>> index ca17a43..bf9df41 100644 > > >>> --- a/hw/ide/core.c > > >>> +++ b/hw/ide/core.c > > >>> @@ -523,6 +523,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int op) > > >>> s->bus->error_status = op; > > >>> bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); > > >>> vm_stop(VMSTOP_DISKFULL); > > >>> + vm_status_set(VMST_IOERROR); > > >>> } else { > > >>> if (op & BM_STATUS_DMA_RETRY) { > > >>> dma_buf_commit(s, 0); > > >>> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c > > >>> index a8c7372..66037fd 100644 > > >>> --- a/hw/scsi-disk.c > > >>> +++ b/hw/scsi-disk.c > > >>> @@ -216,6 +216,7 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type) > > >>> > > >>> bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); > > >>> vm_stop(VMSTOP_DISKFULL); > > >>> + vm_status_set(VMST_IOERROR); > > >>> } else { > > >>> if (type == SCSI_REQ_STATUS_RETRY_READ) { > > >>> scsi_req_data(&r->req, 0); > > >>> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c > > >>> index 91e0394..bf70200 100644 > > >>> --- a/hw/virtio-blk.c > > >>> +++ b/hw/virtio-blk.c > > >>> @@ -79,6 +79,7 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, > > >>> s->rq = req; > > >>> bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); > > >>> vm_stop(VMSTOP_DISKFULL); > > >>> + vm_status_set(VMST_IOERROR); > > >>> } else { > > >>> virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR); > > >>> bdrv_mon_event(s->bs, BDRV_ACTION_REPORT, is_read); > > >>> diff --git a/hw/watchdog.c b/hw/watchdog.c > > >>> index 1c900a1..d130cbb 100644 > > >>> --- a/hw/watchdog.c > > >>> +++ b/hw/watchdog.c > > >>> @@ -133,6 +133,7 @@ void watchdog_perform_action(void) > > >>> case WDT_PAUSE: /* same as 'stop' command in monitor */ > > >>> watchdog_mon_event("pause"); > > >>> vm_stop(VMSTOP_WATCHDOG); > > >>> + vm_status_set(VMST_WATCHDOG); > > >>> break; > > >>> > > >>> case WDT_DEBUG: > > >>> diff --git a/kvm-all.c b/kvm-all.c > > >>> index cbc2532..aee9e0a 100644 > > >>> --- a/kvm-all.c > > >>> +++ b/kvm-all.c > > >>> @@ -1015,6 +1015,7 @@ int kvm_cpu_exec(CPUState *env) > > >>> if (ret < 0) { > > >>> cpu_dump_state(env, stderr, fprintf, CPU_DUMP_CODE); > > >>> vm_stop(VMSTOP_PANIC); > > >>> + vm_status_set(VMST_INTERROR); > > >>> } > > >>> > > >>> env->exit_request = 0; > > >>> diff --git a/migration.c b/migration.c > > >>> index af3a1f2..674792f 100644 > > >>> --- a/migration.c > > >>> +++ b/migration.c > > >>> @@ -394,6 +394,9 @@ void migrate_fd_put_ready(void *opaque) > > >>> } > > >>> state = MIG_STATE_ERROR; > > >>> } > > >>> + if (state == MIG_STATE_COMPLETED) { > > >>> + vm_status_set(VMST_POSTMIGRATE); > > >>> + } > > >>> s->state = state; > > >>> notifier_list_notify(&migration_state_notifiers); > > >>> } > > >>> diff --git a/monitor.c b/monitor.c > > >>> index 67ceb46..1cb3191 100644 > > >>> --- a/monitor.c > > >>> +++ b/monitor.c > > >>> @@ -1258,6 +1258,7 @@ static void do_singlestep(Monitor *mon, const QDict *qdict) > > >>> static int do_stop(Monitor *mon, const QDict *qdict, QObject **ret_data) > > >>> { > > >>> vm_stop(VMSTOP_USER); > > >>> + vm_status_set(VMST_PAUSED); > > >>> return 0; > > >>> } > > >>> > > >>> @@ -2782,7 +2783,9 @@ static void do_loadvm(Monitor *mon, const QDict *qdict) > > >>> > > >>> vm_stop(VMSTOP_LOADVM); > > >>> > > >>> - if (load_vmstate(name) == 0 && saved_vm_running) { > > >>> + if (load_vmstate(name) < 0) { > > >>> + vm_status_set(VMST_LOADERROR); > > >>> + } else if (saved_vm_running) { > > >>> vm_start(); > > >>> } > > >>> } > > >>> diff --git a/sysemu.h b/sysemu.h > > >>> index d3013f5..7308ac5 100644 > > >>> --- a/sysemu.h > > >>> +++ b/sysemu.h > > >>> @@ -77,6 +77,25 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f); > > >>> void qemu_savevm_state_cancel(Monitor *mon, QEMUFile *f); > > >>> int qemu_loadvm_state(QEMUFile *f); > > >>> > > >>> +typedef enum { > > >>> + VMST_NOSTATUS, > > >>> + VMST_DEBUG, > > >>> + VMST_INMIGRATE, > > >>> + VMST_POSTMIGRATE, > > >>> + VMST_INTERROR, > > >>> + VMST_IOERROR, > > >>> + VMST_LOADERROR, > > >>> + VMST_PAUSED, > > >>> + VMST_PRELAUNCH, > > >>> + VMST_RUNNING, > > >>> + VMST_SHUTDOWN, > > >>> + VMST_WATCHDOG, > > >>> + VMST_MAX, > > >>> +} VMStatus; > > >> > > >> How are these related to the VMSTOP_*? > > >> > > >> Why do we need a separate enumeration? > > > > Luiz, what about this part? For me, this was the most important > > question. We already have VMSTOP_*, and every caller of vm_stop() should > > change the status (most of the vm_status_set() calls come immediately > > after a vm_stop() call), so it would appear logical that vm_stop(), > > which already gets a reason, sets the status. > > > > Probably we would need a few additional reasons for vm_stop(), but > > keeping two separate status values for almost the same thing looks > > suspicious. > > Well, that's how I was doing it but I had a conversation with Anthony > and he was against using vm_stop() for this, because (IIRC) they are > different things. Let me elaborate the way I see it. The VMSTOP_* macros are stop reasons. They say why the VM stopped, not what the VM status is. For example, "running" is a valid vm state, but it doesn't make sense as a "stop reason". It's possible to change vm_stop() (and vm_start()) to set the state instead, but it's a more profound surgery than it may seem at first. For example, we have things like vm stop notifiers, which would have to be changed to get the vm status instead of a stop reason. I started doing it that way and have to admit that having the vm state as a different thing made the implementation simpler.
Am 12.07.2011 18:03, schrieb Luiz Capitulino: > On Tue, 12 Jul 2011 12:12:31 -0300 > Luiz Capitulino <lcapitulino@redhat.com> wrote: > >> On Tue, 12 Jul 2011 16:51:03 +0200 >> Kevin Wolf <kwolf@redhat.com> wrote: >> >>> Am 12.07.2011 16:25, schrieb Luiz Capitulino: >>>> On Tue, 12 Jul 2011 09:28:05 +0200 >>>> Markus Armbruster <armbru@redhat.com> wrote: >>>> >>>>> Luiz Capitulino <lcapitulino@redhat.com> writes: >>>>> >>>>>> We need to track the VM status so that QMP can report it to clients. >>>>>> >>>>>> This commit adds the VMStatus type and related functions. The >>>>>> vm_status_set() function is used to keep track of the current >>>>>> VM status. >>>>>> >>>>>> The current statuses are: >>>>> >>>>> Nitpicking about names, bear with me. >>>>> >>>>>> - debug: guest is running under gdb >>>>>> - inmigrate: guest is paused waiting for an incoming migration >>>>> >>>>> incoming-migration? >>>>> >>>>>> - postmigrate: guest is paused following a successful migration >>>>> >>>>> post-migrate? >>>>> >>>>>> - internal-error: Fatal internal error that prevents further guest >>>>>> execution >>>>>> - load-state-error: guest is paused following a failed 'loadvm' >>>>> >>>>> Less than obvious. If you like concrete, name it loadvm-failed. If you >>>>> like abstract, name it restore-vm-failed. >>>> >>>> Ok for your suggestions above. >>>> >>>>> >>>>>> - io-error: the last IOP has failed and the device is configured >>>>>> to pause on I/O errors >>>>>> - watchdog-error: the watchdog action is configured to pause and >>>>>> has been triggered >>>>> >>>>> Sounds like the watchdog suffered an error. watchdog-fired? >>>> >>>> Maybe watchdog-paused. >>>> >>>>> >>>>>> - paused: guest has been paused via the 'stop' command >>>>> >>>>> stop-command? >>>> >>>> I prefer 'paused', it communicates better the state we're in. >>>> >>>>> >>>>>> - prelaunch: QEMU was started with -S and guest has not started >>>>> >>>>> unstarted? >>>> >>>> Looks the same to me. >>>> >>>>> >>>>>> - running: guest is actively running >>>>>> - shutdown: guest is shut down (and -no-shutdown is in use) >>>>>> >>>>>> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> >>>>>> --- >>>>>> gdbstub.c | 4 ++++ >>>>>> hw/ide/core.c | 1 + >>>>>> hw/scsi-disk.c | 1 + >>>>>> hw/virtio-blk.c | 1 + >>>>>> hw/watchdog.c | 1 + >>>>>> kvm-all.c | 1 + >>>>>> migration.c | 3 +++ >>>>>> monitor.c | 5 ++++- >>>>>> sysemu.h | 19 +++++++++++++++++++ >>>>>> vl.c | 37 +++++++++++++++++++++++++++++++++++++ >>>>>> 10 files changed, 72 insertions(+), 1 deletions(-) >>>>>> >>>>>> diff --git a/gdbstub.c b/gdbstub.c >>>>>> index c085a5a..61b700a 100644 >>>>>> --- a/gdbstub.c >>>>>> +++ b/gdbstub.c >>>>>> @@ -2358,6 +2358,7 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...) >>>>>> s->state = RS_SYSCALL; >>>>>> #ifndef CONFIG_USER_ONLY >>>>>> vm_stop(VMSTOP_DEBUG); >>>>>> + vm_status_set(VMST_DEBUG); >>>>>> #endif >>>>>> s->state = RS_IDLE; >>>>>> va_start(va, fmt); >>>>>> @@ -2432,6 +2433,7 @@ static void gdb_read_byte(GDBState *s, int ch) >>>>>> /* when the CPU is running, we cannot do anything except stop >>>>>> it when receiving a char */ >>>>>> vm_stop(VMSTOP_USER); >>>>>> + vm_status_set(VMST_DEBUG); >>>>>> } else >>>>>> #endif >>>>>> { >>>>>> @@ -2694,6 +2696,7 @@ static void gdb_chr_event(void *opaque, int event) >>>>>> switch (event) { >>>>>> case CHR_EVENT_OPENED: >>>>>> vm_stop(VMSTOP_USER); >>>>>> + vm_status_set(VMST_DEBUG); >>>>>> gdb_has_xml = 0; >>>>>> break; >>>>>> default: >>>>> >>>>> Previous hunk has VMST_DEBUG with VMST_DEBUG. Odd. >>>>> >>>>>> @@ -2735,6 +2738,7 @@ static void gdb_sigterm_handler(int signal) >>>>>> { >>>>>> if (vm_running) { >>>>>> vm_stop(VMSTOP_USER); >>>>>> + vm_status_set(VMST_DEBUG); >>>>>> } >>>>>> } >>>>>> #endif >>>>>> diff --git a/hw/ide/core.c b/hw/ide/core.c >>>>>> index ca17a43..bf9df41 100644 >>>>>> --- a/hw/ide/core.c >>>>>> +++ b/hw/ide/core.c >>>>>> @@ -523,6 +523,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int op) >>>>>> s->bus->error_status = op; >>>>>> bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); >>>>>> vm_stop(VMSTOP_DISKFULL); >>>>>> + vm_status_set(VMST_IOERROR); >>>>>> } else { >>>>>> if (op & BM_STATUS_DMA_RETRY) { >>>>>> dma_buf_commit(s, 0); >>>>>> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c >>>>>> index a8c7372..66037fd 100644 >>>>>> --- a/hw/scsi-disk.c >>>>>> +++ b/hw/scsi-disk.c >>>>>> @@ -216,6 +216,7 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type) >>>>>> >>>>>> bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); >>>>>> vm_stop(VMSTOP_DISKFULL); >>>>>> + vm_status_set(VMST_IOERROR); >>>>>> } else { >>>>>> if (type == SCSI_REQ_STATUS_RETRY_READ) { >>>>>> scsi_req_data(&r->req, 0); >>>>>> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c >>>>>> index 91e0394..bf70200 100644 >>>>>> --- a/hw/virtio-blk.c >>>>>> +++ b/hw/virtio-blk.c >>>>>> @@ -79,6 +79,7 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, >>>>>> s->rq = req; >>>>>> bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); >>>>>> vm_stop(VMSTOP_DISKFULL); >>>>>> + vm_status_set(VMST_IOERROR); >>>>>> } else { >>>>>> virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR); >>>>>> bdrv_mon_event(s->bs, BDRV_ACTION_REPORT, is_read); >>>>>> diff --git a/hw/watchdog.c b/hw/watchdog.c >>>>>> index 1c900a1..d130cbb 100644 >>>>>> --- a/hw/watchdog.c >>>>>> +++ b/hw/watchdog.c >>>>>> @@ -133,6 +133,7 @@ void watchdog_perform_action(void) >>>>>> case WDT_PAUSE: /* same as 'stop' command in monitor */ >>>>>> watchdog_mon_event("pause"); >>>>>> vm_stop(VMSTOP_WATCHDOG); >>>>>> + vm_status_set(VMST_WATCHDOG); >>>>>> break; >>>>>> >>>>>> case WDT_DEBUG: >>>>>> diff --git a/kvm-all.c b/kvm-all.c >>>>>> index cbc2532..aee9e0a 100644 >>>>>> --- a/kvm-all.c >>>>>> +++ b/kvm-all.c >>>>>> @@ -1015,6 +1015,7 @@ int kvm_cpu_exec(CPUState *env) >>>>>> if (ret < 0) { >>>>>> cpu_dump_state(env, stderr, fprintf, CPU_DUMP_CODE); >>>>>> vm_stop(VMSTOP_PANIC); >>>>>> + vm_status_set(VMST_INTERROR); >>>>>> } >>>>>> >>>>>> env->exit_request = 0; >>>>>> diff --git a/migration.c b/migration.c >>>>>> index af3a1f2..674792f 100644 >>>>>> --- a/migration.c >>>>>> +++ b/migration.c >>>>>> @@ -394,6 +394,9 @@ void migrate_fd_put_ready(void *opaque) >>>>>> } >>>>>> state = MIG_STATE_ERROR; >>>>>> } >>>>>> + if (state == MIG_STATE_COMPLETED) { >>>>>> + vm_status_set(VMST_POSTMIGRATE); >>>>>> + } >>>>>> s->state = state; >>>>>> notifier_list_notify(&migration_state_notifiers); >>>>>> } >>>>>> diff --git a/monitor.c b/monitor.c >>>>>> index 67ceb46..1cb3191 100644 >>>>>> --- a/monitor.c >>>>>> +++ b/monitor.c >>>>>> @@ -1258,6 +1258,7 @@ static void do_singlestep(Monitor *mon, const QDict *qdict) >>>>>> static int do_stop(Monitor *mon, const QDict *qdict, QObject **ret_data) >>>>>> { >>>>>> vm_stop(VMSTOP_USER); >>>>>> + vm_status_set(VMST_PAUSED); >>>>>> return 0; >>>>>> } >>>>>> >>>>>> @@ -2782,7 +2783,9 @@ static void do_loadvm(Monitor *mon, const QDict *qdict) >>>>>> >>>>>> vm_stop(VMSTOP_LOADVM); >>>>>> >>>>>> - if (load_vmstate(name) == 0 && saved_vm_running) { >>>>>> + if (load_vmstate(name) < 0) { >>>>>> + vm_status_set(VMST_LOADERROR); >>>>>> + } else if (saved_vm_running) { >>>>>> vm_start(); >>>>>> } >>>>>> } >>>>>> diff --git a/sysemu.h b/sysemu.h >>>>>> index d3013f5..7308ac5 100644 >>>>>> --- a/sysemu.h >>>>>> +++ b/sysemu.h >>>>>> @@ -77,6 +77,25 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f); >>>>>> void qemu_savevm_state_cancel(Monitor *mon, QEMUFile *f); >>>>>> int qemu_loadvm_state(QEMUFile *f); >>>>>> >>>>>> +typedef enum { >>>>>> + VMST_NOSTATUS, >>>>>> + VMST_DEBUG, >>>>>> + VMST_INMIGRATE, >>>>>> + VMST_POSTMIGRATE, >>>>>> + VMST_INTERROR, >>>>>> + VMST_IOERROR, >>>>>> + VMST_LOADERROR, >>>>>> + VMST_PAUSED, >>>>>> + VMST_PRELAUNCH, >>>>>> + VMST_RUNNING, >>>>>> + VMST_SHUTDOWN, >>>>>> + VMST_WATCHDOG, >>>>>> + VMST_MAX, >>>>>> +} VMStatus; >>>>> >>>>> How are these related to the VMSTOP_*? >>>>> >>>>> Why do we need a separate enumeration? >>> >>> Luiz, what about this part? For me, this was the most important >>> question. We already have VMSTOP_*, and every caller of vm_stop() should >>> change the status (most of the vm_status_set() calls come immediately >>> after a vm_stop() call), so it would appear logical that vm_stop(), >>> which already gets a reason, sets the status. >>> >>> Probably we would need a few additional reasons for vm_stop(), but >>> keeping two separate status values for almost the same thing looks >>> suspicious. >> >> Well, that's how I was doing it but I had a conversation with Anthony >> and he was against using vm_stop() for this, because (IIRC) they are >> different things. > > Let me elaborate the way I see it. The VMSTOP_* macros are stop reasons. > They say why the VM stopped, not what the VM status is. For example, "running" > is a valid vm state, but it doesn't make sense as a "stop reason". > > It's possible to change vm_stop() (and vm_start()) to set the state instead, > but it's a more profound surgery than it may seem at first. For example, we > have things like vm stop notifiers, which would have to be changed to get the > vm status instead of a stop reason. > > I started doing it that way and have to admit that having the vm state as > a different thing made the implementation simpler. Maybe we can have vm_stop() take two arguments at least, so that you can't forget updating the status when you call vm_stop()? Or are there cases of vm_stop() that shouldn't change the status? Kevin
On Tue, 12 Jul 2011 18:16:26 +0200 Kevin Wolf <kwolf@redhat.com> wrote: > Am 12.07.2011 18:03, schrieb Luiz Capitulino: > > On Tue, 12 Jul 2011 12:12:31 -0300 > > Luiz Capitulino <lcapitulino@redhat.com> wrote: > > > >> On Tue, 12 Jul 2011 16:51:03 +0200 > >> Kevin Wolf <kwolf@redhat.com> wrote: > >> > >>> Am 12.07.2011 16:25, schrieb Luiz Capitulino: > >>>> On Tue, 12 Jul 2011 09:28:05 +0200 > >>>> Markus Armbruster <armbru@redhat.com> wrote: > >>>> > >>>>> Luiz Capitulino <lcapitulino@redhat.com> writes: > >>>>> > >>>>>> We need to track the VM status so that QMP can report it to clients. > >>>>>> > >>>>>> This commit adds the VMStatus type and related functions. The > >>>>>> vm_status_set() function is used to keep track of the current > >>>>>> VM status. > >>>>>> > >>>>>> The current statuses are: > >>>>> > >>>>> Nitpicking about names, bear with me. > >>>>> > >>>>>> - debug: guest is running under gdb > >>>>>> - inmigrate: guest is paused waiting for an incoming migration > >>>>> > >>>>> incoming-migration? > >>>>> > >>>>>> - postmigrate: guest is paused following a successful migration > >>>>> > >>>>> post-migrate? > >>>>> > >>>>>> - internal-error: Fatal internal error that prevents further guest > >>>>>> execution > >>>>>> - load-state-error: guest is paused following a failed 'loadvm' > >>>>> > >>>>> Less than obvious. If you like concrete, name it loadvm-failed. If you > >>>>> like abstract, name it restore-vm-failed. > >>>> > >>>> Ok for your suggestions above. > >>>> > >>>>> > >>>>>> - io-error: the last IOP has failed and the device is configured > >>>>>> to pause on I/O errors > >>>>>> - watchdog-error: the watchdog action is configured to pause and > >>>>>> has been triggered > >>>>> > >>>>> Sounds like the watchdog suffered an error. watchdog-fired? > >>>> > >>>> Maybe watchdog-paused. > >>>> > >>>>> > >>>>>> - paused: guest has been paused via the 'stop' command > >>>>> > >>>>> stop-command? > >>>> > >>>> I prefer 'paused', it communicates better the state we're in. > >>>> > >>>>> > >>>>>> - prelaunch: QEMU was started with -S and guest has not started > >>>>> > >>>>> unstarted? > >>>> > >>>> Looks the same to me. > >>>> > >>>>> > >>>>>> - running: guest is actively running > >>>>>> - shutdown: guest is shut down (and -no-shutdown is in use) > >>>>>> > >>>>>> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> > >>>>>> --- > >>>>>> gdbstub.c | 4 ++++ > >>>>>> hw/ide/core.c | 1 + > >>>>>> hw/scsi-disk.c | 1 + > >>>>>> hw/virtio-blk.c | 1 + > >>>>>> hw/watchdog.c | 1 + > >>>>>> kvm-all.c | 1 + > >>>>>> migration.c | 3 +++ > >>>>>> monitor.c | 5 ++++- > >>>>>> sysemu.h | 19 +++++++++++++++++++ > >>>>>> vl.c | 37 +++++++++++++++++++++++++++++++++++++ > >>>>>> 10 files changed, 72 insertions(+), 1 deletions(-) > >>>>>> > >>>>>> diff --git a/gdbstub.c b/gdbstub.c > >>>>>> index c085a5a..61b700a 100644 > >>>>>> --- a/gdbstub.c > >>>>>> +++ b/gdbstub.c > >>>>>> @@ -2358,6 +2358,7 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...) > >>>>>> s->state = RS_SYSCALL; > >>>>>> #ifndef CONFIG_USER_ONLY > >>>>>> vm_stop(VMSTOP_DEBUG); > >>>>>> + vm_status_set(VMST_DEBUG); > >>>>>> #endif > >>>>>> s->state = RS_IDLE; > >>>>>> va_start(va, fmt); > >>>>>> @@ -2432,6 +2433,7 @@ static void gdb_read_byte(GDBState *s, int ch) > >>>>>> /* when the CPU is running, we cannot do anything except stop > >>>>>> it when receiving a char */ > >>>>>> vm_stop(VMSTOP_USER); > >>>>>> + vm_status_set(VMST_DEBUG); > >>>>>> } else > >>>>>> #endif > >>>>>> { > >>>>>> @@ -2694,6 +2696,7 @@ static void gdb_chr_event(void *opaque, int event) > >>>>>> switch (event) { > >>>>>> case CHR_EVENT_OPENED: > >>>>>> vm_stop(VMSTOP_USER); > >>>>>> + vm_status_set(VMST_DEBUG); > >>>>>> gdb_has_xml = 0; > >>>>>> break; > >>>>>> default: > >>>>> > >>>>> Previous hunk has VMST_DEBUG with VMST_DEBUG. Odd. > >>>>> > >>>>>> @@ -2735,6 +2738,7 @@ static void gdb_sigterm_handler(int signal) > >>>>>> { > >>>>>> if (vm_running) { > >>>>>> vm_stop(VMSTOP_USER); > >>>>>> + vm_status_set(VMST_DEBUG); > >>>>>> } > >>>>>> } > >>>>>> #endif > >>>>>> diff --git a/hw/ide/core.c b/hw/ide/core.c > >>>>>> index ca17a43..bf9df41 100644 > >>>>>> --- a/hw/ide/core.c > >>>>>> +++ b/hw/ide/core.c > >>>>>> @@ -523,6 +523,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int op) > >>>>>> s->bus->error_status = op; > >>>>>> bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); > >>>>>> vm_stop(VMSTOP_DISKFULL); > >>>>>> + vm_status_set(VMST_IOERROR); > >>>>>> } else { > >>>>>> if (op & BM_STATUS_DMA_RETRY) { > >>>>>> dma_buf_commit(s, 0); > >>>>>> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c > >>>>>> index a8c7372..66037fd 100644 > >>>>>> --- a/hw/scsi-disk.c > >>>>>> +++ b/hw/scsi-disk.c > >>>>>> @@ -216,6 +216,7 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type) > >>>>>> > >>>>>> bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); > >>>>>> vm_stop(VMSTOP_DISKFULL); > >>>>>> + vm_status_set(VMST_IOERROR); > >>>>>> } else { > >>>>>> if (type == SCSI_REQ_STATUS_RETRY_READ) { > >>>>>> scsi_req_data(&r->req, 0); > >>>>>> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c > >>>>>> index 91e0394..bf70200 100644 > >>>>>> --- a/hw/virtio-blk.c > >>>>>> +++ b/hw/virtio-blk.c > >>>>>> @@ -79,6 +79,7 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, > >>>>>> s->rq = req; > >>>>>> bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); > >>>>>> vm_stop(VMSTOP_DISKFULL); > >>>>>> + vm_status_set(VMST_IOERROR); > >>>>>> } else { > >>>>>> virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR); > >>>>>> bdrv_mon_event(s->bs, BDRV_ACTION_REPORT, is_read); > >>>>>> diff --git a/hw/watchdog.c b/hw/watchdog.c > >>>>>> index 1c900a1..d130cbb 100644 > >>>>>> --- a/hw/watchdog.c > >>>>>> +++ b/hw/watchdog.c > >>>>>> @@ -133,6 +133,7 @@ void watchdog_perform_action(void) > >>>>>> case WDT_PAUSE: /* same as 'stop' command in monitor */ > >>>>>> watchdog_mon_event("pause"); > >>>>>> vm_stop(VMSTOP_WATCHDOG); > >>>>>> + vm_status_set(VMST_WATCHDOG); > >>>>>> break; > >>>>>> > >>>>>> case WDT_DEBUG: > >>>>>> diff --git a/kvm-all.c b/kvm-all.c > >>>>>> index cbc2532..aee9e0a 100644 > >>>>>> --- a/kvm-all.c > >>>>>> +++ b/kvm-all.c > >>>>>> @@ -1015,6 +1015,7 @@ int kvm_cpu_exec(CPUState *env) > >>>>>> if (ret < 0) { > >>>>>> cpu_dump_state(env, stderr, fprintf, CPU_DUMP_CODE); > >>>>>> vm_stop(VMSTOP_PANIC); > >>>>>> + vm_status_set(VMST_INTERROR); > >>>>>> } > >>>>>> > >>>>>> env->exit_request = 0; > >>>>>> diff --git a/migration.c b/migration.c > >>>>>> index af3a1f2..674792f 100644 > >>>>>> --- a/migration.c > >>>>>> +++ b/migration.c > >>>>>> @@ -394,6 +394,9 @@ void migrate_fd_put_ready(void *opaque) > >>>>>> } > >>>>>> state = MIG_STATE_ERROR; > >>>>>> } > >>>>>> + if (state == MIG_STATE_COMPLETED) { > >>>>>> + vm_status_set(VMST_POSTMIGRATE); > >>>>>> + } > >>>>>> s->state = state; > >>>>>> notifier_list_notify(&migration_state_notifiers); > >>>>>> } > >>>>>> diff --git a/monitor.c b/monitor.c > >>>>>> index 67ceb46..1cb3191 100644 > >>>>>> --- a/monitor.c > >>>>>> +++ b/monitor.c > >>>>>> @@ -1258,6 +1258,7 @@ static void do_singlestep(Monitor *mon, const QDict *qdict) > >>>>>> static int do_stop(Monitor *mon, const QDict *qdict, QObject **ret_data) > >>>>>> { > >>>>>> vm_stop(VMSTOP_USER); > >>>>>> + vm_status_set(VMST_PAUSED); > >>>>>> return 0; > >>>>>> } > >>>>>> > >>>>>> @@ -2782,7 +2783,9 @@ static void do_loadvm(Monitor *mon, const QDict *qdict) > >>>>>> > >>>>>> vm_stop(VMSTOP_LOADVM); > >>>>>> > >>>>>> - if (load_vmstate(name) == 0 && saved_vm_running) { > >>>>>> + if (load_vmstate(name) < 0) { > >>>>>> + vm_status_set(VMST_LOADERROR); > >>>>>> + } else if (saved_vm_running) { > >>>>>> vm_start(); > >>>>>> } > >>>>>> } > >>>>>> diff --git a/sysemu.h b/sysemu.h > >>>>>> index d3013f5..7308ac5 100644 > >>>>>> --- a/sysemu.h > >>>>>> +++ b/sysemu.h > >>>>>> @@ -77,6 +77,25 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f); > >>>>>> void qemu_savevm_state_cancel(Monitor *mon, QEMUFile *f); > >>>>>> int qemu_loadvm_state(QEMUFile *f); > >>>>>> > >>>>>> +typedef enum { > >>>>>> + VMST_NOSTATUS, > >>>>>> + VMST_DEBUG, > >>>>>> + VMST_INMIGRATE, > >>>>>> + VMST_POSTMIGRATE, > >>>>>> + VMST_INTERROR, > >>>>>> + VMST_IOERROR, > >>>>>> + VMST_LOADERROR, > >>>>>> + VMST_PAUSED, > >>>>>> + VMST_PRELAUNCH, > >>>>>> + VMST_RUNNING, > >>>>>> + VMST_SHUTDOWN, > >>>>>> + VMST_WATCHDOG, > >>>>>> + VMST_MAX, > >>>>>> +} VMStatus; > >>>>> > >>>>> How are these related to the VMSTOP_*? > >>>>> > >>>>> Why do we need a separate enumeration? > >>> > >>> Luiz, what about this part? For me, this was the most important > >>> question. We already have VMSTOP_*, and every caller of vm_stop() should > >>> change the status (most of the vm_status_set() calls come immediately > >>> after a vm_stop() call), so it would appear logical that vm_stop(), > >>> which already gets a reason, sets the status. > >>> > >>> Probably we would need a few additional reasons for vm_stop(), but > >>> keeping two separate status values for almost the same thing looks > >>> suspicious. > >> > >> Well, that's how I was doing it but I had a conversation with Anthony > >> and he was against using vm_stop() for this, because (IIRC) they are > >> different things. > > > > Let me elaborate the way I see it. The VMSTOP_* macros are stop reasons. > > They say why the VM stopped, not what the VM status is. For example, "running" > > is a valid vm state, but it doesn't make sense as a "stop reason". > > > > It's possible to change vm_stop() (and vm_start()) to set the state instead, > > but it's a more profound surgery than it may seem at first. For example, we > > have things like vm stop notifiers, which would have to be changed to get the > > vm status instead of a stop reason. > > > > I started doing it that way and have to admit that having the vm state as > > a different thing made the implementation simpler. > > Maybe we can have vm_stop() take two arguments at least, so that you > can't forget updating the status when you call vm_stop()? Or are there > cases of vm_stop() that shouldn't change the status? Today we don't set when saving the vm_state, not sure we should.
diff --git a/gdbstub.c b/gdbstub.c index c085a5a..61b700a 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -2358,6 +2358,7 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...) s->state = RS_SYSCALL; #ifndef CONFIG_USER_ONLY vm_stop(VMSTOP_DEBUG); + vm_status_set(VMST_DEBUG); #endif s->state = RS_IDLE; va_start(va, fmt); @@ -2432,6 +2433,7 @@ static void gdb_read_byte(GDBState *s, int ch) /* when the CPU is running, we cannot do anything except stop it when receiving a char */ vm_stop(VMSTOP_USER); + vm_status_set(VMST_DEBUG); } else #endif { @@ -2694,6 +2696,7 @@ static void gdb_chr_event(void *opaque, int event) switch (event) { case CHR_EVENT_OPENED: vm_stop(VMSTOP_USER); + vm_status_set(VMST_DEBUG); gdb_has_xml = 0; break; default: @@ -2735,6 +2738,7 @@ static void gdb_sigterm_handler(int signal) { if (vm_running) { vm_stop(VMSTOP_USER); + vm_status_set(VMST_DEBUG); } } #endif diff --git a/hw/ide/core.c b/hw/ide/core.c index ca17a43..bf9df41 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -523,6 +523,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int op) s->bus->error_status = op; bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); vm_stop(VMSTOP_DISKFULL); + vm_status_set(VMST_IOERROR); } else { if (op & BM_STATUS_DMA_RETRY) { dma_buf_commit(s, 0); diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index a8c7372..66037fd 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -216,6 +216,7 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type) bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); vm_stop(VMSTOP_DISKFULL); + vm_status_set(VMST_IOERROR); } else { if (type == SCSI_REQ_STATUS_RETRY_READ) { scsi_req_data(&r->req, 0); diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c index 91e0394..bf70200 100644 --- a/hw/virtio-blk.c +++ b/hw/virtio-blk.c @@ -79,6 +79,7 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, s->rq = req; bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); vm_stop(VMSTOP_DISKFULL); + vm_status_set(VMST_IOERROR); } else { virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR); bdrv_mon_event(s->bs, BDRV_ACTION_REPORT, is_read); diff --git a/hw/watchdog.c b/hw/watchdog.c index 1c900a1..d130cbb 100644 --- a/hw/watchdog.c +++ b/hw/watchdog.c @@ -133,6 +133,7 @@ void watchdog_perform_action(void) case WDT_PAUSE: /* same as 'stop' command in monitor */ watchdog_mon_event("pause"); vm_stop(VMSTOP_WATCHDOG); + vm_status_set(VMST_WATCHDOG); break; case WDT_DEBUG: diff --git a/kvm-all.c b/kvm-all.c index cbc2532..aee9e0a 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -1015,6 +1015,7 @@ int kvm_cpu_exec(CPUState *env) if (ret < 0) { cpu_dump_state(env, stderr, fprintf, CPU_DUMP_CODE); vm_stop(VMSTOP_PANIC); + vm_status_set(VMST_INTERROR); } env->exit_request = 0; diff --git a/migration.c b/migration.c index af3a1f2..674792f 100644 --- a/migration.c +++ b/migration.c @@ -394,6 +394,9 @@ void migrate_fd_put_ready(void *opaque) } state = MIG_STATE_ERROR; } + if (state == MIG_STATE_COMPLETED) { + vm_status_set(VMST_POSTMIGRATE); + } s->state = state; notifier_list_notify(&migration_state_notifiers); } diff --git a/monitor.c b/monitor.c index 67ceb46..1cb3191 100644 --- a/monitor.c +++ b/monitor.c @@ -1258,6 +1258,7 @@ static void do_singlestep(Monitor *mon, const QDict *qdict) static int do_stop(Monitor *mon, const QDict *qdict, QObject **ret_data) { vm_stop(VMSTOP_USER); + vm_status_set(VMST_PAUSED); return 0; } @@ -2782,7 +2783,9 @@ static void do_loadvm(Monitor *mon, const QDict *qdict) vm_stop(VMSTOP_LOADVM); - if (load_vmstate(name) == 0 && saved_vm_running) { + if (load_vmstate(name) < 0) { + vm_status_set(VMST_LOADERROR); + } else if (saved_vm_running) { vm_start(); } } diff --git a/sysemu.h b/sysemu.h index d3013f5..7308ac5 100644 --- a/sysemu.h +++ b/sysemu.h @@ -77,6 +77,25 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f); void qemu_savevm_state_cancel(Monitor *mon, QEMUFile *f); int qemu_loadvm_state(QEMUFile *f); +typedef enum { + VMST_NOSTATUS, + VMST_DEBUG, + VMST_INMIGRATE, + VMST_POSTMIGRATE, + VMST_INTERROR, + VMST_IOERROR, + VMST_LOADERROR, + VMST_PAUSED, + VMST_PRELAUNCH, + VMST_RUNNING, + VMST_SHUTDOWN, + VMST_WATCHDOG, + VMST_MAX, +} VMStatus; + +void vm_status_set(VMStatus s); +const char *vm_status_get_name(void); + /* SLIRP */ void do_info_slirp(Monitor *mon); diff --git a/vl.c b/vl.c index fcd7395..fb7208f 100644 --- a/vl.c +++ b/vl.c @@ -309,6 +309,37 @@ static int default_driver_check(QemuOpts *opts, void *opaque) } /***********************************************************/ +/* VMStatus */ + +static const char *const vm_status_name[VMST_MAX] = { + [VMST_DEBUG] = "debug", + [VMST_INMIGRATE] = "inmigrate", + [VMST_POSTMIGRATE] = "postmigrate", + [VMST_INTERROR] = "internal-error", + [VMST_IOERROR] = "io-error", + [VMST_LOADERROR] = "load-state-error", + [VMST_PAUSED] = "paused", + [VMST_PRELAUNCH] = "prelaunch", + [VMST_RUNNING] = "running", + [VMST_SHUTDOWN] = "shutdown", + [VMST_WATCHDOG] = "watchdog-error", +}; + +static VMStatus qemu_vm_status = VMST_NOSTATUS; + +void vm_status_set(VMStatus s) +{ + assert(s > VMST_NOSTATUS && s < VMST_MAX); + qemu_vm_status = s; +} + +const char *vm_status_get_name(void) +{ + assert(qemu_vm_status > VMST_NOSTATUS && qemu_vm_status < VMST_MAX); + return vm_status_name[qemu_vm_status]; +} + +/***********************************************************/ /* real time host monotonic timer */ /***********************************************************/ @@ -1150,6 +1181,7 @@ void vm_start(void) vm_state_notify(1, 0); resume_all_vcpus(); monitor_protocol_event(QEVENT_RESUME, NULL); + vm_status_set(VMST_RUNNING); } } @@ -1392,12 +1424,14 @@ static void main_loop(void) if (qemu_debug_requested()) { vm_stop(VMSTOP_DEBUG); + vm_status_set(VMST_DEBUG); } if (qemu_shutdown_requested()) { qemu_kill_report(); monitor_protocol_event(QEVENT_SHUTDOWN, NULL); if (no_shutdown) { vm_stop(VMSTOP_SHUTDOWN); + vm_status_set(VMST_SHUTDOWN); no_shutdown = 0; } else break; @@ -2476,6 +2510,7 @@ int main(int argc, char **argv, char **envp) break; case QEMU_OPTION_S: autostart = 0; + vm_status_set(VMST_PRELAUNCH); break; case QEMU_OPTION_k: keyboard_layout = optarg; @@ -3307,11 +3342,13 @@ int main(int argc, char **argv, char **envp) qemu_system_reset(VMRESET_SILENT); if (loadvm) { if (load_vmstate(loadvm) < 0) { + vm_status_set(VMST_LOADERROR); autostart = 0; } } if (incoming) { + vm_status_set(VMST_INMIGRATE); int ret = qemu_start_incoming_migration(incoming); if (ret < 0) { fprintf(stderr, "Migration failed. Exit code %s(%d), exiting.\n",
We need to track the VM status so that QMP can report it to clients. This commit adds the VMStatus type and related functions. The vm_status_set() function is used to keep track of the current VM status. The current statuses are: - debug: guest is running under gdb - inmigrate: guest is paused waiting for an incoming migration - postmigrate: guest is paused following a successful migration - internal-error: Fatal internal error that prevents further guest execution - load-state-error: guest is paused following a failed 'loadvm' - io-error: the last IOP has failed and the device is configured to pause on I/O errors - watchdog-error: the watchdog action is configured to pause and has been triggered - paused: guest has been paused via the 'stop' command - prelaunch: QEMU was started with -S and guest has not started - running: guest is actively running - shutdown: guest is shut down (and -no-shutdown is in use) Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> --- gdbstub.c | 4 ++++ hw/ide/core.c | 1 + hw/scsi-disk.c | 1 + hw/virtio-blk.c | 1 + hw/watchdog.c | 1 + kvm-all.c | 1 + migration.c | 3 +++ monitor.c | 5 ++++- sysemu.h | 19 +++++++++++++++++++ vl.c | 37 +++++++++++++++++++++++++++++++++++++ 10 files changed, 72 insertions(+), 1 deletions(-)