[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH V1 03/32] savevm: QMP command for cprsave
From: |
Steven Sistare |
Subject: |
Re: [PATCH V1 03/32] savevm: QMP command for cprsave |
Date: |
Fri, 25 Sep 2020 18:22:18 -0400 |
User-agent: |
Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 |
On 9/25/2020 2:43 PM, Steven Sistare wrote:
> 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.
This works and is a keeper. I do not need mode to exclude
savevm_block_handlers. However,
I still need mode and mode_mask so my vfio vmstate handler is only applied for
the VMS_RESTART
mode. I could instead iterate through the vfio devices and do something
special on save
and load, but the mode_mask is cleaner and could have more uses in the future.
Do you agree?
- Steve
>>> + qemu_fclose(f);
>>> +
>>> + if (op == VMS_REBOOT) {
>>> + no_shutdown = 0;
>>> + qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
>>> + }
>>> +}
>>> +
> [...]
>