Eric Blake <ebl...@redhat.com> writes: > On 04/09/2013 10:04 AM, Markus Armbruster wrote: > >>> + >>> +## >>> +# @vm-snapshot-save: >>> +# >>> +# Create a snapshot of the whole virtual machine. If tag is >>> provided as @name, >>> +# it is used as human readable identifier. If there is already a snapshot >>> +# with the same tag or id, the force argument needs to be true to >>> replace it. >> >> "tag or id"? >> >> HMP command savevm's argument is matched against both QEMUSnapshotInfo >> member id_str (a.k.a. id) and name (a.k.a. tag). Do we really want that >> kind of overloading in QMP? Shouldn't we make it tag vs. id explicit >> there? > > The qcow2 file format already allows for the creation of a snapshot with > an id but no tag. We've been back and forth about whether QMP should > allow a user to be able to create all valid qcow2 files (and hence allow > the omission of tag). But things really get confusing if you allow the
I don't have all that context; I hope I'm not wasting everyone's time. > creation of a tag that matches an existing id (create a snapshot, then > create another snapshot with the tag '1'); or even if a tag can match a > potential future id (create a snapshot with a tag named '2', then create > another snapshot). So the lookup should always check for BOTH tag and > id, even though the command is supplying only a 'tag'; and if a tag is > omitted, a unique id will always be returned. > >> >>> +# >>> +# The VM is automatically stopped and resumed and saving a snapshot can >>> take >>> +# a long time. >>> +# >>> +# @name: #optional tag of new snapshot or tag|id of existing snapshot >> >> Should we make this mandatory? We have to keep the argument optional in >> HMP, but that needn't concern us here. > > If we mandate that a tag always be supplied, then we cannot create qcow2 > files with a snapshot that lacks a tag using just QMP; but even if we do Are you sure you can do that with the proposed patches? If I read them correctly, we make up a tag vm-%Y%m%d%H%M%S when none is given. > that, we STILL have to support use of existing qcow2 files that were > created by earlier qemu and/or other tools that have snapshots without a > tag. I understand the need to deal with existing qcow2 files lacking tags. I understand the desire to be able to create such files via QMP. I don't share it, though :) For block driver method bdrv_snapshot_create() both tag and ID are optional. If ID is missing, it picks one. qcow2 picks max. existing ID + 1. If tag is missing, the snapshot doesn't get one. When savevm overwrites an existing snapshot, it reuses both tag and ID. One of them matches @name. When savevm creates a new snapshot, @name becomes the tag. If @name isn't given, we make one up. ID is left to the block driver. vm-snapshot-save behaves the same. Say we have two snapshots, with IDs 1 and 5. Say I want to overwrite the first one, and chose to identify it by its ID. Since I got dirt on my monitor, I misread ID 7. Since neither ID nor tag 7 exist, I accidentally create a new snapshot with tag 7 and ID 6 (picked by qcow2 block driver). Next new snapshot will inevitably get ID 7 and the confusion is complete. All because the stupid command doesn't let me express myself clearly: I mean *ID* 7, *not* tag 7! I think I'd try the following for QMP: * Drop the capability to overwrite existing snapshot. No real loss, as it's not an atomic overwrite: it first deletes the old one, then creates the new one, and if create fails, the old one's still gone. * Take parameter @tag and @id. Fail if @tag is given and matches any existing tag. If it's not given, we make one up that doesn't match any existing tag. Fail if @id is given and matches any existing ID. If it's not given, we make one up that doesn't match any existing ID. * Seriously consider making @tag mandatory.
