From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35394) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bhJVn-0006Vc-3Q for qemu-devel@nongnu.org; Tue, 06 Sep 2016 12:46:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bhJVh-0006es-Om for qemu-devel@nongnu.org; Tue, 06 Sep 2016 12:46:13 -0400 Date: Tue, 6 Sep 2016 18:45:55 +0200 From: Kevin Wolf Message-ID: <20160906164555.GI4667@noname.redhat.com> References: <1464943756-14143-1-git-send-email-famz@redhat.com> <1464943756-14143-4-git-send-email-famz@redhat.com> <20160617092308.GC5431@noname.redhat.com> <20160705133726.GC5882@noname.redhat.com> <20160708025642.GB13961@ad.usersys.redhat.com> <20160708095019.GF14684@noname.redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="0F1p//8PRICkK4MW" Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [Qemu-block] [PATCH v6 03/22] blockdev: Add and parse "lock-mode" option for image locking List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: Fam Zheng , berrange@redhat.com, qemu-block@nongnu.org, rjones@redhat.com, Markus Armbruster , qemu-devel@nongnu.org, stefanha@redhat.com, den@openvz.org, pbonzini@redhat.com, jcody@redhat.com --0F1p//8PRICkK4MW Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Am 08.07.2016 um 15:05 hat Max Reitz geschrieben: > On 08.07.2016 11:50, Kevin Wolf wrote: > > Am 08.07.2016 um 04:56 hat Fam Zheng geschrieben: > >> On Tue, 07/05 15:37, Kevin Wolf wrote: > >>> Am 17.06.2016 um 11:23 hat Kevin Wolf geschrieben: > >>>> Am 03.06.2016 um 10:48 hat Fam Zheng geschrieben: > >>>>> Respect the locking mode from CLI or QMP, and set the open flags > >>>>> accordingly. > >>>>> > >>>>> Signed-off-by: Fam Zheng > >>>>> Reviewed-by: Max Reitz > >>> > >>>> This is the wrong level to implement the feature. You would only be = able > >>>> to configure the locking on the top level image this way (because wh= at > >>>> we're doing here is BB, not BDS configuration). If you want to allow > >>>> configuration per node, you need to put the options into > >>>> bdrv_runtime_opts and interpret them in bdrv_open_common(). > >>>> > >>>> Otherwise we would have to specify in the blockdev-add documentation > >>>> that this works only on the top level, but I don't see a reason for > >>>> such a restriction. > >>> > >>> And actually, after some more thinking about block device configurati= on, > >>> I'm not sure any more whether letting the user configure this on the > >>> node level, at least as the primary interface. > >>> > >>> A node usually knows by itself what locking mode it needs to request > >>> from its children, depending on the locking mode that the parent node > >>> requested for it. It could be passing down the locking mode (raw > >>> format), it could require a stricter locking mode (non-raw formats ne= ver > >>> work with r/w sharing) or it could even be less strict (backing files > >>> are normally ro/ and can therefore be shared, even if the guest can't > >>> share its image). > >>> > >>> The real origin of the locking requirement is the top level. So maybe > >>> the right interface for guest devices is adding a qdev option that te= lls > >>> whether the guest can share the image. For NBD servers, we'd add a QMP > >> > >> I think most block devices are not designed to share the data, so in g= eneral > >> it's hard to imagine this as a device property. > >=20 > > Well, it's really a guest OS (or even guest application) property, but > > obviously that doesn't exist. And the device is the qemu component that > > is the closest to the guest. We generally have options about behaviour > > that the guest expects at the device level. >=20 > Can the guest device really make a qualified decision here? If you're > using raw as the image format, sharing may be fine, if you're using > qcow2, it most likely is not. Just noticed while looking at v7 that I never replied here... Yes, you're right that the guest device can't make the decision for the exact locking mode of the images. But the device is the point where the user has to decide whether or not sharing is okay. For everything below it, qemu knows by itself whether it can share the image. This is essentially what I already described above: > >>> A node usually knows by itself what locking mode it needs to request > >>> from its children, depending on the locking mode that the parent node > >>> requested for it. It could be passing down the locking mode (raw > >>> format), it could require a stricter locking mode (non-raw formats ne= ver > >>> work with r/w sharing) or it could even be less strict (backing files > >>> are normally ro/ and can therefore be shared, even if the guest can't > >>> share its image). > I think whether to lock a BDS chain is a host-side property and has not > a lot to do with the guest, thus it should be a BDS property. I can > imagine that a guest may say that sharing should be disallowed under all > circumstances, but a guest is never able to decide to allow sharing. Well, yes and no. The decision which lock mode to use is at the node level, no doubt. But it's not something that a user can configure. Non-raw images simply can't be shared and the user can't do anything about it. Why should they have an option to specify a lock mode when there is only one correct setting? The only possible exception where an option on the node level could make sense (which I already mentioned earlier in this thread) is if the user wants to be stricter than what qemu needs. Kevin --0F1p//8PRICkK4MW Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJXzvLDAAoJEH8JsnLIjy/WT/wP/jTpwLmrqXSh+GuWzv2wDkfD 0FvzpWaVTPgP1Pqg6cCZcL1etwPwM20BDr9c3vMtJrbOPmGHCBOUqaiKBjbehhq3 YViznWxUNG5B1Gq9Pb4W4sDN4x1PbsHQKbZmzzqcyERoA7kRJXdNKJBOHQzU4D3j FnKw/KOHlRx9BZgeLiqRJH3zlxySlqI3VZiNVThnQ+I3gQ3UYxwZEZA9ufJgoPt4 DN05n7sxjF+BEbqLlFVuDihjS2jgB+EoTrn1/Z9lqWvQgFY81zrTJkHls5/WEKNA k2lHSQ+sEiAPnydjnAECbv14itVa0kMiW0/3z6xbc2dP4BbHH7MCan4e0pMcCpSK 0aSrFCXw5/ppDzMn0KwbQbODhyeA+h+uIgkjNIf9XFOLBmenpUOhg2EAwH3zM0yJ zNnshZZ37vEdTPImxWqXixIRkWprzOBjkIJUDNdEP8UnlqM6WZN69Clrgt/o+SuA MWZiqbcS2illH9YC7aBZTvLHDHwfnuBrLByLUbMaW7aq9/WS4cSXJdeOIqqgG6mv 8ktVD7r/9N7+H/z49HdyqYvz8tnrAMjz0ofxxJRD8Von4jDFZpYK/3Dpfq8vpaxN g619Ym8I5D8fe6y5V6DHTzP/SXDspTFyPjEiOIJ9XIgnkQqYI2/F1KSY9d1xfSO8 B7EHBTqcc4bAdFiDjwP9 =ZY/5 -----END PGP SIGNATURE----- --0F1p//8PRICkK4MW--