Am 10.03.2015 um 14:28 hat Stefan Hajnoczi geschrieben: > 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?
Sounds reasonable. We're free to set whatever ID we want. However, I wouldn't set id_str = "" like in the patch below, but remove the check qcow2_snapshot_create() and call find_new_snapshot_id() unconditionally. Kevin > 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",
pgpJLToZ4rjFj.pgp
Description: PGP signature