[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 00/15] hw: expand DEFINE_MACHINE to explicit TypeInfo registration



On Tue, Nov 3, 2026 at 9:23 PM BALATON Zoltan <balaton@xxxxxxxxxx> wrote:
>
> On Tue, 3 Nov 2026, Yonggang Luo wrote:
> > On Tue, Nov 3, 2026 at 4:16 PM Daniel P. Berrangé <berrange@xxxxxxxxxx> 
> > wrote:
> >>
> >> On Fri, Oct 30, 2026 at 05:51:44PM -0800, Pierrick Bouvier wrote:
> >>> On 10/30/2026 4:09 PM, BALATON Zoltan wrote:
> >>>> On Fri, 30 Oct 2026, Yonggang Luo wrote:
> >>>>> DEFINE_MACHINE and DEFINE_MACHINE_EXTENDED hide the TypeInfo and the
> >>>>> type_init
> >>>>> function that calls type_register_static(). A later change needs to
> >>>>> pass a
> >>>>
> >>>> That's the point of the macro to hide the boiler plate code.
> >>>>
> >>>>> target mask into that registration, and the macros have no parameter
> >>>>> for it.
> >>>>
> >>>> Couldn't you just change the macros to take that parameter instead of
> >>>> expanding it everywhere?
> >>>>
> >>>
> >>> Problem of macros is that they don't have default parameters, so you
> >>> need variants for everything:
> >>>
> >>> DEFINE_MACHINE
> >>> DEFINE_MACHINE_EXTENDED
> >>> DEFINE_MACHINE_WITH_INTERFACES
> >>> DEFINE_MACHINE_WITH_INTERFACE_ARRAY
> >>> and now would need a new variant, with variant for other existing ones.
> >>>
> >>> In the end, it's just simpler to remove this and add the boilerplate.
> >>> It's written once and never modified, so using a macro just save typing
> >>> those lines once.
> >>
> >> For each machine, it is only written once, but we add machines
> >> over time, and every time a new machine is added the code style
> >> is slightly different. We introduced all the variuos DEFINE_*
> >> macros for QOM to better ensure consistent style and usage as
> >> new code is added over time.
> >
> > I understand that the DEFINE_* macros are bloating because of this.
> > Indeed currently the only two macros used are "DEFINE_MACHINE" and
> > "DEFINE_MACHINE_EXTENDED"
>
> There are a few machines using some others but these are the most used.
> That suggests maybe we should change the convenience wrappers to match
> what machines need but DEFINE_MACHINE_EXTENDED is meant to be the one that
> allows all parameters which should be rarely needed.
>
> > ; each time people want to add a new attribute, the macros is twiced.
>
> Not necessarily. The SECURE parameter did that to preserve previous
> behaviour but if you add new parameter changing all callers (which is the
> same as expanding the macro that would also change all callers) then it's
> just a new parameter added not changing the number of DEFINE_MACHINE*
> wrappers. Rearranging the wrappers to make DEFINE_MACHINE_EXTENDED used
> less is an independent change.
>
> > And the usage of DEFINE_MACHINE_EXTENDED is rare and can be replaced
> > with DEFINE_TYPES or TypeInfo
> > and be more explicit. When we used as macro parameter, we need think
>
> But that defies the purpose of hiding the QOM boiler plate as much as
> possible. New people are confused by QOM and it's easier for them to look
> up documentation on the DEFINE_MACHINE* macros to see what parameter is
> what and not care about how it's stored in QOM TypeInfo or what is that
> at all.
>
> > what's that parameter means, and for
> > TypeInfo, we can directly know what that member is about(because we
> > are assigned with the member name).
>
> The parameter also has a name and documentation so it's not simpler to
> look up TypeInfo than look at the macro definition or docs. In fact it's
> harder to guess from TypeInfo as then you also need to get into QOM.
>
> > That's why I suggest keeping a single DEFINE_MACHINE, just like
> > DEFINE_TYPES; as they are the most
> > used macros(and simple enough), and won't suffer from changes much.
>
> That means exposing all the boiler plate for all machines other than the
> simplest case. I think it would be better to keep DEFINE_MACHINE_EXTENDED
> but maybe rethink the existing wrappers and drop unused ones and maybe add
> new ones that reduce the usage of DEFINE_MACHINE_EXTENDED but still keep
> the boiler plate hidden.

Thanks, I'll go this way, remove unused macros, and keep DEFINE_MACHINE_EXTENDED
and extend both DEFINE_MACHINE_EXTENDED DEFINE_MACHINE supports
"targets" or "arch_mask" parameter.


>
> Regards,
> BALATON Zoltan



-- 
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.