From patchwork Wed Apr 24 15:32:01 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pavel Hrdina X-Patchwork-Id: 239232 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 681322C0100 for ; Thu, 25 Apr 2013 01:32:57 +1000 (EST) Received: from localhost ([::1]:41296 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UV1gl-0006Rf-FJ for incoming@patchwork.ozlabs.org; Wed, 24 Apr 2013 11:32:55 -0400 Received: from eggs.gnu.org ([208.118.235.92]:57682) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UV1gH-0006Nl-GE for qemu-devel@nongnu.org; Wed, 24 Apr 2013 11:32:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UV1gF-000401-Pq for qemu-devel@nongnu.org; Wed, 24 Apr 2013 11:32:25 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59942) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UV1gF-0003zm-H0 for qemu-devel@nongnu.org; Wed, 24 Apr 2013 11:32:23 -0400 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r3OFWLwW006815 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 24 Apr 2013 11:32:21 -0400 Received: from localhost.localdomain.com (dhcp-27-230.brq.redhat.com [10.34.27.230]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r3OFWDGK029227; Wed, 24 Apr 2013 11:32:19 -0400 From: Pavel Hrdina To: qemu-devel@nongnu.org Date: Wed, 24 Apr 2013 17:32:01 +0200 Message-Id: In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.67 on 10.5.11.11 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: kwolf@redhat.com, phrdina@redhat.com, armbru@redhat.com, lcapitulino@redhat.com, xiawenc@linux.vnet.ibm.com Subject: [Qemu-devel] [PATCH v2 03/12] savevm: update bdrv_snapshot_find() to find snapshot by id or name and add error parameter X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Finding snapshot by a name which could also be an id isn't best way how to do it. There will be rewrite of savevm, loadvm and delvm to improve the behavior of these commands. The savevm and loadvm will have their own patch series. Now bdrv_snapshot_find takes more parameters. The name parameter will be matched only against the name of the snapshot and the same applies to id parameter. There is one exception. If you set the last parameter, the name parameter will be matched against the name or the id of a snapshot. This exception is only for backward compatibility for other commands and it will be dropped after all commands will be rewritten. We only need to know if that snapshot exists or not. We don't care about any error message. If snapshot exists it returns TRUE otherwise it returns FALSE. There is also new Error parameter which will containt error messeage if something goes wrong. Signed-off-by: Pavel Hrdina --- savevm.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 67 insertions(+), 26 deletions(-) diff --git a/savevm.c b/savevm.c index ba97c41..1622c55 100644 --- a/savevm.c +++ b/savevm.c @@ -2262,26 +2262,66 @@ out: return ret; } -static int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info, - const char *name) +static bool bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info, + const char *name, const char *id, Error **errp, + bool old_match) { QEMUSnapshotInfo *sn_tab, *sn; - int nb_sns, i, ret; + int nb_sns, i; + bool found = false; + + assert(name || id); - ret = -ENOENT; nb_sns = bdrv_snapshot_list(bs, &sn_tab); - if (nb_sns < 0) - return ret; - for(i = 0; i < nb_sns; i++) { + if (nb_sns < 0) { + error_setg_errno(errp, -nb_sns, "Failed to get a snapshot list"); + return found; + } + + if (nb_sns == 0) { + error_setg(errp, "Device has no snapshots"); + return found; + } + + for (i = 0; i < nb_sns; i++) { sn = &sn_tab[i]; - if (!strcmp(sn->id_str, name) || !strcmp(sn->name, name)) { - *sn_info = *sn; - ret = 0; - break; + if (name && id) { + if (!strcmp(sn->id_str, id) && !strcmp(sn->name, name)) { + *sn_info = *sn; + found = true; + break; + } + } else if (name) { + /* for compatibility for old bdrv_snapshot_find call + * will be removed */ + if (old_match) { + if (!strcmp(sn->id_str, name) || !strcmp(sn->name, name)) { + *sn_info = *sn; + found = true; + break; + } + } else { + if (!strcmp(sn->name, name)) { + *sn_info = *sn; + found = true; + break; + } + } + } else if (id) { + if (!strcmp(sn->id_str, id)) { + *sn_info = *sn; + found = true; + break; + } } } + + if (!found) { + error_setg(errp, "Failed to find snapshot '%s'", name ? name : id); + } + g_free(sn_tab); - return ret; + return found; } /* @@ -2296,7 +2336,7 @@ static int del_existing_snapshots(Monitor *mon, const char *name) bs = NULL; while ((bs = bdrv_next(bs))) { if (bdrv_can_snapshot(bs) && - bdrv_snapshot_find(bs, snapshot, name) >= 0) + bdrv_snapshot_find(bs, snapshot, name, NULL, NULL, true)) { bdrv_snapshot_delete(bs, name, &local_err); if (error_is_set(&local_err)) { @@ -2358,8 +2398,7 @@ void do_savevm(Monitor *mon, const QDict *qdict) sn->vm_clock_nsec = qemu_get_clock_ns(vm_clock); if (name) { - ret = bdrv_snapshot_find(bs, old_sn, name); - if (ret >= 0) { + if (bdrv_snapshot_find(bs, old_sn, name, NULL, NULL, true)) { pstrcpy(sn->name, sizeof(sn->name), old_sn->name); pstrcpy(sn->id_str, sizeof(sn->id_str), old_sn->id_str); } else { @@ -2440,6 +2479,7 @@ int load_vmstate(const char *name) BlockDriverState *bs, *bs_vm_state; QEMUSnapshotInfo sn; QEMUFile *f; + Error *local_err = NULL; int ret; bs_vm_state = bdrv_snapshots(); @@ -2449,9 +2489,10 @@ int load_vmstate(const char *name) } /* Don't even try to load empty VM states */ - ret = bdrv_snapshot_find(bs_vm_state, &sn, name); - if (ret < 0) { - return ret; + if (!bdrv_snapshot_find(bs_vm_state, &sn, name, NULL, &local_err, true)) { + error_report("Snapshot doesn't exist: %s", error_get_pretty(local_err)); + error_free(local_err); + return -ENOENT; } else if (sn.vm_state_size == 0) { error_report("This is a disk-only snapshot. Revert to it offline " "using qemu-img."); @@ -2473,11 +2514,11 @@ int load_vmstate(const char *name) return -ENOTSUP; } - ret = bdrv_snapshot_find(bs, &sn, name); - if (ret < 0) { - error_report("Device '%s' does not have the requested snapshot '%s'", - bdrv_get_device_name(bs), name); - return ret; + if (!bdrv_snapshot_find(bs, &sn, name, NULL, &local_err, true)) { + error_report("Snapshot doesn't exist for device '%s': %s", + bdrv_get_device_name(bs), error_get_pretty(local_err)); + error_free(local_err); + return -ENOENT; } } @@ -2546,7 +2587,7 @@ void do_info_snapshots(Monitor *mon, const QDict *qdict) { BlockDriverState *bs, *bs1; QEMUSnapshotInfo *sn_tab, *sn, s, *sn_info = &s; - int nb_sns, i, ret, available; + int nb_sns, i, available; int total; int *available_snapshots; char buf[256]; @@ -2577,8 +2618,8 @@ void do_info_snapshots(Monitor *mon, const QDict *qdict) while ((bs1 = bdrv_next(bs1))) { if (bdrv_can_snapshot(bs1) && bs1 != bs) { - ret = bdrv_snapshot_find(bs1, sn_info, sn->id_str); - if (ret < 0) { + if (!bdrv_snapshot_find(bs1, sn_info, sn->id_str, NULL, NULL, + true)) { available = 0; break; }