Re: [Qemu-devel] [Qemu-block] [PATCH v6 03/22] blockdev: Add and parse "lock-mode" option for image locking

[Kevin Wolf]

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 <address@hidden>
> >>>>> Reviewed-by: Max Reitz <address@hidden>
> >>>
> >>>> 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 what
> >>>> 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 configuration,
> >>> 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 never
> >>> 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 tells
> >>> 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 
> >> general
> >> it's hard to imagine this as a device property.
> > 
> > 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.
> 
> 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 never
> >>> 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

pgpZIOg9FcZxG.pgp
Description: PGP signature