[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 9/9] migration: introduce snapshot-{save, load, delete} QM
From: |
Daniel P . Berrangé |
Subject: |
Re: [PATCH v4 9/9] migration: introduce snapshot-{save, load, delete} QMP commands |
Date: |
Wed, 16 Sep 2020 09:27:51 +0100 |
User-agent: |
Mutt/1.14.6 (2020-07-11) |
On Wed, Sep 16, 2020 at 10:17:52AM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
>
> > savevm, loadvm and delvm are some of the few HMP commands that have never
> > been converted to use QMP. The reasons for the lack of conversion are
> > that they blocked execution of the event thread, and the semantics
> > around choice of disks were ill-defined.
> >
> > Despite this downside, however, libvirt and applications using libvirt
> > have used these commands for as long as QMP has existed, via the
> > "human-monitor-command" passthrough command. IOW, while it is clearly
> > desirable to be able to fix the problems, they are not a blocker to
> > all real world usage.
> >
> > Meanwhile there is a need for other features which involve adding new
> > parameters to the commands. This is possible with HMP passthrough, but
> > it provides no reliable way for apps to introspect features, so using
> > QAPI modelling is highly desirable.
> >
> > This patch thus introduces new snapshot-{load,save,delete} commands to
> > QMP that are intended to replace the old HMP counterparts. The new
> > commands are given different names, because they will be using the new
> > QEMU job framework and thus will have diverging behaviour from the HMP
> > originals. It would thus be misleading to keep the same name.
> >
> > While this design uses the generic job framework, the current impl is
> > still blocking. The intention that the blocking problem is fixed later.
> > None the less applications using these new commands should assume that
> > they are asynchronous and thus wait for the job status change event to
> > indicate completion.
> >
> > In addition to using the job framework, the new commands require the
> > caller to be explicit about all the block device nodes used in the
> > snapshot operations, with no built-in default heuristics in use.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> [...]
> > diff --git a/qapi/job.json b/qapi/job.json
> > index 280c2f76f1..b2cbb4fead 100644
> > --- a/qapi/job.json
> > +++ b/qapi/job.json
> > @@ -22,10 +22,17 @@
> > #
> > # @amend: image options amend job type, see "x-blockdev-amend" (since 5.1)
> > #
> > +# @snapshot-load: snapshot load job type, see "snapshot-load" (since 5.2)
> > +#
> > +# @snapshot-save: snapshot save job type, see "snapshot-save" (since 5.2)
> > +#
> > +# @snapshot-delete: snapshot delete job type, see "snapshot-delete" (since
> > 5.2)
> > +#
> > # Since: 1.7
> > ##
> > { 'enum': 'JobType',
> > - 'data': ['commit', 'stream', 'mirror', 'backup', 'create', 'amend'] }
> > + 'data': ['commit', 'stream', 'mirror', 'backup', 'create', 'amend',
> > + 'snapshot-load', 'snapshot-save', 'snapshot-delete'] }
> >
> > ##
> > # @JobStatus:
> > diff --git a/qapi/migration.json b/qapi/migration.json
> > index 675f70bb67..b584c0be31 100644
> > --- a/qapi/migration.json
> > +++ b/qapi/migration.json
> > @@ -1720,3 +1720,123 @@
> > ##
> > { 'event': 'UNPLUG_PRIMARY',
> > 'data': { 'device-id': 'str' } }
> > +
> > +##
> > +# @snapshot-save:
> > +#
> > +# Save a VM snapshot
> > +#
> > +# @job-id: identifier for the newly created job
> > +# @tag: name of the snapshot to create
> > +# @devices: list of block device node names to save a snapshot to
>
> Looks like you dropped the idea to also accept drive IDs. Is that for
> good, or would you like to add it later?
I'm still kind of on the fence, but if general opinion is that we should
accept drive IDs, I'll add it.
I wonder what the other blockdev-* APIs accept - some consistency between
APIs is desirable.
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 :|
- [PATCH v4 4/9] block: allow specifying name of block device for vmstate storage, (continued)
- [PATCH v4 4/9] block: allow specifying name of block device for vmstate storage, Daniel P . Berrangé, 2020/09/15
- [PATCH v4 5/9] migration: control whether snapshots are ovewritten, Daniel P . Berrangé, 2020/09/15
- [PATCH v4 6/9] migration: wire up support for snapshot device selection, Daniel P . Berrangé, 2020/09/15
- [PATCH v4 8/9] iotests: add support for capturing and matching QMP events, Daniel P . Berrangé, 2020/09/15
- [PATCH v4 7/9] migration: introduce a delete_snapshot wrapper, Daniel P . Berrangé, 2020/09/15
- [PATCH v4 9/9] migration: introduce snapshot-{save, load, delete} QMP commands, Daniel P . Berrangé, 2020/09/15