Am 03.05.2016 um 13:19 hat Max Reitz geschrieben: > On 02.05.2016 17:36, Kevin Wolf wrote: > > Am 26.04.2016 um 23:32 hat Max Reitz geschrieben: > >> In order to allow block drivers to use that function, it needs to be > >> public. In order to be useful, it needs to take a parameter which allows > >> the caller to specify whether the runtime options allowed by the block > >> driver are actually significant for the guest-visible BDS content. > >> > >> Signed-off-by: Max Reitz <mre...@redhat.com> > > > > Is this actually good enough? I expect that many drivers will have some > > options that are significant and other options that aren't. We already > > have some (Quorum: children are significant, rewrite-corrupted isn't), > > but as we convert more things to proper options, we'll get more of them > > (raw-posix: filename is significant, aio=native isn't). > > > > We might actually need to pass a list of significant fields instead that > > append_open_options() can use. > > Well, in theory, every driver with insignificant options would just > implement .bdrv_refresh_filename() however it's needed. Making > bdrv_default_refresh_format_filename() function public is just a way of > keeping that implementation very simple for some drivers that only have > insignificant options. > > I'm not opposed to extending this function in the future when it > actually makes sense. Right now I don't think it does. The only thing > that changes if a significant option is detected is that no plain > filename is generated; however, for Quorum we can never generate such a > filename. Therefore, we cannot use this function for Quorum anyway.
If you integrate it into append_open_options(), I suppose it would also mean that insignificant options are dropped from the json: description, i.e. Quorum would return a json: object with all children, but not the rewrite-corrupted setting. Which I think would be a good thing. Kevin > However, instead of extending this function, it may make more sense then > to introduce a new field to the BlockDriver struct which is a > NULL-terminated array of significant option names, or something like > that. If .bdrv_refresh_filename is NULL but that array pointer is not, > then the default implementation could behave accordingly. But this is > something I'd defer to the future, too, unless you can point out a > current block driver that would benefit from this functionality (I don't > think Quorum does, as I said above). > > Max
pgp53p7GQeChA.pgp
Description: PGP signature
