[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 15/33] snapshot: distinguish id and name in snaps
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PULL 15/33] snapshot: distinguish id and name in snapshot delete |
Date: |
Fri, 13 Sep 2013 12:42:24 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130805 Thunderbird/17.0.8 |
On 09/13/2013 05:50 AM, Kevin Wolf wrote:
> From: Wenchao Xia <address@hidden>
>
> Snapshot creation actually already distinguish id and name since it take
> a structured parameter *sn, but delete can't. Later an accurate delete
> is needed in qmp_transaction abort and blockdev-snapshot-delete-sync,
> so change its prototype. Also *errp is added to tip error, but return
> value is kepted to let caller check what kind of error happens. Existing
s/kepted/kept/
> caller for it are savevm, delvm and qemu-img, they are not impacted by
> introducing a new function bdrv_snapshot_delete_by_id_or_name(), which
> check the return value and do the operation again.
>
> Before this patch:
> For qcow2, it search id first then name to find the one to delete.
> For rbd, it search name.
> For sheepdog, it does nothing.
>
> After this patch:
> For qcow2, logic is the same by call it twice in caller.
> For rbd, it always fails in delete with id, but still search for name
> in second try, no change to user.
>
> Some code for *errp is based on Pavel's patch.
>
> Signed-off-by: Wenchao Xia <address@hidden>
> Signed-off-by: Pavel Hrdina <address@hidden>
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>
> static int qemu_rbd_snap_remove(BlockDriverState *bs,
> - const char *snapshot_name)
> + const char *snapshot_id,
> + const char *snapshot_name,
> + Error **errp)
> {
> BDRVRBDState *s = bs->opaque;
> int r;
>
> + if (!snapshot_name) {
> + error_setg(errp, "rbd need a valid snapshot name");
s/need/needs/
> + return -EINVAL;
> + }
> +
> + /* If snapshot_id is specified, it must be equal to name, see
> + qemu_rbd_snap_list() */
> + if (snapshot_id && strcmp(snapshot_id, snapshot_name)) {
> + error_setg(errp,
> + "rbd do not support snapshot id, it should be NULL or "
s/do/does/
> -int bdrv_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
> +/**
> + * Delete an internal snapshot by @snapshot_id and @name.
> + * @bs: block device used in the operation
> + * @snapshot_id: unique snapshot ID, or NULL
> + * @name: snapshot name, or NULL
> + * @errp: location to store error
> + *
> + * If both @snapshot_id and @name are specified, delete the first one with
> + * id @snapshot_id and name @name.
> + * If only @snapshot_id is specified, delete the first one with id
> + * @snapshot_id.
> + * If only @name is specified, delete the first one with name @name.
> + * if none is specified, return -ENINVAL.
s/ENINVAL/EINVAL/
[Given that this is already in the pull request phase, I'm fine if the
typo in the commit message stays, and if the remaining typos are cleaned
up in a separate followup patch, rather than delaying this series any
longer - my fault for not reviewing sooner]
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PULL 01/33] qcow2: Pass discard type to qcow2_discard_clusters(), (continued)
- [Qemu-devel] [PULL 01/33] qcow2: Pass discard type to qcow2_discard_clusters(), Kevin Wolf, 2013/09/13
- [Qemu-devel] [PULL 02/33] qcow2: Discard VM state in active L1 after creating snapshot, Kevin Wolf, 2013/09/13
- [Qemu-devel] [PULL 11/33] qemu-iotests: add infrastructure of fd passing via SCM, Kevin Wolf, 2013/09/13
- [Qemu-devel] [PULL 10/33] qemu-iotests: add unix socket help program, Kevin Wolf, 2013/09/13
- [Qemu-devel] [PULL 09/33] qemu-iotest: qcow2 image option amendment, Kevin Wolf, 2013/09/13
- [Qemu-devel] [PULL 08/33] qcow2: Implement bdrv_amend_options, Kevin Wolf, 2013/09/13
- [Qemu-devel] [PULL 12/33] qemu-iotests: add tests for runtime fd passing via SCM rights, Kevin Wolf, 2013/09/13
- [Qemu-devel] [PULL 13/33] qemu-iotests: New test case in 061, Kevin Wolf, 2013/09/13
- [Qemu-devel] [PULL 15/33] snapshot: distinguish id and name in snapshot delete, Kevin Wolf, 2013/09/13
- Re: [Qemu-devel] [PULL 15/33] snapshot: distinguish id and name in snapshot delete,
Eric Blake <=
- [Qemu-devel] [PULL 14/33] snapshot: new function bdrv_snapshot_find_by_id_and_name(), Kevin Wolf, 2013/09/13
- [Qemu-devel] [PULL 16/33] qmp: add internal snapshot support in qmp_transaction, Kevin Wolf, 2013/09/13
- [Qemu-devel] [PULL 17/33] qmp: add interface blockdev-snapshot-internal-sync, Kevin Wolf, 2013/09/13
- [Qemu-devel] [PULL 18/33] qmp: add interface blockdev-snapshot-delete-internal-sync, Kevin Wolf, 2013/09/13
- [Qemu-devel] [PULL 19/33] hmp: add interface hmp_snapshot_blkdev_internal, Kevin Wolf, 2013/09/13
- [Qemu-devel] [PULL 20/33] hmp: add interface hmp_snapshot_delete_blkdev_internal, Kevin Wolf, 2013/09/13
- [Qemu-devel] [PULL 21/33] qemu-iotests: add 057 internal snapshot for block device test case, Kevin Wolf, 2013/09/13
- [Qemu-devel] [PULL 22/33] bdrv: Use "Error" for opening images, Kevin Wolf, 2013/09/13
- [Qemu-devel] [PULL 23/33] bdrv: Use "Error" for creating images, Kevin Wolf, 2013/09/13
- [Qemu-devel] [PULL 26/33] qemu-img create: Emit filename on error, Kevin Wolf, 2013/09/13