From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45722) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VJaCh-0007NR-1d for qemu-devel@nongnu.org; Tue, 10 Sep 2013 22:30:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VJaCX-0004CV-KT for qemu-devel@nongnu.org; Tue, 10 Sep 2013 22:30:50 -0400 Received: from e28smtp04.in.ibm.com ([122.248.162.4]:45201) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VJaCW-0004Ai-PB for qemu-devel@nongnu.org; Tue, 10 Sep 2013 22:30:41 -0400 Received: from /spool/local by e28smtp04.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 11 Sep 2013 07:51:54 +0530 Received: from d28relay01.in.ibm.com (d28relay01.in.ibm.com [9.184.220.58]) by d28dlp02.in.ibm.com (Postfix) with ESMTP id 3798E394004D for ; Wed, 11 Sep 2013 08:00:13 +0530 (IST) Received: from d28av02.in.ibm.com (d28av02.in.ibm.com [9.184.220.64]) by d28relay01.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r8B2WI7X29753440 for ; Wed, 11 Sep 2013 08:02:19 +0530 Received: from d28av02.in.ibm.com (localhost [127.0.0.1]) by d28av02.in.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id r8B2UKqM029819 for ; Wed, 11 Sep 2013 08:00:20 +0530 Message-ID: <522FD5BA.8040102@linux.vnet.ibm.com> Date: Wed, 11 Sep 2013 10:30:18 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1375844419-7665-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1375844419-7665-3-git-send-email-xiawenc@linux.vnet.ibm.com> <20130910123620.GD2797@dhcp-200-207.str.redhat.com> In-Reply-To: <20130910123620.GD2797@dhcp-200-207.str.redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH V7 2/8] snapshot: distinguish id and name in snapshot delete List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: phrdina@redhat.com, famz@redhat.com, armbru@redhat.com, qemu-devel@nongnu.org, lcapitulino@redhat.com, stefanha@redhat.com, pbonzini@redhat.com, dietmar@proxmox.com 于 2013/9/10 20:36, 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 >> Signed-off-by: Pavel Hrdina >> 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 > Thanks for reviewing, will fix and rebase.