From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54629) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V06kW-0008Fq-Va for qemu-devel@nongnu.org; Fri, 19 Jul 2013 05:13:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V06kV-0002iU-Jw for qemu-devel@nongnu.org; Fri, 19 Jul 2013 05:13:16 -0400 Received: from e23smtp09.au.ibm.com ([202.81.31.142]:40354) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V06kV-0002iF-13 for qemu-devel@nongnu.org; Fri, 19 Jul 2013 05:13:15 -0400 Received: from /spool/local by e23smtp09.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Sat, 20 Jul 2013 06:08:37 +1000 Received: from d23relay05.au.ibm.com (d23relay05.au.ibm.com [9.190.235.152]) by d23dlp02.au.ibm.com (Postfix) with ESMTP id 91CDA2BB0052 for ; Fri, 19 Jul 2013 19:13:07 +1000 (EST) Received: from d23av01.au.ibm.com (d23av01.au.ibm.com [9.190.234.96]) by d23relay05.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r6J8veEI7668162 for ; Fri, 19 Jul 2013 18:57:40 +1000 Received: from d23av01.au.ibm.com (loopback [127.0.0.1]) by d23av01.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r6J9D7Pc025488 for ; Fri, 19 Jul 2013 19:13:07 +1000 Message-ID: <51E9031C.2010609@linux.vnet.ibm.com> Date: Fri, 19 Jul 2013 17:13:00 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1373521624-4380-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1373521624-4380-3-git-send-email-xiawenc@linux.vnet.ibm.com> <20130718114839.GI3582@dhcp-200-207.str.redhat.com> In-Reply-To: <20130718114839.GI3582@dhcp-200-207.str.redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH V5 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-7-18 19:48, Kevin Wolf 写道: > Am 11.07.2013 um 07:46 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 >> check the return value and do the operations 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 for user. >> >> Some code for *errp is based on Pavel's patch. >> >> Signed-off-by: Wenchao Xia >> Signed-off-by: Pavel Hrdina >> --- >> block/qcow2-snapshot.c | 66 ++++++++++++++++++++++++++++++++++----------- >> block/qcow2.h | 5 +++- >> block/rbd.c | 23 +++++++++++++++- >> block/sheepdog.c | 5 +++- >> block/snapshot.c | 36 ++++++++++++++++++++++-- >> include/block/block_int.h | 5 +++- >> include/block/snapshot.h | 5 +++- >> include/qemu-common.h | 3 ++ >> qemu-img.c | 5 +++- >> savevm.c | 10 +++++- >> 10 files changed, 136 insertions(+), 27 deletions(-) > >> @@ -531,15 +547,23 @@ fail: >> return ret; >> } >> >> -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; >> + const char *device = bdrv_get_device_name(bs); >> >> /* 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' " >> + "on device '%s'", >> + STR_PRINT_CHAR(snapshot_id), STR_PRINT_CHAR(name), device); > > At least the "on device '%s'" part should be removed from the error > messages. It is something the caller can add if there is any ambiguity. > will remove. >> --- a/qemu-img.c >> +++ b/qemu-img.c >> @@ -1883,7 +1883,10 @@ static int img_snapshot(int argc, char **argv) >> break; >> >> case SNAPSHOT_DELETE: >> - ret = bdrv_snapshot_delete(bs, snapshot_name); >> + ret = bdrv_snapshot_delete(bs, snapshot_name, NULL, NULL); >> + if (ret == -ENOENT || ret == -EINVAL) { >> + ret = bdrv_snapshot_delete(bs, NULL, snapshot_name, NULL); >> + } >> if (ret) { >> error_report("Could not delete snapshot '%s': %d (%s)", >> snapshot_name, ret, strerror(-ret)); > > Why don't you use the new error messages? > >> diff --git a/savevm.c b/savevm.c >> index e0491e7..56afebb 100644 >> --- a/savevm.c >> +++ b/savevm.c >> @@ -2332,7 +2332,10 @@ static int del_existing_snapshots(Monitor *mon, const char *name) >> if (bdrv_can_snapshot(bs) && >> bdrv_snapshot_find(bs, snapshot, name) >= 0) >> { >> - ret = bdrv_snapshot_delete(bs, name); >> + ret = bdrv_snapshot_delete(bs, name, NULL, NULL); >> + if (ret == -ENOENT || ret == -EINVAL) { >> + ret = bdrv_snapshot_delete(bs, NULL, name, NULL); >> + } >> if (ret < 0) { >> monitor_printf(mon, >> "Error while deleting snapshot on '%s'\n", >> @@ -2562,7 +2565,10 @@ void do_delvm(Monitor *mon, const QDict *qdict) >> bs1 = NULL; >> while ((bs1 = bdrv_next(bs1))) { >> if (bdrv_can_snapshot(bs1)) { >> - ret = bdrv_snapshot_delete(bs1, name); >> + ret = bdrv_snapshot_delete(bs1, name, NULL, NULL); >> + if (ret == -ENOENT || ret == -EINVAL) { >> + ret = bdrv_snapshot_delete(bs, NULL, name, NULL); >> + } >> if (ret < 0) { >> if (ret == -ENOTSUP) >> monitor_printf(mon, > > Same thing here. > > Kevin > -- Best Regards Wenchao Xia