On Wed, Sep 16, 2020 at 10:17:52AM +0200, Markus Armbruster wrote: > Daniel P. Berrangé <berra...@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é <berra...@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 :|