From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51179) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cCc5b-0007En-VU for qemu-devel@nongnu.org; Thu, 01 Dec 2016 19:52:36 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cCc5b-0001W0-7J for qemu-devel@nongnu.org; Thu, 01 Dec 2016 19:52:36 -0500 References: <1477928314-11184-1-git-send-email-famz@redhat.com> <1477928314-11184-5-git-send-email-famz@redhat.com> From: Max Reitz Message-ID: Date: Fri, 2 Dec 2016 01:52:23 +0100 MIME-Version: 1.0 In-Reply-To: <1477928314-11184-5-git-send-email-famz@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="qHHj1r4JHTxDIPmMSsfhHWVqI1nDteDS5" Subject: Re: [Qemu-devel] [PATCH 04/14] qemu-img: Set "share-rw" flag in read-only commands List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng , qemu-devel@nongnu.org Cc: "Daniel P. Berrange" , Kevin Wolf , qemu-block@nongnu.org, rjones@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --qHHj1r4JHTxDIPmMSsfhHWVqI1nDteDS5 From: Max Reitz To: Fam Zheng , qemu-devel@nongnu.org Cc: "Daniel P. Berrange" , Kevin Wolf , qemu-block@nongnu.org, rjones@redhat.com Message-ID: Subject: Re: [PATCH 04/14] qemu-img: Set "share-rw" flag in read-only commands References: <1477928314-11184-1-git-send-email-famz@redhat.com> <1477928314-11184-5-git-send-email-famz@redhat.com> In-Reply-To: <1477928314-11184-5-git-send-email-famz@redhat.com> Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable On 31.10.2016 16:38, Fam Zheng wrote: > Checking the status of an image when it is being used by guest is > usually useful, True for qemu-img info and maybe even qemu-img compare (and qemu-img map is just a debugging tool, so that's fine, too), but I don't think qemu-img check is very useful. You're not unlikely to see leaks and maybe even errors (with lazy_refcounts=3Don) which don't mean anything because they will go away once the VM is shut down. > and there is no risk of corrupting data, so don't let > the upcoming image locking feature limit this use case. I agree that there is no harm in doing it, but for qemu-img check I also think it isn't very useful either. Anyway, you can keep it since I think it should not be doing anything: The formats implementing qemu-img check should clear BDRV_O_SHARE_RW anyway (unless overridden however that may work). >=20 > Signed-off-by: Fam Zheng > --- > qemu-img.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) >=20 > diff --git a/qemu-img.c b/qemu-img.c > index afcd51f..b2f4077 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -679,6 +679,10 @@ static int img_check(int argc, char **argv) > break; > } > } > + > + if (!(flags & BDRV_O_RDWR)) { > + flags |=3D BDRV_O_SHARE_RW; > + } If you want to keep this for img_check() (and I'm not going to stop you if you do), I think it would be better to put this right in front of img_open() to make it really clear that both are not set at the same time (without having to look into bdrv_parse_cache_mode()). Max > if (optind !=3D argc - 1) { > error_exit("Expecting one image file name"); > } > @@ -1231,6 +1235,7 @@ static int img_compare(int argc, char **argv) > goto out3; > } > =20 > + flags |=3D BDRV_O_SHARE_RW; > blk1 =3D img_open(image_opts, filename1, fmt1, flags, writethrough= , quiet); > if (!blk1) { > ret =3D 2; > @@ -2279,7 +2284,8 @@ static ImageInfoList *collect_image_info_list(boo= l image_opts, > g_hash_table_insert(filenames, (gpointer)filename, NULL); > =20 > blk =3D img_open(image_opts, filename, fmt, > - BDRV_O_NO_BACKING | BDRV_O_NO_IO, false, false)= ; > + BDRV_O_NO_BACKING | BDRV_O_NO_IO | BDRV_O_SHARE= _RW, > + false, false); > if (!blk) { > goto err; > } > @@ -2605,7 +2611,7 @@ static int img_map(int argc, char **argv) > return 1; > } > =20 > - blk =3D img_open(image_opts, filename, fmt, 0, false, false); > + blk =3D img_open(image_opts, filename, fmt, BDRV_O_SHARE_RW, false= , false); > if (!blk) { > return 1; > } >=20 --qHHj1r4JHTxDIPmMSsfhHWVqI1nDteDS5 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- iQFGBAEBCAAwFiEEkb62CjDbPohX0Rgp9AfbAGHVz0AFAlhAxcgSHG1yZWl0ekBy ZWRoYXQuY29tAAoJEPQH2wBh1c9AL1sH/1CnSXltxgPyTpFfUi025kiXZSZImsho bvTZ3zzvQYyDq16qk+vB6rZXUrFPBLnDXWVXE0oJtLSbnLNvUB0fDyPXFzsMdAFZ lEfBDz6oUo3OX+1ye1fUVrHMy34TpP7TwxmQTJeeyUIeKIKiuGWHMBOIiUtn6oo9 PKKb02Lu6nWo5nKihWGm4tsul7rEJmv9biSJA03Pk3qnzYwNu9XB2WCIIaJACJSp t9fEAL4ul0JA9VLKDFD8WydhzFGemdzqCk5cZ3PXhyyoF4wGvIDYEyb7ROPSP0EZ 6hR8guJqulj4CBA73DZ2j3MiZqEi94C+TKaRw9KXvEFfW3WXZhET/Ag= =NoGe -----END PGP SIGNATURE----- --qHHj1r4JHTxDIPmMSsfhHWVqI1nDteDS5--