From patchwork Fri Apr 26 09:31:12 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Wayne Xia X-Patchwork-Id: 239792 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 082C22C00F3 for ; Fri, 26 Apr 2013 19:59:55 +1000 (EST) Received: from localhost ([::1]:53319 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UVf3v-0003IQ-IC for incoming@patchwork.ozlabs.org; Fri, 26 Apr 2013 05:35:27 -0400 Received: from eggs.gnu.org ([208.118.235.92]:45355) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UVf27-0001Ei-EL for qemu-devel@nongnu.org; Fri, 26 Apr 2013 05:33:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UVf25-0007Iw-Cw for qemu-devel@nongnu.org; Fri, 26 Apr 2013 05:33:35 -0400 Received: from e23smtp05.au.ibm.com ([202.81.31.147]:58526) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UVf24-0007IE-DN for qemu-devel@nongnu.org; Fri, 26 Apr 2013 05:33:33 -0400 Received: from /spool/local by e23smtp05.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 26 Apr 2013 19:28:24 +1000 Received: from d23dlp03.au.ibm.com (202.81.31.214) by e23smtp05.au.ibm.com (202.81.31.211) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Fri, 26 Apr 2013 19:28:22 +1000 Received: from d23relay04.au.ibm.com (d23relay04.au.ibm.com [9.190.234.120]) by d23dlp03.au.ibm.com (Postfix) with ESMTP id 4534D3578052 for ; Fri, 26 Apr 2013 19:33:25 +1000 (EST) Received: from d23av01.au.ibm.com (d23av01.au.ibm.com [9.190.234.96]) by d23relay04.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r3Q9Jl5915401158 for ; Fri, 26 Apr 2013 19:19:48 +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 r3Q9XNn0006879 for ; Fri, 26 Apr 2013 19:33:24 +1000 Received: from RH63Wenchao ([9.77.181.121]) by d23av01.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with ESMTP id r3Q9VDHf002956; Fri, 26 Apr 2013 19:33:20 +1000 From: Wenchao Xia To: qemu-devel@nongnu.org Date: Fri, 26 Apr 2013 17:31:12 +0800 Message-Id: <1366968675-1451-5-git-send-email-xiawenc@linux.vnet.ibm.com> X-Mailer: git-send-email 1.7.1 In-Reply-To: <1366968675-1451-1-git-send-email-xiawenc@linux.vnet.ibm.com> References: <1366968675-1451-1-git-send-email-xiawenc@linux.vnet.ibm.com> X-Content-Scanned: Fidelis XPS MAILER x-cbid: 13042609-1396-0000-0000-000002DDA432 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.4.x-2.6.x [generic] X-Received-From: 202.81.31.147 Cc: kwolf@redhat.com, phrdina@redhat.com, stefanha@gmail.com, armbru@redhat.com, lcapitulino@redhat.com, pbonzini@redhat.com, Wenchao Xia Subject: [Qemu-devel] [PATCH 4/7] block: distinguish id and name in bdrv_find_snapshot() 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 To make it clear about id and name in searching, the API is changed a bit to distinguish them, and caller can choose to search by id or name. If not found, *errp will be set to tip why. Note that the caller logic is changed a bit: 1) In del_existing_snapshots() called by do_savevm(), it travers twice to find the snapshot, instead once, so matching sequence may change if there are unwisely chosen, mixed id and names. 2) In do_savevm(), same with del_existing_snapshot(), when it tries to find the snapshot to overwrite, matching sequence may change for same reason. 3) In load_vmstate(), first when it tries to find the snapshot to be loaded, sequence may change for the same reason of above. Later in validation, the logic is changed to be more strict to require both id and name matching. 4) In do_info_snapshot(), in validation, the logic is changed to be more strict to require both id and name matching. Savevm, loadvm logic may need to be improved later, to avoid mixing of them. Some code is borrowed from Pavel's patch. Signed-off-by: Wenchao Xia Signed-off-by: Pavel Hrdina --- block/snapshot.c | 72 +++++++++++++++++++++++++++++++++++++++------- include/block/snapshot.h | 5 ++- savevm.c | 35 ++++++++++++---------- 3 files changed, 83 insertions(+), 29 deletions(-) diff --git a/block/snapshot.c b/block/snapshot.c index cdc2a76..fa02c4e 100644 --- a/block/snapshot.c +++ b/block/snapshot.c @@ -25,25 +25,75 @@ #include "block/snapshot.h" #include "block/block_int.h" -int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info, - const char *name) +/** + * Look up an internal snapshot by @id and @name. + * @bs: block device to search + * @sn_info: location to store information on the snapshot found + * @id: unique snapshot ID, or NULL + * @name: snapshot name, or NULL + * @errp: location to store error + * + * This function will travers snapshot list in @bs to search the matching + * one, @id and @name are the matching condition: + * If both @id and @name are specified, find the first one with id @id and + * name @name. + * If only @id is specified, find the first one with id @id. + * If only @name is specified, find the first one with name @name. + * if none is specified, return false. + * + * Returns: true when a snapshot is found and @sn_info will be filled, false + * when error or not found with @errp filled if errp != NULL. + */ +bool bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info, + const char *id, const char *name, Error **errp) { QEMUSnapshotInfo *sn_tab, *sn; - int nb_sns, i, ret; + int nb_sns, i; + bool ret = false; - ret = -ENOENT; nb_sns = bdrv_snapshot_list(bs, &sn_tab); if (nb_sns < 0) { - return ret; + error_setg_errno(errp, -nb_sns, "Failed to get a snapshot list"); + return false; + } else if (nb_sns == 0) { + error_setg(errp, "Device has no snapshots"); + return false; } - 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 (id && name) { + for (i = 0; i < nb_sns; i++) { + sn = &sn_tab[i]; + if (!strcmp(sn->id_str, id) && !strcmp(sn->name, name)) { + *sn_info = *sn; + ret = true; + break; + } + } + } else if (id) { + for (i = 0; i < nb_sns; i++) { + sn = &sn_tab[i]; + if (!strcmp(sn->id_str, id)) { + *sn_info = *sn; + ret = true; + break; + } + } + } else if (name) { + for (i = 0; i < nb_sns; i++) { + sn = &sn_tab[i]; + if (!strcmp(sn->name, name)) { + *sn_info = *sn; + ret = true; + break; + } } } + + if (!ret) { + error_setg(errp, "Device have no matching snapshot"); + } + g_free(sn_tab); return ret; } diff --git a/include/block/snapshot.h b/include/block/snapshot.h index 96d4a50..6aac5ed 100644 --- a/include/block/snapshot.h +++ b/include/block/snapshot.h @@ -26,6 +26,7 @@ #define SNAPSHOT_H #include "qemu-common.h" +#include "qapi/error.h" typedef struct QEMUSnapshotInfo { char id_str[128]; /* unique snapshot id */ @@ -38,8 +39,8 @@ typedef struct QEMUSnapshotInfo { uint64_t vm_clock_nsec; /* VM clock relative to boot */ } QEMUSnapshotInfo; -int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info, - const char *name); +bool bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info, + const char *id, const char *name, Error **errp); int bdrv_can_snapshot(BlockDriverState *bs); int bdrv_is_snapshot(BlockDriverState *bs); int bdrv_snapshot_create(BlockDriverState *bs, diff --git a/savevm.c b/savevm.c index ab53a02..5dd2d14 100644 --- a/savevm.c +++ b/savevm.c @@ -2286,8 +2286,8 @@ 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) || + bdrv_snapshot_find(bs, snapshot, NULL, name, NULL))) { ret = bdrv_snapshot_delete(bs, name); if (ret < 0) { monitor_printf(mon, @@ -2346,8 +2346,8 @@ 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) || + bdrv_snapshot_find(bs, old_sn, NULL, name, NULL)) { pstrcpy(sn->name, sizeof(sn->name), old_sn->name); pstrcpy(sn->id_str, sizeof(sn->id_str), old_sn->id_str); } else { @@ -2437,12 +2437,14 @@ 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; - } else if (sn.vm_state_size == 0) { - error_report("This is a disk-only snapshot. Revert to it offline " - "using qemu-img."); + if (bdrv_snapshot_find(bs_vm_state, &sn, name, NULL, NULL) || + bdrv_snapshot_find(bs_vm_state, &sn, NULL, name, NULL)) { + if (sn.vm_state_size == 0) { + error_report("This is a disk-only snapshot. Revert to it " + "offline using qemu-img."); + return -EINVAL; + } + } else { return -EINVAL; } @@ -2461,11 +2463,11 @@ int load_vmstate(const char *name) return -ENOTSUP; } - ret = bdrv_snapshot_find(bs, &sn, name); - if (ret < 0) { + /* vm snapshot will always have same id and name, check do_savevm(). */ + if (!bdrv_snapshot_find(bs, &sn, sn.id_str, sn.name, NULL)) { error_report("Device '%s' does not have the requested snapshot '%s'", bdrv_get_device_name(bs), name); - return ret; + return -ENOENT; } } @@ -2536,7 +2538,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]; @@ -2567,8 +2569,9 @@ 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) { + /* vm snapshot will always have same id and name */ + if (!bdrv_snapshot_find(bs1, sn_info, + sn->id_str, sn->name, NULL)) { available = 0; break; }