Re: [PATCH 8/9] hw/i386/pc_sysfw.c: convert pflash0 and pflash1 object props to class props

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Jul 08, 2026 at 02:50:54PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > On Wed, Jul 08, 2026 at 01:21:49PM +0200, Markus Armbruster wrote:
> >> Daniel P. Berrangé <berrange@redhat.com> writes:
> >> 
> >> > On Wed, Jul 08, 2026 at 11:16:30AM +0100, Peter Maydell wrote:
> >> >> On Fri, 3 Jul 2026 at 10:09, Mark Cave-Ayland
> >> >> <mark.caveayland@nutanix.com> wrote:
> >> >> >
> >> >> > Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
> >> >> > ---
> >> >> >  include/hw/i386/pc.h | 2 ++
> >> >> >  hw/i386/pc.c         | 8 ++++++++
> >> >> >  hw/i386/pc_sysfw.c   | 7 +------
> >> >> >  3 files changed, 11 insertions(+), 6 deletions(-)
> >> >> >
> >> >> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> >> >> > index 309de2eda1..16cb0ec01f 100644
> >> >> > --- a/include/hw/i386/pc.h
> >> >> > +++ b/include/hw/i386/pc.h
> >> >> > @@ -40,6 +40,8 @@ typedef struct PCMachineState {
> >> >> >
> >> >> >      Object *alias_pcspk;
> >> >> >      Object *alias_rtc_time;
> >> >> > +    Object *alias_pflash0;
> >> >> > +    Object *alias_pflash1;
> >> >> >
> >> >> >      /* Configuration options: */
> >> >> >      uint64_t max_ram_below_4g;
> >> >> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> >> >> > index 13c9c1bfc8..11eef176f4 100644
> >> >> > --- a/hw/i386/pc.c
> >> >> > +++ b/hw/i386/pc.c
> >> >> > @@ -1762,6 +1762,14 @@ static void pc_machine_class_init(ObjectClass *oc, const void *data)
> >> >> >                                     offsetof(X86MachineState, acpi_dev),
> >> >> >                                     object_property_allow_set_link,
> >> >> >                                     OBJ_PROP_LINK_STRONG);
> >> >> > +    object_class_property_add_alias(oc, "pflash0",
> >> >> > +                                    offsetof(PCMachineState, alias_pflash0),
> >> >> > +                                    TYPE_PFLASH_CFI01,
> >> >> > +                                    "drive");
> >> >> > +    object_class_property_add_alias(oc, "pflash1",
> >> >> > +                                    offsetof(PCMachineState, alias_pflash1),
> >> >> > +                                    TYPE_PFLASH_CFI01,
> >> >> > +                                    "drive");
> >> >> 
> >> >> Does making these class properties fix
> >> >> https://gitlab.com/qemu-project/qemu/-/work_items/3254
> >> >> (which is a report that "qemu-system-x86_64 -machine pc-q35,help"
> >> >> doesn't list these properties) ?
> >> 
> >> Not showing object properties is a bug.
> >> 
> >> Cannot be fixed for object properties that are created outside the
> >> object's instance_init().
> >> 
> >> Now, object properties created in instance_init() should almost
> >> certainly be class properties instead.  Converting them all fixes the
> >> fixable part of the bug:
> >> 
> >> > Yes, it ought to, as that CLI logic iterates over class properties.
> >> 
> >> One property conversion at a time.
> >> 
> >> There's a quicker fix, though.
> >> 
> >> -machine TYPE.help uses type_print_class_properties() to show
> >> properties.  Here's its loop:
> >> 
> >>     object_class_property_iter_init(&iter, klass);
> >>     while ((prop = object_property_iter_next(&iter))) {
> >>         if (!prop->set) {
> >>             continue;
> >>         }
> >> 
> >>         g_ptr_array_add(array,
> >>                         object_property_help(prop->name, prop->type,
> >>                                              prop->defval, prop->description));
> >>     }
> >> 
> >> -device TYPE,help uses qmp_device_list_properties() to show both.
> >> Here's its loop:
> >> 
> >>         obj = object_new_with_class(klass);
> >> 
> >>         object_property_iter_init(&iter, obj);
> >>         while ((prop = object_property_iter_next(&iter))) {
> >>             ObjectPropertyInfo *info;
> >> 
> >>             [...]
> >> 
> >>             info = g_new0(ObjectPropertyInfo, 1);
> >>             info->name = g_strdup(prop->name);
> >>             info->type = g_strdup(prop->type);
> >>             info->description = g_strdup(prop->description);
> >>             info->default_value = qobject_ref(prop->defval);
> >> 
> >>             QAPI_LIST_PREPEND(prop_list, info);
> >>         }
> >> 
> >>         object_unref(obj);
> >> 
> >> Is there any excuse not to do it this way?
> >
> > Something would need to validate that instantiating a machine type
> > for help usage won't trip over any edge cases. Could be done with
> > unit tests.
> 
> tests/qtest/device-introspect-test.c covers this for devices.

Ah, good, I was misled by its name into thinking it was only devices,
not machine types too.

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|