Re: [Qemu-devel] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI
Hello, Juan Quintela writes: > "Daniel P. Berrange" wrote: >> On Thu, Jan 11, 2018 at 01:23:05PM +, Dr. David Alan Gilbert wrote: >>> * Daniel P. Berrange (berra...@redhat.com) wrote: >>> > On Thu, Jan 11, 2018 at 01:46:38PM +0100, Max Reitz wrote: >>> > > On 2018-01-08 14:52, Eric Blake wrote: >>> > > > On 01/07/2018 06:23 AM, Richard Palethorpe wrote: >>> > > >> Add QAPI wrapper functions for the existing snapshot functionality. >>> > > >> These >>> > > >> functions behave the same way as the HMP savevm, loadvm and delvm >>> > > >> commands. This will allow applications, such as OpenQA, to >>> > > >> programmatically >>> > > >> revert the VM to a previous state with no dependence on HMP or >>> > > >> qemu-img. >>> > > > >>> > > > That's already possible; libvirt uses QMP's human-monitor-command to >>> > > > access these HMP commands programmatically. >>> > > > >>> > > > We've had discussions in the past about what it would take to have >>> > > > specific QMP commands for these operations; the biggest problem is >>> > > > that >>> > > > these commands promote the use of internal snapshots, and there are >>> > > > enough performance and other issues with internal snapshots that we >>> > > > are >>> > > > not yet ready to commit to a long-term interface for making their use >>> > > > easier. At this point, our recommendation is to prefer external >>> > > > snapshots. >>> > > >>> > > We already have QMP commands for internal snapshots, though. Isn't the >>> > > biggest issue that savevm takes too much time to be a synchronous QMP >>> > > command? >>> > >>> > Ultimately savevm/loadvm are using much of the migration code internally, >>> > but are not exposed as URI schemes. Could we perhaps take advantage of >>> > the internal common layer and define a migration URI scheme >>> > >>> >snapshot: >>> > >>> > where '' is the name of the internal snapshot in the qcow2 file. >>> >>> I had wondered about that; I'd just thought of doing the migration >>> saving to a block device rather than the rest of the snapshot >>> activity around it; >>> but I guess that's possible. >> >> One possible gotcha is whether the current savevm/loadvm QEMUFile impl >> actually does non-blocking I/O properly. eg same reason why we don't >> support a plain file: protocol - POSIX I/O on plain files doesn't >> honour O_NONBLOCK. The block layer does AIO though, so we might be OK, >> depending on which block layer APIs the QEMUFile impl uses. I've not >> looked at the code recently though. > > The blocking part is less important (for the write side), because we > have a thread there. For loading it would be great to get one > migration thread also. > >>> > Then you could just use the regular migrate QMP commands for loading >>> > and saving snapshots. Might need a little extra work on the incoming >>> > side, since we need to be able to load snapshots, despite QEMU not >>> > being started with '-incoming defer', but might still be doable ? >>> > This would theoretically give us progress monitoring, cancellation, >>> > etc for free. >>> >>> What actually stops this working other than the sanity check in >>> migrate_incoming ? >> >> No idea really - not looked closely at the code implications. > > It would be a plus for migration code, right now there are _two_ > implementations, and savevm/loadvm one gets less love. > > And we will check "much more" the way to load migration in a > non-pristine qemu, so > > Later, Juan. This looks like the best option so far for my use case. -- Thank you, Richard.
Re: [Qemu-devel] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI
Coming to this thread rather late, sorry. Eric Blake writes: > On 01/10/2018 10:19 AM, Richard Palethorpe wrote: >> Hello Eric & Peter, >> >> Eric Blake writes: >> >>> On 01/07/2018 06:23 AM, Richard Palethorpe wrote: Add QAPI wrapper functions for the existing snapshot functionality. These functions behave the same way as the HMP savevm, loadvm and delvm commands. This will allow applications, such as OpenQA, to programmatically revert the VM to a previous state with no dependence on HMP or qemu-img. >>> >>> That's already possible; libvirt uses QMP's human-monitor-command to >>> access these HMP commands programmatically. >> >> That is what we are currently doing and is an improvement over >> programatically using HMP, but I wanted to improve upon >> that. Occasionally saving or loading snapshots fails and it is not clear >> why. > > Straightforward mapping of the existing HMP commands into QMP without > any thought about the design won't make the errors any clearer. My > argument is that any QMP design for managing internal snapshots must be > well-designed, but that since we discourage internal snapshots, no one > has been actively working on that design. savevm and loadvm are HMP only precisely because an exact QMP equivalent wouldn't be a sane interface, and designing a sane QMP interface would be work. Things that won't do for QMP include automatic selection of the block device receiving the VM state, and synchronous operation. Building blocks might be a better fit for QMP than a one-size-fits-all savevm command. Internal and external snapshots have different advantages. Because external snapshots are more flexible, they've received a lot more attention[*] than internal ones. I wouldn't let friends use internal snapshots for production. You're welcome to put in the work to push internal snapshots to parity with external ones. Do not underestimate how much work that's going to be. External snapshots' head start is years. [...] [*] "All the attention" would be a fair first order approximation, I think.
Re: [Qemu-devel] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI
"Daniel P. Berrange" wrote: > On Thu, Jan 11, 2018 at 01:23:05PM +, Dr. David Alan Gilbert wrote: >> * Daniel P. Berrange (berra...@redhat.com) wrote: >> > On Thu, Jan 11, 2018 at 01:46:38PM +0100, Max Reitz wrote: >> > > On 2018-01-08 14:52, Eric Blake wrote: >> > > > On 01/07/2018 06:23 AM, Richard Palethorpe wrote: >> > > >> Add QAPI wrapper functions for the existing snapshot functionality. >> > > >> These >> > > >> functions behave the same way as the HMP savevm, loadvm and delvm >> > > >> commands. This will allow applications, such as OpenQA, to >> > > >> programmatically >> > > >> revert the VM to a previous state with no dependence on HMP or >> > > >> qemu-img. >> > > > >> > > > That's already possible; libvirt uses QMP's human-monitor-command to >> > > > access these HMP commands programmatically. >> > > > >> > > > We've had discussions in the past about what it would take to have >> > > > specific QMP commands for these operations; the biggest problem is that >> > > > these commands promote the use of internal snapshots, and there are >> > > > enough performance and other issues with internal snapshots that we are >> > > > not yet ready to commit to a long-term interface for making their use >> > > > easier. At this point, our recommendation is to prefer external >> > > > snapshots. >> > > >> > > We already have QMP commands for internal snapshots, though. Isn't the >> > > biggest issue that savevm takes too much time to be a synchronous QMP >> > > command? >> > >> > Ultimately savevm/loadvm are using much of the migration code internally, >> > but are not exposed as URI schemes. Could we perhaps take advantage of >> > the internal common layer and define a migration URI scheme >> > >> >snapshot: >> > >> > where '' is the name of the internal snapshot in the qcow2 file. >> >> I had wondered about that; I'd just thought of doing the migration >> saving to a block device rather than the rest of the snapshot >> activity around it; >> but I guess that's possible. > > One possible gotcha is whether the current savevm/loadvm QEMUFile impl > actually does non-blocking I/O properly. eg same reason why we don't > support a plain file: protocol - POSIX I/O on plain files doesn't > honour O_NONBLOCK. The block layer does AIO though, so we might be OK, > depending on which block layer APIs the QEMUFile impl uses. I've not > looked at the code recently though. The blocking part is less important (for the write side), because we have a thread there. For loading it would be great to get one migration thread also. >> > Then you could just use the regular migrate QMP commands for loading >> > and saving snapshots. Might need a little extra work on the incoming >> > side, since we need to be able to load snapshots, despite QEMU not >> > being started with '-incoming defer', but might still be doable ? >> > This would theoretically give us progress monitoring, cancellation, >> > etc for free. >> >> What actually stops this working other than the sanity check in >> migrate_incoming ? > > No idea really - not looked closely at the code implications. It would be a plus for migration code, right now there are _two_ implementations, and savevm/loadvm one gets less love. And we will check "much more" the way to load migration in a non-pristine qemu, so Later, Juan.
Re: [Qemu-devel] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI
On Thu, Jan 11, 2018 at 01:23:05PM +, Dr. David Alan Gilbert wrote: > * Daniel P. Berrange (berra...@redhat.com) wrote: > > On Thu, Jan 11, 2018 at 01:46:38PM +0100, Max Reitz wrote: > > > On 2018-01-08 14:52, Eric Blake wrote: > > > > On 01/07/2018 06:23 AM, Richard Palethorpe wrote: > > > >> Add QAPI wrapper functions for the existing snapshot functionality. > > > >> These > > > >> functions behave the same way as the HMP savevm, loadvm and delvm > > > >> commands. This will allow applications, such as OpenQA, to > > > >> programmatically > > > >> revert the VM to a previous state with no dependence on HMP or > > > >> qemu-img. > > > > > > > > That's already possible; libvirt uses QMP's human-monitor-command to > > > > access these HMP commands programmatically. > > > > > > > > We've had discussions in the past about what it would take to have > > > > specific QMP commands for these operations; the biggest problem is that > > > > these commands promote the use of internal snapshots, and there are > > > > enough performance and other issues with internal snapshots that we are > > > > not yet ready to commit to a long-term interface for making their use > > > > easier. At this point, our recommendation is to prefer external > > > > snapshots. > > > > > > We already have QMP commands for internal snapshots, though. Isn't the > > > biggest issue that savevm takes too much time to be a synchronous QMP > > > command? > > > > Ultimately savevm/loadvm are using much of the migration code internally, > > but are not exposed as URI schemes. Could we perhaps take advantage of > > the internal common layer and define a migration URI scheme > > > >snapshot: > > > > where '' is the name of the internal snapshot in the qcow2 file. > > I had wondered about that; I'd just thought of doing the migration > saving to a block device rather than the rest of the snapshot activity around > it; > but I guess that's possible. One possible gotcha is whether the current savevm/loadvm QEMUFile impl actually does non-blocking I/O properly. eg same reason why we don't support a plain file: protocol - POSIX I/O on plain files doesn't honour O_NONBLOCK. The block layer does AIO though, so we might be OK, depending on which block layer APIs the QEMUFile impl uses. I've not looked at the code recently though. > > > Then you could just use the regular migrate QMP commands for loading > > and saving snapshots. Might need a little extra work on the incoming > > side, since we need to be able to load snapshots, despite QEMU not > > being started with '-incoming defer', but might still be doable ? > > This would theoretically give us progress monitoring, cancellation, > > etc for free. > > What actually stops this working other than the sanity check in > migrate_incoming ? No idea really - not looked closely at the code implications. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI
* Daniel P. Berrange (berra...@redhat.com) wrote: > On Thu, Jan 11, 2018 at 01:46:38PM +0100, Max Reitz wrote: > > On 2018-01-08 14:52, Eric Blake wrote: > > > On 01/07/2018 06:23 AM, Richard Palethorpe wrote: > > >> Add QAPI wrapper functions for the existing snapshot functionality. These > > >> functions behave the same way as the HMP savevm, loadvm and delvm > > >> commands. This will allow applications, such as OpenQA, to > > >> programmatically > > >> revert the VM to a previous state with no dependence on HMP or qemu-img. > > > > > > That's already possible; libvirt uses QMP's human-monitor-command to > > > access these HMP commands programmatically. > > > > > > We've had discussions in the past about what it would take to have > > > specific QMP commands for these operations; the biggest problem is that > > > these commands promote the use of internal snapshots, and there are > > > enough performance and other issues with internal snapshots that we are > > > not yet ready to commit to a long-term interface for making their use > > > easier. At this point, our recommendation is to prefer external > > > snapshots. > > > > We already have QMP commands for internal snapshots, though. Isn't the > > biggest issue that savevm takes too much time to be a synchronous QMP > > command? > > Ultimately savevm/loadvm are using much of the migration code internally, > but are not exposed as URI schemes. Could we perhaps take advantage of > the internal common layer and define a migration URI scheme > >snapshot: > > where '' is the name of the internal snapshot in the qcow2 file. I had wondered about that; I'd just thought of doing the migration saving to a block device rather than the rest of the snapshot activity around it; but I guess that's possible. > Then you could just use the regular migrate QMP commands for loading > and saving snapshots. Might need a little extra work on the incoming > side, since we need to be able to load snapshots, despite QEMU not > being started with '-incoming defer', but might still be doable ? > This would theoretically give us progress monitoring, cancellation, > etc for free. What actually stops this working other than the sanity check in migrate_incoming ? Dave > > > Regards, > Daniel > -- > |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o-https://fstop138.berrange.com :| > |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI
On Thu, Jan 11, 2018 at 01:46:38PM +0100, Max Reitz wrote: > On 2018-01-08 14:52, Eric Blake wrote: > > On 01/07/2018 06:23 AM, Richard Palethorpe wrote: > >> Add QAPI wrapper functions for the existing snapshot functionality. These > >> functions behave the same way as the HMP savevm, loadvm and delvm > >> commands. This will allow applications, such as OpenQA, to programmatically > >> revert the VM to a previous state with no dependence on HMP or qemu-img. > > > > That's already possible; libvirt uses QMP's human-monitor-command to > > access these HMP commands programmatically. > > > > We've had discussions in the past about what it would take to have > > specific QMP commands for these operations; the biggest problem is that > > these commands promote the use of internal snapshots, and there are > > enough performance and other issues with internal snapshots that we are > > not yet ready to commit to a long-term interface for making their use > > easier. At this point, our recommendation is to prefer external snapshots. > > We already have QMP commands for internal snapshots, though. Isn't the > biggest issue that savevm takes too much time to be a synchronous QMP > command? Ultimately savevm/loadvm are using much of the migration code internally, but are not exposed as URI schemes. Could we perhaps take advantage of the internal common layer and define a migration URI scheme snapshot: where '' is the name of the internal snapshot in the qcow2 file. Then you could just use the regular migrate QMP commands for loading and saving snapshots. Might need a little extra work on the incoming side, since we need to be able to load snapshots, despite QEMU not being started with '-incoming defer', but might still be doable ? This would theoretically give us progress monitoring, cancellation, etc for free. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI
On 2018-01-08 14:52, Eric Blake wrote: > On 01/07/2018 06:23 AM, Richard Palethorpe wrote: >> Add QAPI wrapper functions for the existing snapshot functionality. These >> functions behave the same way as the HMP savevm, loadvm and delvm >> commands. This will allow applications, such as OpenQA, to programmatically >> revert the VM to a previous state with no dependence on HMP or qemu-img. > > That's already possible; libvirt uses QMP's human-monitor-command to > access these HMP commands programmatically. > > We've had discussions in the past about what it would take to have > specific QMP commands for these operations; the biggest problem is that > these commands promote the use of internal snapshots, and there are > enough performance and other issues with internal snapshots that we are > not yet ready to commit to a long-term interface for making their use > easier. At this point, our recommendation is to prefer external snapshots. We already have QMP commands for internal snapshots, though. Isn't the biggest issue that savevm takes too much time to be a synchronous QMP command? Max signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI
Hello Eric & Peter,
Eric Blake writes:
> On 01/07/2018 06:23 AM, Richard Palethorpe wrote:
>> Add QAPI wrapper functions for the existing snapshot functionality. These
>> functions behave the same way as the HMP savevm, loadvm and delvm
>> commands. This will allow applications, such as OpenQA, to programmatically
>> revert the VM to a previous state with no dependence on HMP or qemu-img.
>
> That's already possible; libvirt uses QMP's human-monitor-command to
> access these HMP commands programmatically.
That is what we are currently doing and is an improvement over
programatically using HMP, but I wanted to improve upon
that. Occasionally saving or loading snapshots fails and it is not clear
why.
>
> We've had discussions in the past about what it would take to have
> specific QMP commands for these operations; the biggest problem is that
> these commands promote the use of internal snapshots, and there are
> enough performance and other issues with internal snapshots that we are
> not yet ready to commit to a long-term interface for making their use
> easier. At this point, our recommendation is to prefer external
> snapshots.
I don't think there are any issues with using external snapshots for the
use case I have in mind, so long as they can contain the CPU & RAM
state. All that is really required is a good clean way of reverting to a
previous state (where the VM is running). I don't see in the
documentation or code that blockdev-snapshot-* or any other command can
save the CPU & RAM state to a file then load it along with a storage
snapshot?
Perhaps instead we could use migrate with exec or fd URIs as Peter
suggests. So we could do a differential migration to a file, then
continue until the guest enters a bad state, then do an incoming
migration to go back to the file.
I don't think that is ideal however, especially from a useability point
of view. Probably many users approaching the problem will just choose to
use loadvm/savevm.
>
> https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg02427.html is
> an example thread that has tried to tackle the QMP-ification of these
> HMP commands in the past; here we are two years later, and I'm still not
> sure we are ready.
Sorry I didn't see that.
>
> I'm worried that taking this commands as-is will bake in an interface
> that we don't want to support in the long term. In particular, the
> qemu-img counterpart of taking an internal snapshot of an offline guest
> is different from QMP context where the guest is running and would
> always include a snapshot of CPU state in addition to disk state; I'm
> also worried that the command is too broad, compared to doing a
> transaction built on lower-level building blocks (take an internal
> snapshot of disk storage, coupled with take an internal snapshot of cpu
> state to be placed into a particular BDS); the HMP command is a bit hard
> to use because the internal snapshot of CPU state has no user control
> over which BDS will actually hold the state (other than which disk was
> installed/hot-plugged first), whereas at the lower level, we probably do
> want to allow management more fine-grained control over where the
> internal snapshot lands.
Savevm and loadvm are actually very easy to use in our particular use
case. However using a transaction comprised of lower-level commands
should not be too difficult either. Although for the common case it
might be a good idea to have a simpler command which wraps the
transaction. Whether it defaults to an internal location or requires an
external file path is not a problem for me at least. The issue seems to
be just adding a QMP command to take a snapshot of the CPU state,
similar to the blockdev-snapshot commands, and allowing it to be part of
a transaction.
>
> Also, when sending a two-patch series, be sure to include a 0/2 cover
> letter. More patch submission hints at
> https://wiki.qemu.org/Contribute/SubmitAPatch
>
>>
>> I used the term snapshot instead of VM because I think it is less ambiguous
>> and matches the internal function names.
>>
>> Signed-off-by: Richard Palethorpe
>> ---
>> migration/savevm.c | 27 ++
>> qapi-schema.json | 66
>> ++
>> 2 files changed, 93 insertions(+)
>>
>
>> +++ b/qapi-schema.json
>> @@ -1125,6 +1125,72 @@
>> { 'command': 'pmemsave',
>>'data': {'val': 'int', 'size': 'int', 'filename': 'str'} }
>>
>> +##
>> +# @save-snapshot:
>> +#
>> +# Save a snapshot of the entire Virtual Machine
>> +#
>> +# @name: A string identifying the snapshot. This can later be used with
>> +#@load-snapshot or @delete-snapshot
>> +#
>> +# Since: 2.12.0
>> +#
>> +# Returns: If successful, nothing
>> +#
>> +# Notes: The equivalent HMP command is savevm. This stores all of the
>> virtual
>
> I don't see why we have to document HMP commands in the QMP document.
>
>> +#machine's current state within the virtual machine's
>> +#image(s)/storage. If
Re: [Qemu-devel] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI
On 01/10/2018 10:19 AM, Richard Palethorpe wrote: > Hello Eric & Peter, > > Eric Blake writes: > >> On 01/07/2018 06:23 AM, Richard Palethorpe wrote: >>> Add QAPI wrapper functions for the existing snapshot functionality. These >>> functions behave the same way as the HMP savevm, loadvm and delvm >>> commands. This will allow applications, such as OpenQA, to programmatically >>> revert the VM to a previous state with no dependence on HMP or qemu-img. >> >> That's already possible; libvirt uses QMP's human-monitor-command to >> access these HMP commands programmatically. > > That is what we are currently doing and is an improvement over > programatically using HMP, but I wanted to improve upon > that. Occasionally saving or loading snapshots fails and it is not clear > why. Straightforward mapping of the existing HMP commands into QMP without any thought about the design won't make the errors any clearer. My argument is that any QMP design for managing internal snapshots must be well-designed, but that since we discourage internal snapshots, no one has been actively working on that design. > >> >> We've had discussions in the past about what it would take to have >> specific QMP commands for these operations; the biggest problem is that >> these commands promote the use of internal snapshots, and there are >> enough performance and other issues with internal snapshots that we are >> not yet ready to commit to a long-term interface for making their use >> easier. At this point, our recommendation is to prefer external >> snapshots. > > I don't think there are any issues with using external snapshots for the > use case I have in mind, so long as they can contain the CPU & RAM > state. All that is really required is a good clean way of reverting to a > previous state (where the VM is running). I don't see in the > documentation or code that blockdev-snapshot-* or any other command can > save the CPU & RAM state to a file then load it along with a storage > snapshot? The trick is to time a blockdev external snapshot with a migration-to-file as your point-in-time live snapshot, then rollback means reverting back to the right disk state before starting via incoming migration from that saved CPU state. Libvirt has managed to combine blockdev-snapshot (external snapshots) plus migration to file in order to properly save CPU + RAM state to a combination of files (admittedly, it is more files to manage than what you get with a single internal snapshot that captures CPU + RAM + disk state all into a single qcow2 image, but the building blocks allow more flexibility). What libvirt has not done well (yet) is the ability to roll back to a particular live snapshot, and has ended up documenting hacks that a user has to manually massage their domain XML to point back to the correct disk image when restoring state from a given CPU state. But again, all the pieces are there, and it's more a matter of just wiring them up correctly. > > Perhaps instead we could use migrate with exec or fd URIs as Peter > suggests. So we could do a differential migration to a file, then > continue until the guest enters a bad state, then do an incoming > migration to go back to the file. > > I don't think that is ideal however, especially from a useability point > of view. Probably many users approaching the problem will just choose to > use loadvm/savevm. As long as a management app hides the complexity, external disk snapshots coupled with external CPU state saved by migration-to-file is just as featured as internal snapshots. Yes, it's not as convenient at the low level as loadvm/savevm using internal snapshots, but it is more powerful, better tested, and less error prone. > > Savevm and loadvm are actually very easy to use in our particular use > case. However using a transaction comprised of lower-level commands > should not be too difficult either. Although for the common case it > might be a good idea to have a simpler command which wraps the > transaction. Whether it defaults to an internal location or requires an > external file path is not a problem for me at least. The issue seems to > be just adding a QMP command to take a snapshot of the CPU state, That exists, in the form of 'migrate'. > similar to the blockdev-snapshot commands, and allowing it to be part of > a transaction. Having 'migrate' be part of a block transaction is not implemented, but given that libvirt has already figured out how to time the combination of the two commands (block snapshots + migration to file) to get a consistent snapshot, I'm not sure that there is any urgent need to improve 'migrate' to work inside 'transaction'. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI
On Sun, Jan 07, 2018 at 01:23:35PM +0100, Richard Palethorpe wrote:
> Add QAPI wrapper functions for the existing snapshot functionality. These
> functions behave the same way as the HMP savevm, loadvm and delvm
> commands. This will allow applications, such as OpenQA, to programmatically
> revert the VM to a previous state with no dependence on HMP or qemu-img.
>
> I used the term snapshot instead of VM because I think it is less ambiguous
> and matches the internal function names.
>
> Signed-off-by: Richard Palethorpe
> ---
> migration/savevm.c | 27 ++
> qapi-schema.json | 66
> ++
> 2 files changed, 93 insertions(+)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index b7908f62be..d7bc0f0d41 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2242,6 +2242,11 @@ int save_snapshot(const char *name, Error **errp)
> return ret;
> }
>
> +void qmp_save_snapshot(const char *name, Error **errp)
> +{
> +save_snapshot(name, errp);
> +}
> +
> void qmp_xen_save_devices_state(const char *filename, bool has_live, bool
> live,
> Error **errp)
> {
> @@ -2404,6 +2409,28 @@ err_drain:
> return ret;
> }
>
> +void qmp_load_snapshot(const char *name, Error **errp)
> +{
> +int saved_vm_running = runstate_is_running();
> +
> +vm_stop(RUN_STATE_RESTORE_VM);
> +
> +if (!load_snapshot(name, errp) && saved_vm_running) {
> +vm_start();
> +}
> +}
Hi, Richard,
I think it's good to have, but from interface POV for sure I'll leave
it for maintainers.
For the code, I would prefer at least unify the code instead of
duplicating it. I think calling qmp_*() functions in hmp_*() would be
good since that's what we do now mostly AFAICT.
Also, comparing to exposing snapshot operations, I am curious about
what would be the difference if we just migrate (which can use QMP) to
a file and then load that file using incoming migration. Thanks,
--
Peter Xu
Re: [Qemu-devel] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI
On 01/07/2018 06:23 AM, Richard Palethorpe wrote:
> Add QAPI wrapper functions for the existing snapshot functionality. These
> functions behave the same way as the HMP savevm, loadvm and delvm
> commands. This will allow applications, such as OpenQA, to programmatically
> revert the VM to a previous state with no dependence on HMP or qemu-img.
That's already possible; libvirt uses QMP's human-monitor-command to
access these HMP commands programmatically.
We've had discussions in the past about what it would take to have
specific QMP commands for these operations; the biggest problem is that
these commands promote the use of internal snapshots, and there are
enough performance and other issues with internal snapshots that we are
not yet ready to commit to a long-term interface for making their use
easier. At this point, our recommendation is to prefer external snapshots.
https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg02427.html is
an example thread that has tried to tackle the QMP-ification of these
HMP commands in the past; here we are two years later, and I'm still not
sure we are ready.
I'm worried that taking this commands as-is will bake in an interface
that we don't want to support in the long term. In particular, the
qemu-img counterpart of taking an internal snapshot of an offline guest
is different from QMP context where the guest is running and would
always include a snapshot of CPU state in addition to disk state; I'm
also worried that the command is too broad, compared to doing a
transaction built on lower-level building blocks (take an internal
snapshot of disk storage, coupled with take an internal snapshot of cpu
state to be placed into a particular BDS); the HMP command is a bit hard
to use because the internal snapshot of CPU state has no user control
over which BDS will actually hold the state (other than which disk was
installed/hot-plugged first), whereas at the lower level, we probably do
want to allow management more fine-grained control over where the
internal snapshot lands.
Also, when sending a two-patch series, be sure to include a 0/2 cover
letter. More patch submission hints at
https://wiki.qemu.org/Contribute/SubmitAPatch
>
> I used the term snapshot instead of VM because I think it is less ambiguous
> and matches the internal function names.
>
> Signed-off-by: Richard Palethorpe
> ---
> migration/savevm.c | 27 ++
> qapi-schema.json | 66
> ++
> 2 files changed, 93 insertions(+)
>
> +++ b/qapi-schema.json
> @@ -1125,6 +1125,72 @@
> { 'command': 'pmemsave',
>'data': {'val': 'int', 'size': 'int', 'filename': 'str'} }
>
> +##
> +# @save-snapshot:
> +#
> +# Save a snapshot of the entire Virtual Machine
> +#
> +# @name: A string identifying the snapshot. This can later be used with
> +#@load-snapshot or @delete-snapshot
> +#
> +# Since: 2.12.0
> +#
> +# Returns: If successful, nothing
> +#
> +# Notes: The equivalent HMP command is savevm. This stores all of the virtual
I don't see why we have to document HMP commands in the QMP document.
> +#machine's current state within the virtual machine's
> +#image(s)/storage. If the VM is currently running, this includes the
> +#state of the CPU and RAM. Later the VM can be reverted to this
> state.
How would the VM _not_ be currently running at the point where QMP can
be executed?
> +#
> +#qemu-img can also be used to manipulate snapshots.
> +#
> +# Examples:
> +#
> +# -> { "execute": "save-snapshot", "arguments": { "name": "lastgood" } }
> +# <- { "return": {} }
> +##
> +{ 'command': 'save-snapshot',
> + 'data': {'name': 'str'} }
> +
> +##
> +# @load-snapshot:
> +#
> +# Load a snapshot of the entire Virtual Machine, completely reverting it to
> +# that state
> +#
> +# Since: 2.12.0
> +#
> +# Returns: If successful, nothing
> +#
> +# Notes: The equivalent HMP command is loadvm. See the @save-snapshot notes.
> +#
> +# Examples:
> +#
> +# -> { "execute": "load-snapshot", "arguments": { "name": "lastgood" } }
> +# <- { "return": {} }
> +##
> +{ 'command': 'load-snapshot',
> + 'data': {'name': 'str'} }
> +
> +##
> +# @delete-snapshot:
> +#
> +# Delete a VM snapshot
> +#
> +# Since: 2.12.0
> +#
> +# Returns: If successful, nothing
> +#
> +# Notes: The equivalent HMP command is delvm. See the @save-snapshot notes.
> +#
> +# Examples:
> +#
> +# -> { "execute": "delete-snapshot", "arguments": { "name": "lastgood" } }
> +# <- { "return": {} }
> +##
> +{ 'command': 'delete-snapshot',
> + 'data': {'name': 'str'} }
> +
> ##
> # @cont:
> #
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
