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

[Fam Zheng]

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.

> option that tells whether client can share the image. And starting from
> these requirements, the locking mode would propagate through the graph,
> with each node deciding what it needs to request from its children in
> order to achieve the protection that its parent requested.
> 
> And at this point I start wondering... Doesn't this look an awful lot
> like op blockers? (The new ones.) Should image locking be integrated
> there?
> 
> I still see a (limited) use for the node-level configuration: The user
> might want to request a stricter locking mode than is necessary because
> they foresee an operation that will change the requirements (e.g. commit
> to a backing file) and they don't want to risk failure then.
> 
> Any opinions?

Who is going to enable the default auto lock with an unattached (no BB or no
device) image, such as the qemu-img case?  Lock mode there needs to be
configurable too, but moving the option away from the BB/BDS makes this
trickier to do.

Fam