Re: [Qemu-devel] [PATCH V7 2/8] snapshot: distinguish id and name in snapshot delete

[Kevin Wolf]

Am 07.08.2013 um 05:00 hat Wenchao Xia geschrieben:
> 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
> 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>

> diff --git a/savevm.c b/savevm.c
> index 03fc4d9..0808414 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -2325,18 +2325,21 @@ static int del_existing_snapshots(Monitor *mon, const 
> char *name)
>  {
>      BlockDriverState *bs;
>      QEMUSnapshotInfo sn1, *snapshot = &sn1;
> -    int ret;
> +    Error *err = NULL;
>  
>      bs = NULL;
>      while ((bs = bdrv_next(bs))) {
>          if (bdrv_can_snapshot(bs) &&
>              bdrv_snapshot_find(bs, snapshot, name) >= 0)
>          {
> -            ret = bdrv_snapshot_delete(bs, name);
> -            if (ret < 0) {
> +            bdrv_snapshot_delete_by_id_or_name(bs, name, &err);
> +            if (error_is_set(&err)) {
>                  monitor_printf(mon,
> -                               "Error while deleting snapshot on '%s'\n",
> -                               bdrv_get_device_name(bs));
> +                               "Error while deleting snapshot on device 
> '%s', "
> +                               "reason: %s\n",

More commonly, error messages just use a colon before the detailed
error code instead of saying ", reason:"

> +                               bdrv_get_device_name(bs),
> +                               error_get_pretty(err));
> +                error_free(err);
>                  return -1;
>              }
>          }

Kevin