Am 03.11.2021 um 22:26 hat Paolo Bonzini geschrieben: > On 11/3/21 18:29, Kevin Wolf wrote: > > This series adds QOM class definitions to the QAPI schema, introduces > > a new TypeInfo.instance_config() callback that configures the object at > > creation time (instead of setting properties individually) and is > > separate from runtime property setters (which often used to be not > > really tested for runtime use), and finally generates a marshalling > > function for .instance_config() from the QAPI schema that makes this a > > natural C interface rather than a visitor based one. > > That's pretty cool! > > Just one question: why not always use boxed configuration? It should not > make the instance_config types any larger, and it avoids unwieldy argument > lists.
Basically for the same reason as for commands (and for consistency with commands): If you have only one or two options, then creating a separate type for them feels just a little over the top, and boxing doesn't work with implicit types. I really like the concise definitions without a separate struct like in: { 'class': 'rng-egd', 'parent': 'rng-backend', 'config': { 'chardev': 'str' } } Though maybe we could make it work by giving the implicit type another prefixed name? > Also, for the obligatory bikeshedding remark, do you have any other plans or > ideas for the colon-separated auto generated typenames? Having both the > "namespace" (qom) and the more specific use (config) before the classname is > a bit weird, compared to the existing structs like RngRandomProperties. > Especially if boxed config is more used (or becomes the only possibility), > it might be that qom:class-name:config, or even just class-name:config, make > for nicer code in the object implementation. 'qom-config:classname' isn't a type that is useful for the object implementations at all. Its only use is really storing the whole configuration temporarily in a QAPI C struct before applying it. The class implementations always want to store only their "local" config options, but 'qom-config:classname' contains those of the parent class as well. In my example conversion, I left RngProperties as a separate type that is just referenced in the class definition: { 'class': 'rng-backend', 'config': 'RngProperties' } This is what you would do when rng-backend wants to store its options in a single struct, and then it's still RngProperties. As for the naming, I chose a prefix because I think QOM doesn't restrict the characters used in class names, so suffixes could become ambiguous. The colon is just a character that is invalid in QAPI user types and looked nice to me, any other reserved character would do. Oh, and I also wanted to say something about why not just directly using the class name, which was my first idea: 'foo': 'iothread' looks more like referencing an existing iothread rather than the configuration for a new one. I wanted to leave us the option that we could possibly later take a string for such options (a QOM path) and then pass the referenced object to QMP commands as the proper QOM type. (Maybe there aren't currently many commands that take specific QOM objects, but I'm dreaming of a future state with QMP commands that take 'block-node' arguments etc.) Kevin