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

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Steven Sistare <steven.sistare@oracle.com>
Cc: "Daniel P. Berrange" <berrange@redhat.com>,
	"Juan Quintela" <quintela@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	qemu-devel@nongnu.org, "Markus Armbruster" <armbru@redhat.com>,
	"Alex Williamson" <alex.williamson@redhat.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Alex Bennée" <alex.bennee@linaro.org>
Subject: Re: [PATCH V1 05/32] savevm: QMP command for cprload
Date: Fri, 11 Sep 2020 18:18:09 +0100	[thread overview]
Message-ID: <20200911171809.GM3310@work-vm> (raw)
In-Reply-To: <c135f2e5-10d5-81f5-7251-cead777428aa@oracle.com>

* 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
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK