From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:60633)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <famz@redhat.com>) id 1UlDzD-0003WF-Hn
	for qemu-devel@nongnu.org; Sat, 08 Jun 2013 03:54:58 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <famz@redhat.com>) id 1UlDz9-0005mX-Ui
	for qemu-devel@nongnu.org; Sat, 08 Jun 2013 03:54:55 -0400
Received: from mx1.redhat.com ([209.132.183.28]:34709)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <famz@redhat.com>) id 1UlDz9-0005mL-Ls
	for qemu-devel@nongnu.org; Sat, 08 Jun 2013 03:54:51 -0400
Date: Sat, 8 Jun 2013 15:54:32 +0800
From: Fam Zheng <famz@redhat.com>
Message-ID: <20130608075432.GB9648@localhost.nay.redhat.com>
References: <1370674687-13849-1-git-send-email-xiawenc@linux.vnet.ibm.com>
	<1370674687-13849-7-git-send-email-xiawenc@linux.vnet.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1370674687-13849-7-git-send-email-xiawenc@linux.vnet.ibm.com>
Subject: Re: [Qemu-devel] [PATCH 06/11] snapshot: distinguish id and name in
 snapshot delete
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Cc: kwolf@redhat.com, phrdina@redhat.com, armbru@redhat.com, qemu-devel@nongnu.org, lcapitulino@redhat.com, stefanha@redhat.com, pbonzini@redhat.com, dietmar@proxmox.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?