From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55281) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cCcEi-0001Ol-QN for qemu-devel@nongnu.org; Thu, 01 Dec 2016 20:02:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cCcEi-0006iK-12 for qemu-devel@nongnu.org; Thu, 01 Dec 2016 20:02:00 -0500 References: <1477928314-11184-1-git-send-email-famz@redhat.com> <1477928314-11184-6-git-send-email-famz@redhat.com> From: Max Reitz Message-ID: Date: Fri, 2 Dec 2016 02:01:48 +0100 MIME-Version: 1.0 In-Reply-To: <1477928314-11184-6-git-send-email-famz@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="uOUQTFagiXUUpcO4Q2GCfmoT9mAe0sJDW" Subject: Re: [Qemu-devel] [PATCH 05/14] block: Set "share-rw" flag in drive-backup when sync=none 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) --uOUQTFagiXUUpcO4Q2GCfmoT9mAe0sJDW 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 05/14] block: Set "share-rw" flag in drive-backup when sync=none References: <1477928314-11184-1-git-send-email-famz@redhat.com> <1477928314-11184-6-git-send-email-famz@redhat.com> In-Reply-To: <1477928314-11184-6-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: > In this case we may open the source's backing image chain multiple > times. Setting share flag means the new open won't try to acquire or > check any lock, once we implement image locking. >=20 > Signed-off-by: Fam Zheng >=20 > --- >=20 > An alternative is reusing (and bdrv_ref) the existing source's backing > bs instead of opening another one. If we decide that approach is better= , > it's better to do it in a separate series. Yes, it is better (there's a reason why we do it for drive-mirror), and while I somehow agree that we could put it off until later, I'm not sure we should. Opening an image with both BDRV_O_RDWR and BDRV_O_SHARE_RW at the same time just because our implementation is lacking is not ideal. Anyway, the whole issue becomes more complex when involving format drivers. I'll write more to that in a response to the cover letter. > --- > blockdev.c | 1 + > 1 file changed, 1 insertion(+) >=20 > diff --git a/blockdev.c b/blockdev.c > index d11a74f..9992c5d 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -3160,6 +3160,7 @@ static void do_drive_backup(DriveBackup *backup, = BlockJobTxn *txn, Error **errp) > } > if (backup->sync =3D=3D MIRROR_SYNC_MODE_NONE) { > source =3D bs; > + flags |=3D BDRV_O_SHARE_RW; In any case, there should be a comment explaining the situation here (having to go through git blame is a bit tedious...), possibly involving a TODO or FIXME regarding that we really shouldn't be using BDRV_O_SHARE_RW (or maybe we should? I'm not sure, I'll explore it in said cover letter response). Max > } > =20 > size =3D bdrv_getlength(bs); >=20 --uOUQTFagiXUUpcO4Q2GCfmoT9mAe0sJDW Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- iQFGBAEBCAAwFiEEkb62CjDbPohX0Rgp9AfbAGHVz0AFAlhAx/wSHG1yZWl0ekBy ZWRoYXQuY29tAAoJEPQH2wBh1c9A03AH/2rQQ+xqCHWSOQATTiVQ7LTGjh+OCDeK pRDObz9ZXymcnpVunMqX4LXWaBLD+PU7hHcJanDBko3cCCP4LFP2zC+5/jFpSHXl YHLEI2yMOy46gnO6bnCy605W/ka9FRn0uzGLxktf6Z+jBm9PnbQLVsQR6laSwB7D KP0fSQMB1ih4RsP3WQzaWJlLZvHRBnI8Mau8985aXzLhnLS+L+oFIugpyR0zmdPA tvXyJNpIVHhY2vnSwW1vjlAm06HfgPjt28OIVUSuHRO+tARBjJ/SoU3ft5YSESwN Qloomivt1AxQyyPBJFtWd5NsWXZ1rGh1oCm4FC4j/jE4s94CeX+WSxo= =G2FS -----END PGP SIGNATURE----- --uOUQTFagiXUUpcO4Q2GCfmoT9mAe0sJDW--