On 2018-08-29 16:56, Alberto Garcia wrote: > On Wed 29 Aug 2018 02:59:01 PM CEST, Max Reitz wrote: >>>> Specifically, that means when you don't specify @backing, the default >>>> chain will be opened and may replace the current one. >>> >>> At the moment "backing" is the only child option that can be omitted, >>> all others must be specified. So if you leave "backing" out on reopen() >>> we have the following possibilities: >>> >>> a) We open the default backing file from disk. I don't think we want >>> to open a new image on reopen(). Plus, what happens if the current >>> backing image is the default one? Do we need to detect that? And >>> what if it's the default image but has non-default options? The >>> whole thing looks like a can of worms. >> >> True. >> >>> b) We leave the current backing file untouched. >> >> Hm. That doesn't make sense for blockdev-add, so you could argue that >> this behavior works as a default for reopen (because it cannot be >> "confused" with the blockdev-add default). >> >>> c) We forbid it, and the user has to pass an explicit child, or NULL. >> >> That sounds good. >> >>> I chose (b) in this series. I suppose (c) could also be an option. But >>> (a) doesn't looke like a good idea. >> >> I agree with the last sentiment, but I'd prefer making it an error >> instead of choosing (b). Yes, you could argue that choosing (b) as a >> default makes sense in a way, but in another, it just doesn't make >> sense. (Because while (b) makes no sense for blockdev-add, (a) would >> make sense for both blockdev-add and reopen. Sure, it's horrible to >> support, but from a user's POV, it makes sense. So it's just >> confusing not to make that the default for reopen as well). >> >> But the most important point is that it's trivial for the user to keep >> the current backing file. They just need to give the current >> backing's node-name. > > Or NULL if there's no backing file, right?
Ah, right, that case... Yes. Or we could make an exception (if @backing wasn't specified, and there is no default backing file, then we can suppress the error and handle it like backing=null). Right now, I'm not sure whether making an exception is a great idea, but it's stupid always having to pass backing=null for all qcow2 nodes that do not have a backing file... > The problem I see with this is that it would make the options quite > verbose. "backing" is an attribute of all BDSs, also protocol ones. I > suppose we can make "backing" optional in those, or is there any case > where it makes sense? Right. Making it optional when there is no default backing file maybe is less bad than always requiring it. Max
signature.asc
Description: OpenPGP digital signature
