Re: [Qemu-devel] [RFC] Intermediate block mirroring

[Kevin Wolf <kwolf@redhat.com>]

Am 01.06.2018 um 12:51 hat Alberto Garcia geschrieben:
> On Thu 03 May 2018 02:22:41 PM CEST, Kevin Wolf wrote:
> >> > Were the (more or less) exact requirements of QMP blockdev-reopen
> >> > discussed? How is it different from qemu-io's "reopen" command?
> >> > What are the options that you can and can not change?
> >> 
> >> I can't quite remember, I'm afraid.  I think it was supposed to be
> >> pretty much qemu-io's reopen (so just bdrv_reopen()).  I suppose you
> >> cannot change the driver (obviously) or probably the node name, because
> >> either would result in the node being replaced by a completely new one.
> >> 
> >> Other than that, it probably depends on what the block driver supports,
> >> but ideally you should be able to change everything.
> >
> > Honestly the design of bdrv_reopen() is quite broken because of the
> > way it tries to maintain old options if they aren't specified, and
> > guesses what you might mean when you add flags to the mix. The exact
> > semantics are quite complicated and I'd rather avoid them in a stable
> > API.
> >
> > A clean QMP command would probably apply the same defaults as
> > blockdev-add, so you just get to specify the full options again.
> 
> I have a prototype of this working and almost ready to be published, but
> there's a tricky thing with this part:
> 
> If we want blockdev-reopen to apply the defaults for all options except
> from the ones expliclity specified by the user, then it means that we
> need to check not just the options that are present, but also the ones
> that are omitted.
> 
> For example:
> 
>    { "execute": "blockdev-add",
>      "arguments": { "driver": "null-aio",
>                     "node-name": "root",
>                     "size": 1024 }
> 
> This adds a null-aio block device with the "size" option set to 1024
> (the default is 1 << 30).
> 
> null_reopen_prepare() allows reopening that block device, but it does
> not allow changing any of its options. Attempting to change the value of
> "size" is detected by the loop that checks unhandled options at the end
> of bdrv_reopen_prepare() and returns "Cannot change the option 'size'".
> 
> So far, so good. We have this generic check for all options that works
> with all drivers, so as long as we only specify options that we know
> that can be changed, everything is fine.
> 
> However if we want blockdev-reopen to apply the default values for all
> omitted options, then omitting "size" would be equivalent to setting it
> to its default value (1 << 30). And if "size" cannot be changed then
> QEMU should complain unless we explicitly set "size" to 1024 again on
> reopen.
> 
> This complicates things a bit, because we would go from "the options
> that can't be changed are the ones that are not handled by each driver's
> _prepare() function" to "options that are absent can also produce an
> error".

To be honest, I think this is fine. If the user can specify the size
once (in blockdev-add), they can do it again (in blockdev-reopen).

We just need to make sure that we don't break existing bdrv_reopen*()
calls that come from places other than the monitor.

Kevin