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

[Kevin Wolf]

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
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?

Kevin