From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:39338) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UPcGp-00066g-CC for qemu-devel@nongnu.org; Tue, 09 Apr 2013 13:23:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UPcGn-0007dZ-EZ for qemu-devel@nongnu.org; Tue, 09 Apr 2013 13:23:47 -0400 Received: from mx1.redhat.com ([209.132.183.28]:64253) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UPcGn-0007dJ-7e for qemu-devel@nongnu.org; Tue, 09 Apr 2013 13:23:45 -0400 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r39HNi52007260 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 9 Apr 2013 13:23:44 -0400 From: Markus Armbruster References: <87d2u3667x.fsf@blackfin.pond.sub.org> <51643DDB.1090309@redhat.com> Date: Tue, 09 Apr 2013 19:23:42 +0200 In-Reply-To: <51643DDB.1090309@redhat.com> (Eric Blake's message of "Tue, 09 Apr 2013 10:12:11 -0600") Message-ID: <8738uz39ep.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v4 07/11] qapi: Convert savevm List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Pavel Hrdina , qemu-devel@nongnu.org, lcapitulino@redhat.com Eric Blake 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.