Re: [PATCH V1 03/32] savevm: QMP command for cprsave

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Steve Sistare (steven.sistare@oracle.com) wrote:
> To enable live reboot, provide the cprsave QMP command and the VMS_REBOOT
> vmstate-saving operation, which saves the state of the virtual machine in a
> simple file.
> 
> Syntax:
>   {'command':'cprsave', 'data':{'file':'str', 'mode':'str'}}
> 
>   The mode argument must be 'reboot'.  Additional modes will be defined in
>   the future.
> 
> Unlike the savevm command, cprsave supports any type of guest image and
> block device.  cprsave stops the VM so that guest ram and block devices are
> not modified after state is saved.  Guest ram must be mapped to a persistent
> memory file such as /dev/dax0.0.  The ram object vmstate handler and block
> device handler do not apply to VMS_REBOOT, so restrict them to VMS_MIGRATE
> or VMS_SNAPSHOT.  After cprsave completes successfully, qemu exits.
> 
> After issuing cprsave, the caller may update qemu, update the host kernel,
> reboot, start qemu using the same arguments as the original process, and
> issue the cprload command to restore the guest.  cprload is added by
> subsequent patches.
> 
> If the caller suspends the guest instead of stopping the VM, such as by
> issuing guest-suspend-ram to the qemu guest agent, then cprsave and cprload
> support guests with vfio devices.  The guest drivers suspend methods flush
> outstanding requests and re-initialize the devices, and thus there is no
> device state to save and restore.
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> Signed-off-by: Maran Wilson <maran.wilson@oracle.com>

Going back a step; could you.....

> ---
>  include/migration/vmstate.h |  1 +
>  include/sysemu/sysemu.h     |  2 ++
>  migration/block.c           |  1 +
>  migration/ram.c             |  1 +
>  migration/savevm.c          | 59 +++++++++++++++++++++++++++++++++++++++++++++
>  monitor/qmp-cmds.c          |  6 +++++
>  qapi/migration.json         | 14 +++++++++++
>  7 files changed, 84 insertions(+)
> 
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index fa575f9..c58551a 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -161,6 +161,7 @@ typedef enum {
>  typedef enum {
>      VMS_MIGRATE  = (1U << 1),
>      VMS_SNAPSHOT = (1U << 2),
> +    VMS_REBOOT   = (1U << 3),
>      VMS_MODE_ALL = ~0U
>  } VMStateMode;
>  
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 4b6a5c4..6fe86e6 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -24,6 +24,8 @@ extern bool machine_init_done;
>  void qemu_add_machine_init_done_notifier(Notifier *notify);
>  void qemu_remove_machine_init_done_notifier(Notifier *notify);
>  
> +void save_cpr_snapshot(const char *file, const char *mode, Error **errp);
> +
>  extern int autostart;
>  
>  typedef enum {
> diff --git a/migration/block.c b/migration/block.c
> index 737b649..a69accb 100644
> --- a/migration/block.c
> +++ b/migration/block.c
> @@ -1023,6 +1023,7 @@ static SaveVMHandlers savevm_block_handlers = {
>      .load_state = block_load,
>      .save_cleanup = block_migration_cleanup,
>      .is_active = block_is_active,
> +    .mode_mask = VMS_MIGRATE | VMS_SNAPSHOT,
>  };
>  
>  void blk_mig_init(void)
> diff --git a/migration/ram.c b/migration/ram.c
> index 76d4fee..f0d5d9f 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -3795,6 +3795,7 @@ static SaveVMHandlers savevm_ram_handlers = {
>      .load_setup = ram_load_setup,
>      .load_cleanup = ram_load_cleanup,
>      .resume_prepare = ram_resume_prepare,
> +    .mode_mask = VMS_MIGRATE | VMS_SNAPSHOT,
>  };
>  
>  void ram_mig_init(void)
> diff --git a/migration/savevm.c b/migration/savevm.c
> index ce02b6b..ff1a46e 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2680,6 +2680,65 @@ int qemu_load_device_state(QEMUFile *f)
>      return 0;
>  }
>  
> +static QEMUFile *qf_file_open(const char *filename, int flags, int mode,
> +                              Error **errp)
> +{
> +    QIOChannel *ioc;
> +    int fd = qemu_open(filename, flags, mode);
> +
> +    if (fd < 0) {
> +        error_setg_errno(errp, errno, "%s(%s)", __func__, filename);
> +        return NULL;
> +    }
> +
> +    ioc = QIO_CHANNEL(qio_channel_file_new_fd(fd));
> +
> +    if (flags & O_WRONLY) {
> +        return qemu_fopen_channel_output(ioc);
> +    }
> +
> +    return qemu_fopen_channel_input(ioc);
> +}
> +
> +void save_cpr_snapshot(const char *file, const char *mode, Error **errp)
> +{
> +    int ret = 0;
> +    QEMUFile *f;
> +    VMStateMode op;
> +
> +    if (!strcmp(mode, "reboot")) {
> +        op = VMS_REBOOT;
> +    } else {
> +        error_setg(errp, "cprsave: bad mode %s", mode);
> +        return;
> +    }
> +
> +    f = qf_file_open(file, O_CREAT | O_WRONLY | O_TRUNC, 0600, errp);
> +    if (!f) {
> +        return;
> +    }
> +
> +    ret = global_state_store();
> +    if (ret) {
> +        error_setg(errp, "Error saving global state");
> +        qemu_fclose(f);
> +        return;
> +    }
> +
> +    vm_stop(RUN_STATE_SAVE_VM);
> +
> +    ret = qemu_savevm_state(f, op, errp);
> +    if ((ret < 0) && !*errp) {
> +        error_setg(errp, "qemu_savevm_state failed");
> +    }

just call qemu_save_device_state(f) there rather than introducing the
modes?
What you're doing is VERY similar to qmp_xen_save_devices_state and also
COLO's device state saving.

(and also very similar to migration with the x-ignore-shared flag set).

Dave

> +    qemu_fclose(f);
> +
> +    if (op == VMS_REBOOT) {
> +        no_shutdown = 0;
> +        qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
> +    }
> +}
> +
>  int save_snapshot(const char *name, Error **errp)
>  {
>      BlockDriverState *bs, *bs1;
> diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
> index 864cbfa..9ec7b88 100644
> --- a/monitor/qmp-cmds.c
> +++ b/monitor/qmp-cmds.c
> @@ -35,6 +35,7 @@
>  #include "qapi/qapi-commands-machine.h"
>  #include "qapi/qapi-commands-misc.h"
>  #include "qapi/qapi-commands-ui.h"
> +#include "qapi/qapi-commands-migration.h"
>  #include "qapi/qmp/qerror.h"
>  #include "hw/mem/memory-device.h"
>  #include "hw/acpi/acpi_dev_interface.h"
> @@ -161,6 +162,11 @@ void qmp_cont(Error **errp)
>      }
>  }
>  
> +void qmp_cprsave(const char *file, const char *mode, Error **errp)
> +{
> +    save_cpr_snapshot(file, mode, errp);
> +}
> +
>  void qmp_system_wakeup(Error **errp)
>  {
>      if (!qemu_wakeup_suspend_enabled()) {
> diff --git a/qapi/migration.json b/qapi/migration.json
> index d500055..b61df1d 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -1621,3 +1621,17 @@
>  ##
>  { 'event': 'UNPLUG_PRIMARY',
>    'data': { 'device-id': 'str' } }
> +
> +##
> +# @cprsave:
> +#
> +# Create a checkpoint of the virtual machine device state in @file.
> +# Guest RAM and guest block device blocks are not saved.
> +#
> +# @file: name of checkpoint file
> +# @mode: 'reboot' : checkpoint can be cprload'ed after a host kexec reboot.
> +#
> +# Since 5.0
> +##
> +{ 'command': 'cprsave', 'data': { 'file': 'str', 'mode': 'str' } }
> +
> -- 
> 1.8.3.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK