From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48172) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YQYT5-0008Ml-8x for qemu-devel@nongnu.org; Wed, 25 Feb 2015 04:41:24 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YQYT2-0000tA-2A for qemu-devel@nongnu.org; Wed, 25 Feb 2015 04:41:23 -0500 Received: from mx1.redhat.com ([209.132.183.28]:48560) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YQYT1-0000qe-Rw for qemu-devel@nongnu.org; Wed, 25 Feb 2015 04:41:20 -0500 Date: Wed, 25 Feb 2015 09:41:12 +0000 From: Stefan Hajnoczi Message-ID: <20150225094112.GB18680@stefanha-thinkpad.redhat.com> References: MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="/NkBOFFp2J2Af1nK" Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH] savevm: create snapshot failed when id_str already exits List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Yi Wang Cc: kwolf@redhat.com, amit.shah@redhat.com, wang.yi59@zte.com.cn, qemu-devel@nongnu.org, quintela@redhat.com --/NkBOFFp2J2Af1nK Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Jan 12, 2015 at 01:12:41AM +0800, Yi Wang wrote: > From b119164ba6715f53594facfc4d1022c198852e9d Mon Sep 17 00:00:00 2001 > From: Yi Wang > Date: Mon, 12 Jan 2015 00:05:40 +0800 > Subject: [PATCH] savevm: create snapshot failed when id_str already exits >=20 > Create snapshot failed in this case: > 1) vm has two or more qcow2 disks; > 2) the first disk has snapshot with id_str 1, and the second disk has > snapshots with id_str 2 and 3, for example; > 3) create snapshot using virsh snapshot-create/snapshot-create-as will > fail, 'cause id_str 2 has already existed in disk two. >=20 > The reason is that do_savevm() didn't check id_str before create > bdrv_snapshot_create(), and this patch fixed this. >=20 > Signed-off-by: Yi Wang > --- > block/snapshot.c | 32 ++++++++++++++++++++++++++++++++ > include/block/snapshot.h | 1 + > savevm.c | 7 +++++++ > 3 files changed, 40 insertions(+), 0 deletions(-) >=20 > diff --git a/block/snapshot.c b/block/snapshot.c > index 698e1a1..f2757ab 100644 > --- a/block/snapshot.c > +++ b/block/snapshot.c > @@ -66,6 +66,38 @@ int bdrv_snapshot_find(BlockDriverState *bs, > QEMUSnapshotInfo *sn_info, > return ret; > } >=20 > +int bdrv_snapshot_get_id_str(BlockDriverState *bs, QEMUSnapshotInfo *sn_= info) > +{ > + QEMUSnapshotInfo *sn_tab, *sn; > + int nb_sns, i, ret; > + int max =3D 0, temp_max; > + bool need_max =3D false; > + > + ret =3D -ENOENT; > + nb_sns =3D bdrv_snapshot_list(bs, &sn_tab); > + if (nb_sns < 0) { > + return ret; > + } > + > + for (i =3D 0; i < nb_sns; i++) { > + sn =3D &sn_tab[i]; > + temp_max =3D atoi(sn->id_str); > + if (max < temp_max) { > + max =3D temp_max; > + } > + if (!need_max && !strcmp(sn->id_str, sn_info->id_str)) { > + need_max =3D true; > + } > + if (need_max) { > + snprintf(sn_info->id_str, 128*sizeof(char), "%d", max+1); > + } > + > + } > + g_free(sn_tab); > + ret =3D 0; > + return ret; > +} > + > /** > * Look up an internal snapshot by @id and @name. > * @bs: block device to search > diff --git a/include/block/snapshot.h b/include/block/snapshot.h > index 770d9bb..047ed7b 100644 > --- a/include/block/snapshot.h > +++ b/include/block/snapshot.h > @@ -49,6 +49,7 @@ typedef struct QEMUSnapshotInfo { >=20 > int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info, > const char *name); > +int bdrv_snapshot_get_id_str(BlockDriverState *bs, QEMUSnapshotInfo *sn_= info); > bool bdrv_snapshot_find_by_id_and_name(BlockDriverState *bs, > const char *id, > const char *name, > diff --git a/savevm.c b/savevm.c > index 08ec678..f2edc13 100644 > --- a/savevm.c > +++ b/savevm.c > @@ -1123,6 +1123,13 @@ void do_savevm(Monitor *mon, const QDict *qdict) > if (bdrv_can_snapshot(bs1)) { > /* Write VM state size only to the image that contains the s= tate */ > sn->vm_state_size =3D (bs =3D=3D bs1 ? vm_state_size : 0); > + > + /* To avoid sn->id_str already exists */ > + if (bdrv_snapshot_get_id_str(bs1, sn) < 0) { > + monitor_printf(mon, "Error when get id str.\n"); > + goto the_end; > + } > + Does this solve the issue? /* Images may have existing IDs so let the ID be autogenerated if the * user did not specify a name. */ if (!name) { sn->id_str[0] =3D '\0'; } The effect is similar to your patch but it avoids duplicating the id_str generation. --/NkBOFFp2J2Af1nK Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJU7Zi4AAoJEJykq7OBq3PIIwYH/ivJSVxz5FbNzh8JBwTGF+G0 qiUd42+nOZHhkCBbgzLhWJMkLST5jvwsxTKiZnThdvE6ilkMJVRrBfm/4x1q1fe/ WzZhbhv+wDwieveypW9EnMMtGiF23+x4F3M7ZYY9ikJ4W4EAO2ln2Drcr7kHiOlD GyOYjUh9LCZaJv01z4bZJMuCYTpzZsRi++CrQ0NNW9W7tMgt/Zgo5sliUdI7dD56 3n6T9knGcCuGcMRkwiHGG2b7a/yRPMiTXIl3etFG+2AhWH4YkhMymgKPf/Vsl0h3 fzYzGFf0nBiGHhyJTHSTdVcGe4PBZDyTrP6sG4v3Ec/HvgslWVksrEQPP33UhE4= =KhWz -----END PGP SIGNATURE----- --/NkBOFFp2J2Af1nK--