From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52591) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UoQ4o-00074h-MZ for qemu-devel@nongnu.org; Sun, 16 Jun 2013 23:25:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UoQ4n-0000lj-Gj for qemu-devel@nongnu.org; Sun, 16 Jun 2013 23:25:54 -0400 Received: from e28smtp08.in.ibm.com ([122.248.162.8]:52670) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UoQ4m-0000lV-VX for qemu-devel@nongnu.org; Sun, 16 Jun 2013 23:25:53 -0400 Received: from /spool/local by e28smtp08.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 17 Jun 2013 08:47:32 +0530 Received: from d28relay04.in.ibm.com (d28relay04.in.ibm.com [9.184.220.61]) by d28dlp03.in.ibm.com (Postfix) with ESMTP id 12F8E1258043 for ; Mon, 17 Jun 2013 08:54:44 +0530 (IST) Received: from d28av02.in.ibm.com (d28av02.in.ibm.com [9.184.220.64]) by d28relay04.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r5H3PgEt29163688 for ; Mon, 17 Jun 2013 08:55:43 +0530 Received: from d28av02.in.ibm.com (loopback [127.0.0.1]) by d28av02.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r5H3PkRv008511 for ; Mon, 17 Jun 2013 13:25:46 +1000 Message-ID: <51BE81A6.1090202@linux.vnet.ibm.com> Date: Mon, 17 Jun 2013 11:25:26 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1371209999-15579-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1371209999-15579-10-git-send-email-xiawenc@linux.vnet.ibm.com> <51BC3A10.6040001@redhat.com> In-Reply-To: <51BC3A10.6040001@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH V2 09/12] qmp: add interface blockdev-snapshot-delete-internal-sync List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: kwolf@redhat.com, phrdina@redhat.com, famz@redhat.com, armbru@redhat.com, qemu-devel@nongnu.org, lcapitulino@redhat.com, stefanha@redhat.com, pbonzini@redhat.com, dietmar@proxmox.com 于 2013-6-15 17:55, Eric Blake 写道: > On 06/14/2013 12:39 PM, Wenchao Xia wrote: >> Snapshot ID can't be specified in this interface. > > Why not? If it is possible to look up by id in isolation, then it > should be possible to delete by id in isolation. Also, if it is > possible to create a snapshot that has an id but no name (and I think > qcow2 allows such snapshots, even if we don't want to expose that corner > case of creation via normal QMP), then it is mandatory to have a way to > delete by id, since there is no name to delete by. > OK, qemu-img allows create snapshot with name="" before, so we need take care for that corener case in this interface. Here is the redesign: ## # @blockdev-snapshot-delete-internal-sync # # Synchronously delete an internal snapshot of a block device, when the ormat # of the image used support it. # # @device: the name of the device to delete the snapshot from # # @name: the snapshot's name to be deleted # # @id: optional the snapshot's ID to be deleted. If specified, qemu will # try delete the snapshot matching both @name and @id. # # Returns: nothing on success # If @device is not a valid block device, DeviceNotFound # If snapshot not found, generic error will be returned # # Since 1.6 ## { 'command': 'blockdev-snapshot-delete-internal-sync', 'data': { 'device': 'str', 'name': 'str', '*id': 'str' } } Then caller specify name='' and id=[ID] to handle the corner case. >> >> Signed-off-by: Wenchao Xia >> --- >> blockdev.c | 12 ++++++++++++ >> qapi-schema.json | 19 +++++++++++++++++++ >> qmp-commands.hx | 30 ++++++++++++++++++++++++++++++ >> 3 files changed, 61 insertions(+), 0 deletions(-) >> > >> +++ b/qapi-schema.json >> @@ -1712,6 +1712,25 @@ >> 'data': { 'device': 'str', '*name': 'str'} } >> >> ## >> +# @blockdev-snapshot-delete-internal-sync >> +# >> +# Synchronously delete an internal snapshot of a block device, when the format >> +# of the image used support it. > > Should this command be made available via 'transaction'? That is, if I > have a two-disk VM, and use 'transaction' to take a snapshot of both > disks at once, shouldn't I also have a way to delete the snapshots of > both at once, or gracefully fail without data loss if the second one has > problems? I think adding it in transaction is not very useful but brings more complexity. Transcation is used to guareentee all operations are taken in one time point, for example, snapshot creation use it to make sure all are consistent to VM. But for deletion, this requirement do not exist. > >> +# >> +# @device: the name of the device to delete the snapshot from >> +# >> +# @name: the snapshot's name to be deleted >> +# >> +# Returns: nothing on success >> +# If @device is not a valid block device, DeviceNotFound >> +# If snapshot not found, generic error will be returned >> +# >> +# Since 1.6 >> +## >> +{ 'command': 'blockdev-snapshot-delete-internal-sync', >> + 'data': { 'device': 'str', 'name': 'str'} } > > I'm not convinced that this interface is powerful enough. > -- Best Regards Wenchao Xia