All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
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
Subject: Re: [Qemu-devel] [PATCH V2 09/12] qmp: add interface blockdev-snapshot-delete-internal-sync
Date: Sat, 15 Jun 2013 10:55:28 +0100	[thread overview]
Message-ID: <51BC3A10.6040001@redhat.com> (raw)
In-Reply-To: <1371209999-15579-10-git-send-email-xiawenc@linux.vnet.ibm.com>

[-- Attachment #1: Type: text/plain, Size: 1986 bytes --]

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.

> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  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?

> +#
> +# @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.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

  reply	other threads:[~2013-06-15  9:55 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-14 11:39 [Qemu-devel] [PATCH V2 00/12] add internal snapshot support at block device level Wenchao Xia
2013-06-14 11:39 ` [Qemu-devel] [PATCH V2 01/12] blockdev: drop redundant proto_drv check Wenchao Xia
2013-06-14 11:39 ` [Qemu-devel] [PATCH V2 02/12] blockdev: rename BlkTransactionStates to singular Wenchao Xia
2013-06-14 11:39 ` [Qemu-devel] [PATCH V2 03/12] blockdev: allow BdrvActionOps->commit() to be NULL Wenchao Xia
2013-06-14 11:39 ` [Qemu-devel] [PATCH V2 04/12] snapshot: new function bdrv_snapshot_find_by_id_and_name() Wenchao Xia
2013-06-14 11:39 ` [Qemu-devel] [PATCH V2 05/12] snapshot: add paired functions for internal snapshot id and name Wenchao Xia
2013-06-14 11:39 ` [Qemu-devel] [PATCH V2 06/12] snapshot: distinguish id and name in snapshot delete Wenchao Xia
2013-06-14 11:39 ` [Qemu-devel] [PATCH V2 07/12] qmp: add internal snapshot support in qmp_transaction Wenchao Xia
2013-06-15  9:40   ` Eric Blake
2013-06-17  2:43     ` Wenchao Xia
2013-06-18 14:09   ` Stefan Hajnoczi
2013-06-14 11:39 ` [Qemu-devel] [PATCH V2 08/12] qmp: add interface blockdev-snapshot-internal-sync Wenchao Xia
2013-06-15  9:51   ` Eric Blake
2013-06-17  3:09     ` Wenchao Xia
2013-06-14 11:39 ` [Qemu-devel] [PATCH V2 09/12] qmp: add interface blockdev-snapshot-delete-internal-sync Wenchao Xia
2013-06-15  9:55   ` Eric Blake [this message]
2013-06-17  3:25     ` Wenchao Xia
2013-06-18 14:20       ` Stefan Hajnoczi
2013-06-19  6:18         ` Wenchao Xia
2013-06-19  7:46           ` Stefan Hajnoczi
2013-06-19  8:53             ` Wenchao Xia
2013-06-19 14:24               ` Stefan Hajnoczi
2013-06-20  2:37                 ` Wenchao Xia
2013-06-14 11:39 ` [Qemu-devel] [PATCH V2 10/12] hmp: add interface hmp_snapshot_blkdev_internal Wenchao Xia
2013-06-14 11:39 ` [Qemu-devel] [PATCH V2 11/12] hmp: add interface hmp_snapshot_delete_blkdev_internal Wenchao Xia
2013-06-14 11:39 ` [Qemu-devel] [PATCH V2 12/12] qemu-iotests: add 055 internal snapshot for block device test case Wenchao Xia
2013-06-18 14:32   ` Stefan Hajnoczi
2013-06-19  6:21     ` Wenchao Xia
2013-06-19  7:46       ` Stefan Hajnoczi
2013-06-19  9:01         ` Wenchao Xia
2013-06-19 14:26           ` Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=51BC3A10.6040001@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@redhat.com \
    --cc=dietmar@proxmox.com \
    --cc=famz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=phrdina@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=xiawenc@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.