Re: [Qemu-block] [PATCH 09/13] block: Add 'runtime_opts' and 'mutable_opts' fields to BlockDriver

[Kevin Wolf]

Am 01.03.2019 um 14:05 hat Alberto Garcia geschrieben:
> On Fri 01 Mar 2019 01:56:42 PM CET, Kevin Wolf wrote:
> >> >> >> diff --git a/include/block/block_int.h b/include/block/block_int.h
> >> >> >> index fd0e88d17a..e680dda86b 100644
> >> >> >> --- a/include/block/block_int.h
> >> >> >> +++ b/include/block/block_int.h
> >> >> >> @@ -345,6 +345,13 @@ struct BlockDriver {
> >> >> >>  
> >> >> >>      /* List of options for creating images, terminated by name == 
> >> >> >> NULL */
> >> >> >>      QemuOptsList *create_opts;
> >> >> >> +    /* Runtime options for a block device, terminated by name == 
> >> >> >> NULL */
> >> >> >> +    QemuOptsList *runtime_opts;
> >> >> >
> >> >> > I'm not sure if using a QemuOptsList here is a good idea. Currently,
> >> >> > we use QemuOptsLists for most options, but there are some drivers that
> >> >> > use it only for part of their options, or not at all, using direct
> >> >> > QDict accesses or QAPI objects for the rest.
> >> >> 
> >> >> My intention was to avoid having two separate lists with the runtime
> >> >> options of a driver. For this feature we really need that list to
> >> >> contain all options, otherwise there's no way to know whether a missing
> >> >> option is really missing or if it doesn't exist in the first place.
> >> >
> >> > Yes, I understand your intention and the requirement. My point is just
> >> > that the existing QemuOptsLists are often _not_ complete, so they
> >> > can't fulfill the requirement.
> >> 
> >> Perhaps a new data structure with a list of runtime options and whether
> >> they can be resetted to their default value or not.
> >
> > Basically just an array that contains names and bools, right? I think
> > that would work.
> 
> Yes exactly. My concern is that the array has to be updated by hand
> every time a new option is added to the driver, so they could easily be
> out of sync.

Hm, actually, do you even need the runtime_opts list?

You use it in bdrv_reset_options_allowed() to iterate through each
option and then check whether it was present in the old set of options,
but not in the new set of options.

In order to achieve this, you can just iterate the old options dict
because you don't do anything with options that are present in the
QemuOptsList, but not the old options dict.

Kevin