[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 00/44] Make qdev static property API usable by any QOM typ
From: |
Eduardo Habkost |
Subject: |
Re: [PATCH v2 00/44] Make qdev static property API usable by any QOM type |
Date: |
Fri, 6 Nov 2020 16:10:34 -0500 |
On Fri, Nov 06, 2020 at 10:50:19AM -0500, Eduardo Habkost wrote:
> On Fri, Nov 06, 2020 at 10:45:11AM +0100, Kevin Wolf wrote:
> > Am 04.11.2020 um 16:59 hat Eduardo Habkost geschrieben:
> > > This series refactor the qdev property code so the static
> > > property system can be used by any QOM type. As an example, at
> > > the end of the series some properties in TYPE_MACHINE are
> > > converted to static properties to demonstrate the new API.
> > >
> > > Changes v1 -> v2
> > > ----------------
> > >
> > > * Rename functions and source files to call the new feature
> > > "field property" instead of "static property"
> > >
> > > * Change the API signature from an array-based interface similar
> > > to device_class_set_props() to a object_property_add()-like
> > > interface.
> > >
> > > This means instead of doing this:
> > >
> > > object_class_property_add_static_props(oc, my_array_of_props);
> > >
> > > properties are registered like this:
> > >
> > > object_class_property_add_field(oc, "property-name"
> > > PROP_XXX(MyState, my_field),
> > > prop_allow_set_always);
> > >
> > > where PROP_XXX is a PROP_* macro like PROP_STRING, PROP_BOOL,
> > > etc.
> >
> > In comparison, I really like the resulting code from the array based
> > interface in v1 better.
> >
> > I think it's mostly for two reasons: First, the array is much more
> > compact and easier to read. And maybe even more importantly, you know
> > it's static data and only static data. The function based interface can
> > be mixed with other code or the parameter list can contain calls to
> > other functions with side effects, so there are a lot more opportunities
> > for surprises.
>
> This is a really good point, and I strongly agree with it.
> Letting code do funny tricks at runtime is one of the reasons QOM
> properties became hard to introspect.
>
> >
> > What I didn't like about the v1 interface is that there is still a
> > separate object_class_property_set_description() for each property, but
> > I think that could have been fixed by moving the description to the
> > definitions in the array, too.
>
> This would be very easy to implement.
This was implemented at:
https://gitlab.com/ehabkost/qemu/-/commits/work/qdev-make-generic
This is the interface I'd like to submit as v3:
static Property machine_props[] = {
DEFINE_PROP_STRING("kernel", MachineState, kernel_filename,
.description = "Linux kernel image file"),
DEFINE_PROP_STRING("initrd", MachineState, initrd_filename,
.description = "Linux initial ramdisk file"),
DEFINE_PROP_STRING("append", MachineState, kernel_cmdline,
.description = "Linux kernel command line"),
DEFINE_PROP_STRING("dtb", MachineState, dtb,
.description = "Linux kernel device tree file"),
DEFINE_PROP_STRING("dumpdtb", MachineState, dumpdtb,
.description = "Dump current dtb to a file and quit"),
DEFINE_PROP_STRING("dt-compatible", MachineState, dt_compatible,
.description = "Overrides the \"compatible\" "
"property of the dt root node"),
DEFINE_PROP_STRING("firmware", MachineState, firmware,
.description = "Firmware image"),
DEFINE_PROP_STRING("memory-backend", MachineState, ram_memdev_id,
.description = "ID of memory backend object"),
DEFINE_PROP_BOOL("dump-guest-core", MachineState, dump_guest_core, true,
.description = "Include guest memory in a core dump"),
DEFINE_PROP_BOOL("mem-merge", MachineState, mem_merge, true,
.description = "Enable/disable memory merge support"),
DEFINE_PROP_BOOL("graphics", MachineState, enable_graphics, true,
.description = "Set on/off to enable/disable graphics
emulation"),
DEFINE_PROP_BOOL("suppress-vmdesc", MachineState, suppress_vmdesc, false,
.description = "Set on to disable self-describing
migration"),
DEFINE_PROP_UINT32("phandle-start", MachineState, phandle_start, 0,
.description = "The first phandle ID we may generate
dynamically"),
DEFINE_PROP_END_OF_LIST(),
};
static void machine_class_init(ObjectClass *oc, void *data)
{
...
object_class_add_field_properties(oc, machine_props, prop_allow_set_always);
...
}
>
> >
> > On Fri, Oct 30, 2020 at 06:10:34PM +0100, Paolo Bonzini wrote:
> > > On 29/10/20 23:02, Eduardo Habkost wrote:
> > > > +static Property machine_props[] = {
> > > > + DEFINE_PROP_STRING("kernel", MachineState, kernel_filename),
> > > > + DEFINE_PROP_STRING("initrd", MachineState, initrd_filename),
> > > > + DEFINE_PROP_STRING("append", MachineState, kernel_cmdline),
> > > > + DEFINE_PROP_STRING("dtb", MachineState, dtb),
> > > > + DEFINE_PROP_STRING("dumpdtb", MachineState, dumpdtb),
> > > > + DEFINE_PROP_STRING("dt-compatible", MachineState, dt_compatible),
> > > > + DEFINE_PROP_STRING("firmware", MachineState, firmware),
> > > > + DEFINE_PROP_STRING("memory-backend", MachineState, ram_memdev_id),
> > > > + DEFINE_PROP_END_OF_LIST(),
> > > > +};
> > > > +
> > >
> > > While I think generalizing the _code_ for static properties is obviously
> > > a good idea, I am not sure about generalizing the interface for adding
> > > them.
> > >
> > > The reason is that we already have a place for adding properties in
> > > class_init, and having a second makes things "less local", so to speak.
> >
> > As long as you have the function call to apply the properites array in
> > .class_init, it should be obvious enough what you're doing.
> >
> > Of course, I think we should refrain from mixing both styles in a single
> > object, but generally speaking the array feels so much better that I
> > don't think we should reject it just because QOM only had a different
> > interface so far (and the property array is preexisting in qdev, too, so
> > we already have differences between objects - in fact, the majority of
> > objects is probably qdev today).
>
> This is also a strong argument. The QEMU code base has ~500
> matches for "object*_property_add*" calls, and ~2100 for
> "DEFINE_PROP*".
>
> Converting qdev arrays to object_class_property_add_*() calls
> would be a huge effort with no gains. The end result would be
> two different APIs for registering class field properties
> coexisting for a long time, and people still confused on what's
> the preferred API.
>
> --
> Eduardo
--
Eduardo
- [PATCH v2 37/44] qdev: Move qdev_prop_tpm declaration to tpm_prop.h, (continued)
- [PATCH v2 37/44] qdev: Move qdev_prop_tpm declaration to tpm_prop.h, Eduardo Habkost, 2020/11/04
- [PATCH v2 25/44] qdev: Separate generic and device-specific property registration, Eduardo Habkost, 2020/11/04
- [PATCH v2 35/44] qdev: Rename qdev_propinfo_* to field_prop_*, Eduardo Habkost, 2020/11/04
- [PATCH v2 42/44] qom: Include static property API reference in documentation, Eduardo Habkost, 2020/11/04
- [PATCH v2 36/44] qdev: Rename qdev_get_prop_ptr() to object_field_prop_ptr(), Eduardo Habkost, 2020/11/04
- Re: [PATCH v2 00/44] Make qdev static property API usable by any QOM type, no-reply, 2020/11/04
- Re: [PATCH v2 00/44] Make qdev static property API usable by any QOM type, Kevin Wolf, 2020/11/06
- Re: [PATCH v2 00/44] Make qdev static property API usable by any QOM type, Eduardo Habkost, 2020/11/06
- Re: [PATCH v2 00/44] Make qdev static property API usable by any QOM type,
Eduardo Habkost <=
- Re: [PATCH v2 00/44] Make qdev static property API usable by any QOM type, Paolo Bonzini, 2020/11/08
- Re: [PATCH v2 00/44] Make qdev static property API usable by any QOM type, Kevin Wolf, 2020/11/09
- Re: [PATCH v2 00/44] Make qdev static property API usable by any QOM type, Paolo Bonzini, 2020/11/09
- Re: [PATCH v2 00/44] Make qdev static property API usable by any QOM type, Eduardo Habkost, 2020/11/09
- Re: [PATCH v2 00/44] Make qdev static property API usable by any QOM type, Paolo Bonzini, 2020/11/09
- Re: [PATCH v2 00/44] Make qdev static property API usable by any QOM type, Eduardo Habkost, 2020/11/09
- Re: [PATCH v2 00/44] Make qdev static property API usable by any QOM type, Paolo Bonzini, 2020/11/09
- Re: [PATCH v2 00/44] Make qdev static property API usable by any QOM type, Eduardo Habkost, 2020/11/09
- Re: [PATCH v2 00/44] Make qdev static property API usable by any QOM type, Paolo Bonzini, 2020/11/09
- Re: [PATCH v2 00/44] Make qdev static property API usable by any QOM type, Eduardo Habkost, 2020/11/09