From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1O4xS9-0000QO-Li for qemu-devel@nongnu.org; Thu, 22 Apr 2010 10:32:29 -0400 Received: from [140.186.70.92] (port=59096 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1O4xS0-0000O8-Mx for qemu-devel@nongnu.org; Thu, 22 Apr 2010 10:32:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1O4xRm-0006tL-CG for qemu-devel@nongnu.org; Thu, 22 Apr 2010 10:32:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42619) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O4xRm-0006t5-4H for qemu-devel@nongnu.org; Thu, 22 Apr 2010 10:32:06 -0400 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o3MEW4f4002244 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 22 Apr 2010 10:32:04 -0400 Message-ID: <4BD05DC7.7070108@redhat.com> Date: Thu, 22 Apr 2010 16:31:35 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1271797792-24571-1-git-send-email-lcapitulino@redhat.com> <1271797792-24571-19-git-send-email-lcapitulino@redhat.com> <4BCF092A.2080005@redhat.com> <20100422104814.002539bc@redhat.com> In-Reply-To: <20100422104814.002539bc@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH 18/22] savevm: Convert do_delvm() to QObject, QError List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: quintela@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com Am 22.04.2010 15:48, schrieb Luiz Capitulino: > On Wed, 21 Apr 2010 16:18:18 +0200 > Kevin Wolf wrote: > >> Am 20.04.2010 23:09, schrieb Luiz Capitulino: >>> Signed-off-by: Luiz Capitulino >>> --- >>> qemu-monitor.hx | 3 ++- >>> savevm.c | 14 ++++++++++---- >>> sysemu.h | 2 +- >>> 3 files changed, 13 insertions(+), 6 deletions(-) >>> >>> diff --git a/qemu-monitor.hx b/qemu-monitor.hx >>> index 5ea5748..71cb1a2 100644 >>> --- a/qemu-monitor.hx >>> +++ b/qemu-monitor.hx >>> @@ -274,7 +274,8 @@ ETEXI >>> .args_type = "name:s", >>> .params = "tag|id", >>> .help = "delete a VM snapshot from its tag or id", >>> - .mhandler.cmd = do_delvm, >>> + .user_print = monitor_user_noop, >>> + .mhandler.cmd_new = do_delvm, >>> }, >>> >>> STEXI >>> diff --git a/savevm.c b/savevm.c >>> index 643273e..031eeff 100644 >>> --- a/savevm.c >>> +++ b/savevm.c >>> @@ -1815,24 +1815,30 @@ int load_vmstate(const char *name) >>> return 0; >>> } >>> >>> -void do_delvm(Monitor *mon, const QDict *qdict) >>> +int do_delvm(Monitor *mon, const QDict *qdict, QObject **ret_data) >>> { >>> + int ret; >>> DriveInfo *dinfo; >>> BlockDriverState *bs, *bs1; >>> const char *name = qdict_get_str(qdict, "name"); >>> >>> bs = get_bs_snapshots(); >>> if (!bs) { >>> - monitor_printf(mon, "No block device supports snapshots\n"); >>> - return; >>> + qerror_report(QERR_SNAPSHOT_NO_DEVICE); >>> + return -1; >>> } >>> >>> + ret = -1; >>> + >>> QTAILQ_FOREACH(dinfo, &drives, next) { >>> bs1 = dinfo->bdrv; >>> if (bdrv_has_snapshot(bs1)) { >>> - delete_snapshot(bs1, name); >>> + /* FIXME: will report multiple failures in QMP */ >>> + ret = delete_snapshot(bs1, name); >>> } >>> } >>> + >>> + return (ret < 0 ? -1 : 0); >> >> Doesn't this return success when the first drive fails and the second >> one succeeds? > > Yes, but what's the real status when this happens? Did delvm > succeed or not? > > I think users will ask the same as we'll print error messages. Don't know. We probably need ternary logic. :-) I think I'd call it an error. But either way, that it depends on the order of disks just doesn't feel right. The status of "disk1 fails and disk2 succeeds" should not be different from "disk1 succeeds and disk2 fails". > Another question is: is it acceptable to return on the first > error? I guess it is. Kevin