Re: [Qemu-devel] [PATCH 06/11] snapshot: distinguish id and name in snapshot delete

[Fam Zheng <famz@redhat.com>]

On Sat, 06/08 14:58, Wenchao Xia wrote:
>  
> -static int find_snapshot_by_id(BlockDriverState *bs, const char *id_str)
> +static int find_snapshot_by_id_and_name(BlockDriverState *bs,
> +                                        const char *id,
> +                                        const char *name)
>  {
>      BDRVQcowState *s = bs->opaque;
>      int i;
>  
> -    for(i = 0; i < s->nb_snapshots; i++) {
> -        if (!strcmp(s->snapshots[i].id_str, id_str))
> -            return i;
> +    if (id && name) {
> +        for (i = 0; i < s->nb_snapshots; i++) {
> +            if (!strcmp(s->snapshots[i].id_str, id) &&
> +                !strcmp(s->snapshots[i].name, name)) {
> +                return i;
> +            }
> +        }
> +    } else if (id) {
> +        for (i = 0; i < s->nb_snapshots; i++) {
> +            if (!strcmp(s->snapshots[i].id_str, id)) {
> +                return i;
> +            }
> +        }
> +    } else if (name) {
> +        for (i = 0; i < s->nb_snapshots; i++) {
> +            if (!strcmp(s->snapshots[i].name, name)) {
> +                return i;
> +            }
> +        }
>      }
Can be simplified the same way with patch 4.
>  
> -int qcow2_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
> +int qcow2_snapshot_delete(BlockDriverState *bs,
> +                          const char *snapshot_id,
> +                          const char *name,
> +                          Error **errp)
>  {
>      BDRVQcowState *s = bs->opaque;
>      QCowSnapshot sn;
>      int snapshot_index, ret;
>  
>      /* Search the snapshot */
> -    snapshot_index = find_snapshot_by_id_or_name(bs, snapshot_id);
> +    snapshot_index = find_snapshot_by_id_and_name(bs, snapshot_id, name);
>      if (snapshot_index < 0) {
> +        error_setg(errp,
> +                   "Can't find a snapshot with ID %s and name %s",
> +                   snapshot_id, name);
>          return -ENOENT;
>      }
>      sn = s->snapshots[snapshot_index];
> @@ -550,6 +570,7 @@ int qcow2_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
>      s->nb_snapshots--;
>      ret = qcow2_write_snapshots(bs);
>      if (ret < 0) {
> +        error_setg(errp, "Failed to remove it from snapshot list");
Maybe put name and id in error message, as above?