[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:55 PM Daniel P. Berrangé <berrange@xxxxxxxxxx> wrote:
>
> On Tue, Nov 03, 2026 at 09:48:44PM +0800, Yonggang Luo wrote:
> > 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.
>
> Given that we don't need to do general per-arch type filtering in
> QOM, we could just move 'is_available' out of TypeInfo, and into the

We do need general per-arch type filtering, not only the Machine,
Device also suffer from
this. So are the CPU QOM objects. Here the MR needs that
https://lore.kernel.org/qemu-devel/20261031164009.2048-1-luoyonggang@xxxxxxxxx/

And the add targets/arch_mask to
DEFINE_MACHINE_EXTENDED/DEFINE_MACHINE is for removing 'is_available'
latter, as is_available has restriction that can not works well for
target_heterogeneous


> MachineClass, so it is a regular class level property, which can be
> set in the class_init methods, and thus avoid any changes to the
> DEFINE_MACHINE macros.
>
> 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 :|
>


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



 


Rackspace

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