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

[Steven Sistare <steven.sistare@oracle.com>]

On 9/11/2020 12:43 PM, Dr. David Alan Gilbert wrote:
> * 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).

Good idea, calling qemu_save_device_state instead of qemu_savevm_state will factor
out the steps that are specific to migration.  I'll still need the mode, though,
to exclude savevm_block_handlers, and maybe for other reasons.  I'll try it.

- Steve

>> +    qemu_fclose(f);
>> +
>> +    if (op == VMS_REBOOT) {
>> +        no_shutdown = 0;
>> +        qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
>> +    }
>> +}
>> +
[...]