|
[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 Hi Daniel, The "is_available" is used for dealing with the QOM name conflict, that is before the calling to MachineClass call or any other class_init. > 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
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |