Re: [Qemu-devel] [PATCH] savevm: create snapshot failed when id_str already exits

[Stefan Hajnoczi]

On Mon, Mar 09, 2015 at 09:32:47PM +0800, Yi Wang wrote:
> This will trigger the problem: vda has snapshot s1 with id "1", vdb
> doesn't have s1 but has snapshot s2 with id "1"。When we want to run
> command "virsh create s1", del_existing_snapshots() only deletes s1 in
> vda, and bdrv_snapshot_create() tries to create vdb's snapshot s1 with
> id "1", but id "1" alreay exists in vdb with name "s2"!
> 
> This shows the condition:
> # qemu-img snapshot -l win7.img.1
> Snapshot list:
> ID TAG VM SIZE DATE VM CLOCK
> 1 s1 534M 2015-03-09 10:28:54 00:03:54.504
> 
> # qemu-img snapshot -l win7.append
> Snapshot list:
> ID TAG VM SIZE DATE VM CLOCK
> 1 s2 0 2015-03-05 10:29:21 00:00:00.000
> 
> # virsh snapshot-create-as win7 s1
> error: operation failed: Failed to take snapshot: Error while creating
> snapshot on 'drive-virtio-disk1'

Then we've arrived at changing the behavior of 'savevm' so it always
generates IDs.

Kevin: What do you think about changing savevm semantics to always
generate IDs?

diff --git a/savevm.c b/savevm.c
index ce2b6a2..bee2143 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1141,7 +1141,6 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
         ret = bdrv_snapshot_find(bs, old_sn, name);
         if (ret >= 0) {
             pstrcpy(sn->name, sizeof(sn->name), old_sn->name);
-            pstrcpy(sn->id_str, sizeof(sn->id_str), old_sn->id_str);
         } else {
             pstrcpy(sn->name, sizeof(sn->name), name);
         }
@@ -1178,6 +1177,12 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
         if (bdrv_can_snapshot(bs1)) {
             /* Write VM state size only to the image that contains the state */
             sn->vm_state_size = (bs == bs1 ? vm_state_size : 0);
+
+            /* Force ID generation for each image since there could be naming
+             * collisions.
+             */
+            sn->id_str[0] = '\0';
+
             ret = bdrv_snapshot_create(bs1, sn);
             if (ret < 0) {
                 monitor_printf(mon, "Error while creating snapshot on '%s'\n",

pgpp_Pp1pHAnV.pgp
Description: PGP signature