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