diff mbox series

[V1,05/32] savevm: QMP command for cprload

Message ID 1596122076-341293-6-git-send-email-steven.sistare@oracle.com (mailing list archive)
State New, archived
Headers show
Series Live Update | expand

Commit Message

Steven Sistare July 30, 2020, 3:14 p.m. UTC
Provide the cprload QMP command.  The VM is created from the file produced
by the cprsave command.  Guest RAM is restored in-place from the shared
memory backend file, and guest block devices are used as is.  The contents
of such devices must not be modified between the cprsave and cprload
operations.  If the VM was running at cprsave time, then VM execution
resumes.

Syntax:
  {'command':'cprload', 'data':{'file':'str'}}

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
Signed-off-by: Maran Wilson <maran.wilson@oracle.com>
---
 include/sysemu/sysemu.h |  2 ++
 migration/savevm.c      | 34 ++++++++++++++++++++++++++++++++++
 monitor/qmp-cmds.c      |  5 +++++
 qapi/migration.json     | 11 +++++++++++
 softmmu/vl.c            | 15 ++++++++++++++-
 5 files changed, 66 insertions(+), 1 deletion(-)

Comments

Eric Blake July 30, 2020, 4:14 p.m. UTC | #1
On 7/30/20 10:14 AM, Steve Sistare wrote:
> Provide the cprload QMP command.  The VM is created from the file produced
> by the cprsave command.  Guest RAM is restored in-place from the shared
> memory backend file, and guest block devices are used as is.  The contents
> of such devices must not be modified between the cprsave and cprload
> operations.  If the VM was running at cprsave time, then VM execution
> resumes.

Is it always wise to unconditionally resume, or might this command need 
an additional optional knob that says what state (paused or running) to 
move into?

> 
> Syntax:
>    {'command':'cprload', 'data':{'file':'str'}}
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> Signed-off-by: Maran Wilson <maran.wilson@oracle.com>
> ---

> +++ b/qapi/migration.json
> @@ -1635,3 +1635,14 @@
>   ##
>   { 'command': 'cprsave', 'data': { 'file': 'str', 'mode': 'str' } }
>   
> +##
> +# @cprload:
> +#
> +# Start virtual machine from checkpoint file that was created earlier using
> +# the cprsave command.
> +#
> +# @file: name of checkpoint file
> +#
> +# Since 5.0

another 5.2 instance. I'll quit pointing it out for the rest of the series.

> +##
> +{ 'command': 'cprload', 'data': { 'file': 'str' } }
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 660537a..8478778 100644
Steven Sistare July 30, 2020, 6 p.m. UTC | #2
On 7/30/2020 12:14 PM, Eric Blake wrote:
> On 7/30/20 10:14 AM, Steve Sistare wrote:
>> Provide the cprload QMP command.  The VM is created from the file produced
>> by the cprsave command.  Guest RAM is restored in-place from the shared
>> memory backend file, and guest block devices are used as is.  The contents
>> of such devices must not be modified between the cprsave and cprload
>> operations.  If the VM was running at cprsave time, then VM execution
>> resumes.
> 
> Is it always wise to unconditionally resume, or might this command need an additional optional knob that says what state (paused or running) to move into?

This can already be done.  Issue a stop command before cprsave, then cprload will finish in a
paused state.

Also, cprsave re-execs and leaves the guest in a paused state.  One can

send device add commands, then send cprload which continues
.

>> Syntax:
>>    {'command':'cprload', 'data':{'file':'str'}}
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> Signed-off-by: Maran Wilson <maran.wilson@oracle.com>
>> ---
> 
>> +++ b/qapi/migration.json
>> @@ -1635,3 +1635,14 @@
>>   ##
>>   { 'command': 'cprsave', 'data': { 'file': 'str', 'mode': 'str' } }
>>   +##
>> +# @cprload:
>> +#
>> +# Start virtual machine from checkpoint file that was created earlier using
>> +# the cprsave command.
>> +#
>> +# @file: name of checkpoint file
>> +#
>> +# Since 5.0
> 
> another 5.2 instance. I'll quit pointing it out for the rest of the series.

Will find and fix all, thanks.

- Steve
Dr. David Alan Gilbert Sept. 11, 2020, 5:18 p.m. UTC | #3
* Steven Sistare (steven.sistare@oracle.com) wrote:
> On 7/30/2020 12:14 PM, Eric Blake wrote:
> > On 7/30/20 10:14 AM, Steve Sistare wrote:
> >> Provide the cprload QMP command.  The VM is created from the file produced
> >> by the cprsave command.  Guest RAM is restored in-place from the shared
> >> memory backend file, and guest block devices are used as is.  The contents
> >> of such devices must not be modified between the cprsave and cprload
> >> operations.  If the VM was running at cprsave time, then VM execution
> >> resumes.
> > 
> > Is it always wise to unconditionally resume, or might this command need an additional optional knob that says what state (paused or running) to move into?
> 
> This can already be done.  Issue a stop command before cprsave, then cprload will finish in a
> paused state.
> 
> Also, cprsave re-execs and leaves the guest in a paused state.  One can
> 
> send device add commands, then send cprload which continues
> .

You're suffering here because you're reinventing stuff rather than
reusing existing migration paths.
With the existing migration code we require the qemu
to be started with -incoming ... so we know it's in a clean
state ready for being loaded, and we've already got the -S
mechanism that dictates whether or not the VM autostarts
(regardless of the saved state in the image).  The management
layers find this pretty useful if they need to wire some networking
or storage up at the point they know they've got a VM image that's
loaded OK.

Dave

> 
> >> Syntax:
> >>    {'command':'cprload', 'data':{'file':'str'}}
> >>
> >> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> >> Signed-off-by: Maran Wilson <maran.wilson@oracle.com>
> >> ---
> > 
> >> +++ b/qapi/migration.json
> >> @@ -1635,3 +1635,14 @@
> >>   ##
> >>   { 'command': 'cprsave', 'data': { 'file': 'str', 'mode': 'str' } }
> >>   +##
> >> +# @cprload:
> >> +#
> >> +# Start virtual machine from checkpoint file that was created earlier using
> >> +# the cprsave command.
> >> +#
> >> +# @file: name of checkpoint file
> >> +#
> >> +# Since 5.0
> > 
> > another 5.2 instance. I'll quit pointing it out for the rest of the series.
> 
> Will find and fix all, thanks.
> 
> - Steve
>
Steven Sistare Sept. 24, 2020, 9:49 p.m. UTC | #4
On 9/11/2020 1:18 PM, Dr. David Alan Gilbert wrote:
> * Steven Sistare (steven.sistare@oracle.com) wrote:
>> On 7/30/2020 12:14 PM, Eric Blake wrote:
>>> On 7/30/20 10:14 AM, Steve Sistare wrote:
>>>> Provide the cprload QMP command.  The VM is created from the file produced
>>>> by the cprsave command.  Guest RAM is restored in-place from the shared
>>>> memory backend file, and guest block devices are used as is.  The contents
>>>> of such devices must not be modified between the cprsave and cprload
>>>> operations.  If the VM was running at cprsave time, then VM execution
>>>> resumes.
>>>
>>> Is it always wise to unconditionally resume, or might this command need an additional optional knob that says what state (paused or running) to move into?
>>
>> This can already be done.  Issue a stop command before cprsave, then cprload will finish in a
>> paused state.
>>
>> Also, cprsave re-execs and leaves the guest in a paused state.  One can
>>
>> send device add commands, then send cprload which continues
>> .
> 
> You're suffering here because you're reinventing stuff rather than
> reusing existing migration paths.
> With the existing migration code we require the qemu
> to be started with -incoming ... so we know it's in a clean
> state ready for being loaded, and we've already got the -S
> mechanism that dictates whether or not the VM autostarts
> (regardless of the saved state in the image).  The management
> layers find this pretty useful if they need to wire some networking
> or storage up at the point they know they've got a VM image that's
> loaded OK.

I am not seeing the issue here.  The manager can hot plug with cprsave as
easily as with migration, at the same transition points.  I don't see what
migration paths should be reused here.

CPR                                     Migration

- cprsave restarts qemu with the env    - qemu -S -incoming defer
var QEMU_START_FREEZE set, which
clears autostart just like -S.
(see patch 15)

- command-line devices created          - command-line devices created

- vmstate is prelaunch                  - vmstate is inmigrate

- manager sends hotplug commands        - manager sends hotplug commands

- manager sends cprload                 - manager sends migrate_incoming

It would perhaps be more correct for cprsave to leave the vm in the preconfig
state.

I don't feel like I'm suffering.  At least not yet :)

- Steve

>>>> Syntax:
>>>>    {'command':'cprload', 'data':{'file':'str'}}
>>>>
>>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>>>> Signed-off-by: Maran Wilson <maran.wilson@oracle.com>
>>>> ---
>>>
>>>> +++ b/qapi/migration.json
>>>> @@ -1635,3 +1635,14 @@
>>>>   ##
>>>>   { 'command': 'cprsave', 'data': { 'file': 'str', 'mode': 'str' } }
>>>>   +##
>>>> +# @cprload:
>>>> +#
>>>> +# Start virtual machine from checkpoint file that was created earlier using
>>>> +# the cprsave command.
>>>> +#
>>>> +# @file: name of checkpoint file
>>>> +#
>>>> +# Since 5.0
>>>
>>> another 5.2 instance. I'll quit pointing it out for the rest of the series.
>>
>> Will find and fix all, thanks.
>>
>> - Steve
>>
diff mbox series

Patch

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 6fe86e6..5360da5 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -25,6 +25,7 @@  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);
+void load_cpr_snapshot(const char *file, Error **errp);
 
 extern int autostart;
 
@@ -53,6 +54,7 @@  extern uint8_t *boot_splash_filedata;
 extern bool enable_mlock;
 extern bool enable_cpu_pm;
 extern QEMUClockType rtc_clock;
+extern int start_on_wake;
 
 #define MAX_OPTION_ROMS 16
 typedef struct QEMUOptionRom {
diff --git a/migration/savevm.c b/migration/savevm.c
index ff1a46e..1509173 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2948,6 +2948,40 @@  void qmp_xen_load_devices_state(const char *filename, Error **errp)
     migration_incoming_state_destroy();
 }
 
+void load_cpr_snapshot(const char *file, Error **errp)
+{
+    QEMUFile *f;
+    int ret;
+    RunState state;
+
+    if (runstate_is_running()) {
+        error_setg(errp, "cprload called for a running VM");
+        return;
+    }
+
+    f = qf_file_open(file, O_RDONLY, 0, errp);
+    if (!f) {
+        return;
+    }
+
+    ret = qemu_loadvm_state(f, VMS_REBOOT);
+    qemu_fclose(f);
+    if (ret < 0) {
+        error_setg(errp, "Error %d while loading VM state", ret);
+        return;
+    }
+
+    state = global_state_get_runstate();
+    if (state == RUN_STATE_RUNNING) {
+        vm_start();
+    } else {
+        runstate_set(state);
+        if (runstate_check(RUN_STATE_SUSPENDED)) {
+            start_on_wake = 1;
+        }
+    }
+}
+
 int load_snapshot(const char *name, Error **errp)
 {
     BlockDriverState *bs, *bs_vm_state;
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index 9ec7b88..81e6feb 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -167,6 +167,11 @@  void qmp_cprsave(const char *file, const char *mode, Error **errp)
     save_cpr_snapshot(file, mode, errp);
 }
 
+void qmp_cprload(const char *file, Error **errp)
+{
+    load_cpr_snapshot(file, errp);
+}
+
 void qmp_system_wakeup(Error **errp)
 {
     if (!qemu_wakeup_suspend_enabled()) {
diff --git a/qapi/migration.json b/qapi/migration.json
index b61df1d..ce4d32b 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1635,3 +1635,14 @@ 
 ##
 { 'command': 'cprsave', 'data': { 'file': 'str', 'mode': 'str' } }
 
+##
+# @cprload:
+#
+# Start virtual machine from checkpoint file that was created earlier using
+# the cprsave command.
+#
+# @file: name of checkpoint file
+#
+# Since 5.0
+##
+{ 'command': 'cprload', 'data': { 'file': 'str' } }
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 660537a..8478778 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -137,6 +137,7 @@  static time_t rtc_ref_start_datetime;
 static int rtc_realtime_clock_offset; /* used only with QEMU_CLOCK_REALTIME */
 static int rtc_host_datetime_offset = -1; /* valid & used only with
                                              RTC_BASE_DATETIME */
+int start_on_wake;
 QEMUClockType rtc_clock;
 int vga_interface_type = VGA_NONE;
 static DisplayOptions dpy;
@@ -602,6 +603,8 @@  static const RunStateTransition runstate_transitions_def[] = {
     { RUN_STATE_PRELAUNCH, RUN_STATE_RUNNING },
     { RUN_STATE_PRELAUNCH, RUN_STATE_FINISH_MIGRATE },
     { RUN_STATE_PRELAUNCH, RUN_STATE_INMIGRATE },
+    { RUN_STATE_PRELAUNCH, RUN_STATE_SUSPENDED },
+    { RUN_STATE_PRELAUNCH, RUN_STATE_PAUSED },
 
     { RUN_STATE_FINISH_MIGRATE, RUN_STATE_RUNNING },
     { RUN_STATE_FINISH_MIGRATE, RUN_STATE_PAUSED },
@@ -1519,7 +1522,17 @@  void qemu_system_wakeup_request(WakeupReason reason, Error **errp)
     if (!(wakeup_reason_mask & (1 << reason))) {
         return;
     }
-    runstate_set(RUN_STATE_RUNNING);
+
+    /*
+     * Must call vm_start if it has never been called, to invoke the state
+     * change callbacks for the first time.
+     */
+    if (start_on_wake) {
+        start_on_wake = 0;
+        vm_start();
+    } else {
+        runstate_set(RUN_STATE_RUNNING);
+    }
     wakeup_reason = reason;
     qemu_notify_event();
 }