On 09/13/2013 05:50 AM, Kevin Wolf wrote: > From: Wenchao Xia <xiaw...@linux.vnet.ibm.com> > > 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 <xiaw...@linux.vnet.ibm.com>
> Signed-off-by: Pavel Hrdina <phrd...@redhat.com>
> Signed-off-by: Kevin Wolf <kw...@redhat.com>
> ---
>
> 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
