On 1/17/19 9:33 AM, Alberto Garcia wrote: > > Changing options > ================ > The general idea is quite straightforward, but the devil is in the > details. Since this command tries to mimic blockdev-add, the state of > the block device after being reopened should generally be equivalent > to that of a block device added with the same set of options. > > There are two important things with this: > > a) Some options cannot be changed (most drivers don't allow that, in > fact). > b) If an option is missing, it should be reset to its default value > (rather than keeping its previous value).
Could we use QAPI alternate types where we could state that: "option":"new" - set the option "option":null - reset the option to its default omit option - leave the option unchanged basically, making each of the options an Alternate with JSON null, so that a request to reset to the default becomes an explicit action? > > Example: the default value of l2-cache-size is 1MB. If we set a > different value (2MB) on blockdev-add but then omit the option in > x-blockdev-reopen, then it should be reset back to 1MB. The current > bdrv_reopen() code keeps the previous value. > > Trying to change an option that doesn't allow it is already being > handled by the code. The loop at the end of bdrv_reopen_prepare() > checks all options that were not handled by the block driver and sees > if any of them has been modified. > > However, as I explained earlier in (b), omitting an option is supposed > to reset it to its default value, so it's also a way of changing > it. If the option cannot be changed then we should detect it and > return an error. What I'm doing in this series is making every driver > publish its list of run-time options, and which ones of them can be > modified. > > Backing files > ============= > This command allows reconfigurations in the node graph. Currently it > only allows changing an image's backing file, but it should be > possible to implement similar functionalities in drivers that have > other children, like blkverify or quorum. > > Let's add an image with its backing file (hd1 <- hd0) like this: > > { "execute": "blockdev-add", > "arguments": { > "driver": "qcow2", > "node-name": "hd0", > "file": { > "driver": "file", > "filename": "hd0.qcow2", > "node-name": "hd0f" > }, > "backing": { > "driver": "qcow2", > "node-name": "hd1", > "file": { > "driver": "file", > "filename": "hd1.qcow2", > "node-name": "hd1f" > } > } > } > } > > Removing the backing file can be done by simply passing the option { > ..., "backing": null } to x-blockdev-reopen. > > Replacing it is not so straightforward. If we pass something like { > ..., "backing": { "driver": "foo" ... } } then x-blockdev-reopen will > assume that we're trying to change the options of the existing backing > file (and it will fail in most cases because it'll likely think that > we're trying to change the node name, among other options). > > So I decided to use a reference instead: { ..., "backing": "hd2" }, > where "hd2" is of an existing node that has been added previously. > > Leaving out the "backing" option can be ambiguous on some cases (what > should it do? keep the current backing file? open the default one > specified in the image file?) so x-blockdev-reopen requires that this > option is present. For convenience, if the BDS doesn't have a backing > file then we allow leaving the option out. Hmm - that makes my proposal of "option":null as an explicit request to the default a bit trickier, if we are already using null for a different meaning for backing files. > > As you can see in the patch series, I wrote a relatively large amount > of tests for many different scenarios, including some tricky ones. > > Perhaps the part that I'm still not convinced about is the one about > freezing backing links to prevent them from being removed, but the > implementation was quite simple so I decided to give it a go. As > you'll see in the patches I chose to use a bool instead of a counter > because I couldn't think of a case where it would make sense to have > two users freezing the same backing link. > > Thanks, > > Berto > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature