qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH V3 07/11] block: export function bdrv_find_snaps


From: Pavel Hrdina
Subject: Re: [Qemu-devel] [PATCH V3 07/11] block: export function bdrv_find_snapshot()
Date: Tue, 15 Jan 2013 14:18:16 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130110 Thunderbird/17.0.2

On 01/15/2013 01:01 PM, Markus Armbruster wrote:
Eric Blake <address@hidden> writes:

On 01/14/2013 12:09 AM, Wenchao Xia wrote:
   This patch move it from savevm.c to block.c and export it.

Signed-off-by: Wenchao Xia <address@hidden>
---
  block.c               |   23 +++++++++++++++++++++++
  include/block/block.h |    2 ++
  savevm.c              |   22 ----------------------
  3 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/block.c b/block.c
index 8192d8e..b7d2f03 100644
--- a/block.c
+++ b/block.c
@@ -3351,6 +3351,29 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs,
      return -ENOTSUP;
  }

+int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
+                       const char *name)
+{
+    QEMUSnapshotInfo *sn_tab, *sn;
+    int nb_sns, i, ret;
+
+    ret = -ENOENT;
+    nb_sns = bdrv_snapshot_list(bs, &sn_tab);
+    if (nb_sns < 0) {
+        return ret;
+    }
+    for (i = 0; i < nb_sns; i++) {
+        sn = &sn_tab[i];
+        if (!strcmp(sn->id_str, name) || !strcmp(sn->name, name)) {

It is possible (albeit probably stupid) to create a qcow2 file where
snapshot names are merely numeric strings.  In fact, just to see what
would happen, I once[1] created a file where:

snapshot id '1' was named '2'
snapshot id '2' was named 'foo'

This code comparison favors ids over names; so if I request to delvm 2,
I end up removing the second snapshot, not the first.  This is okay, but
probably worth documenting, and probably worth making sure that all code
that looks up a snapshot by name or id goes through this function so
that we get the same behavior everywhere.  My experiment was done
several months ago, but my recollection was that at the time, there was
an inconsistency where 'qemu-img snapshot' picked a different snapshot
for the request of '2' than the online 'delvm' monitor command of qemu;
making it unsafe to rely on either behavior in that version of qemu
source code.

[1]https://bugzilla.redhat.com/show_bug.cgi?id=733143

In QemuOpts, we restricted IDs to [[:alpha:]][[:alnum:]-._]*, see
id_wellformed().  I'd recommend the same for new interfaces.  But this
is an old one, and we shouldn't make existing snapshots inaccessible
just because their names were chosen unwisely.


There is my proposal for handling snapshots id and name.

http://lists.gnu.org/archive/html/qemu-devel/2013-01/msg01551.html

In general, we will have two options for operation with snapshots, an 'id' option and a 'name' option. You could choose one of them or both to specify which snapshot you want to load/delete/create/modify.

This behaviour should be the same for online and offline snapshots even for vm-snapshots or block-snapshots.

With these modifications old snapshots should also works.

Pavel



reply via email to

[Prev in Thread] Current Thread [Next in Thread]