| Message ID | 20240501182759.2934195-2-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | hw: define and enforce a standard lifecycle for versioned machines | expand |
On 01/05/2024 20.27, Daniel P. Berrangé wrote: > The various targets which define versioned machine types have > a bunch of obfuscated macro code for defining unique function > and variable names using string concatenation. > > This addes a couple of helpers to improve the clarity of such > code macro. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > include/hw/boards.h | 166 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 166 insertions(+) > > diff --git a/include/hw/boards.h b/include/hw/boards.h > index 2fa800f11a..47ca450fca 100644 > --- a/include/hw/boards.h > +++ b/include/hw/boards.h > @@ -414,6 +414,172 @@ struct MachineState { > struct NumaState *numa_state; > }; > > +/* > + * The macros which follow are intended to facilitate the > + * definition of versioned machine types, using a somewhat > + * similar pattern across targets: I'd suggest to add a sentence at the end saying something like this: "For example, to create the macro for setting up a versioned "virt" machine could look like this:" (otherwise it's not immediately clear whether the example is about only the "macros which follow" or whether it's about how to set up a machine type) > + * #define DEFINE_VIRT_MACHINE_IMPL(latest, ...) \ > + * static void MACHINE_VER_SYM(class_init, virt, __VA_ARGS__)( \ > + * ObjectClass *oc, \ > + * void *data) \ > + * { \ > + * MachineClass *mc = MACHINE_CLASS(oc); \ > + * MACHINE_VER_SYM(options, virt, __VA_ARGS__)(mc); \ > + * mc->desc = "QEMU " MACHINE_VER_STR(__VA_ARGS__) " Virtual Machine"; \ > + * if (latest) { \ > + * mc->alias = "virt"; \ > + * } \ > + * } \ > + * static const TypeInfo MACHINE_VER_SYM(info, virt, __VA_ARGS__) = { \ > + * .name = MACHINE_VER_TYPE_NAME("virt", __VA_ARGS__), \ > + * .parent = TYPE_VIRT_MACHINE, \ > + * .class_init = MACHINE_VER_SYM(class_init, virt, __VA_ARGS__), \ > + * }; \ > + * static void MACHINE_VER_SYM(register, virt, __VA_ARGS__)(void) \ > + * { \ > + * type_register_static(&MACHINE_VER_SYM(info, virt, __VA_ARGS__)); \ > + * } \ > + * type_init(MACHINE_VER_SYM(register, virt, __VA_ARGS__)); > + * > + * Following this, one (or more) helpers can be added for > + * whichever scenarios need to be catered for with a machine: > + * > + * // Normal 2 digit, marked as latest eg 'virt-9.0' > + * #define DEFINE_VIRT_MACHINE_LATEST(major, minor) \ > + * DEFINE_VIRT_MACHINE_IMPL(true, major, minor) > + * > + * // Normal 2 digit eg 'virt-9.0' > + * #define DEFINE_VIRT_MACHINE(major, minor) \ > + * DEFINE_VIRT_MACHINE_IMPL(false, major, minor) > + * > + * // Bugfix 3 digit eg 'virt-9.0.1' > + * #define DEFINE_VIRT_MACHINE_BUGFIX(major, minor, micro) \ > + * DEFINE_VIRT_MACHINE_IMPL(false, major, minor, micro) > + * > + * // Tagged 2 digit eg 'virt-9.0-extra' > + * #define DEFINE_VIRT_MACHINE_TAGGED(major, minor, tag) \ > + * DEFINE_VIRT_MACHINE_IMPL(false, major, minor, _, tag) > + * > + * // Tagged bugffix 2 digit eg 'virt-9.0.1-extra' s/bugffix/bugfix/ > + * #define DEFINE_VIRT_MACHINE_TAGGED(major, minor, micro, tag) \ > + * DEFINE_VIRT_MACHINE_IMPL(false, major, minor, micro, _, tag) > + */ I'd suggest to add a separate comment for the macro below, explaining that it is supposed to be used with __VA_ARGS__ to pick a certain other macro depending on the amount of entries in __VA_ARGS__. > +#define _MACHINE_VER_PICK(x1, x2, x3, x4, x5, x6, ...) x6 > + > +/* > + * Construct a human targetted machine version string. s/targetted/targeted/ according to my spell checker ? Apart from the nits: Reviewed-by: Thomas Huth <thuth@redhat.com> Thomas
On Wed, May 01, 2024 at 07:27:46PM +0100, Daniel P. Berrangé wrote: > The various targets which define versioned machine types have > a bunch of obfuscated macro code for defining unique function > and variable names using string concatenation. > > This addes a couple of helpers to improve the clarity of such > code macro. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > include/hw/boards.h | 166 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 166 insertions(+) > > diff --git a/include/hw/boards.h b/include/hw/boards.h > index 2fa800f11a..47ca450fca 100644 > --- a/include/hw/boards.h > +++ b/include/hw/boards.h > @@ -414,6 +414,172 @@ struct MachineState { > struct NumaState *numa_state; > }; > > +/* > + * The macros which follow are intended to facilitate the > + * definition of versioned machine types, using a somewhat > + * similar pattern across targets: > + * > + * #define DEFINE_VIRT_MACHINE_IMPL(latest, ...) \ > + * static void MACHINE_VER_SYM(class_init, virt, __VA_ARGS__)( \ > + * ObjectClass *oc, \ > + * void *data) \ > + * { \ > + * MachineClass *mc = MACHINE_CLASS(oc); \ > + * MACHINE_VER_SYM(options, virt, __VA_ARGS__)(mc); \ Nice to include example usage of the macros. __VA_ARGS__ is getting expanded quite a few times here, but I don't see that as being too much of a problem. > + * // Normal 2 digit eg 'virt-9.0' > + * #define DEFINE_VIRT_MACHINE(major, minor) \ > + * DEFINE_VIRT_MACHINE_IMPL(false, major, minor) > + * > + * // Bugfix 3 digit eg 'virt-9.0.1' Inconsistent on whether you are using one or two spaces before 'eg'. > + > +#define _MACHINE_VER_PICK(x1, x2, x3, x4, x5, x6, ...) x6 This helper macro is powerful; it may be worth a short comment, maybe along the lines of: /* Helper macro used to pick the right macro name based on the number * of extra arguments passed to the containing macro; usage: * * _MACHINE_VER_PICK(__VA_ARGS__, \ * MACRO_FOR_5_ARGS, \ * MACRO_FOR_4_ARGS, \ * MACRO_FOR_3_ARGS, \ * MACRO_FOR_2_ARGS)(optional prefix args, __VA_ARGS__) */ But once understood, I see it comes in handy in several places below. Reviewed-by: Eric Blake <eblake@redhat.com>
On 02/05/2024 16.57, Eric Blake wrote: > On Wed, May 01, 2024 at 07:27:46PM +0100, Daniel P. Berrangé wrote: >> The various targets which define versioned machine types have >> a bunch of obfuscated macro code for defining unique function >> and variable names using string concatenation. >> >> This addes a couple of helpers to improve the clarity of such >> code macro. >> >> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> >> --- ... >> + * // Normal 2 digit eg 'virt-9.0' >> + * #define DEFINE_VIRT_MACHINE(major, minor) \ >> + * DEFINE_VIRT_MACHINE_IMPL(false, major, minor) >> + * >> + * // Bugfix 3 digit eg 'virt-9.0.1' > > Inconsistent on whether you are using one or two spaces before 'eg'. While you're at it, maybe also better spell it with dots: e.g. Thomas
On Thu, May 02, 2024 at 12:34:49PM +0200, Thomas Huth wrote: > On 01/05/2024 20.27, Daniel P. Berrangé wrote: > > The various targets which define versioned machine types have > > a bunch of obfuscated macro code for defining unique function > > and variable names using string concatenation. > > > > This addes a couple of helpers to improve the clarity of such > > code macro. > > > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > > --- > > include/hw/boards.h | 166 ++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 166 insertions(+) > > > > diff --git a/include/hw/boards.h b/include/hw/boards.h > > index 2fa800f11a..47ca450fca 100644 > > --- a/include/hw/boards.h > > +++ b/include/hw/boards.h > > @@ -414,6 +414,172 @@ struct MachineState { > > struct NumaState *numa_state; > > }; > > +/* > > + * The macros which follow are intended to facilitate the > > + * definition of versioned machine types, using a somewhat > > + * similar pattern across targets: > > I'd suggest to add a sentence at the end saying something like this: "For > example, to create the macro for setting up a versioned "virt" machine could > look like this:" (otherwise it's not immediately clear whether the example > is about only the "macros which follow" or whether it's about how to set up > a machine type) Yes, will do > > > + * #define DEFINE_VIRT_MACHINE_IMPL(latest, ...) \ > > + * static void MACHINE_VER_SYM(class_init, virt, __VA_ARGS__)( \ > > + * ObjectClass *oc, \ > > + * void *data) \ > > + * #define DEFINE_VIRT_MACHINE_TAGGED(major, minor, micro, tag) \ > > + * DEFINE_VIRT_MACHINE_IMPL(false, major, minor, micro, _, tag) > > + */ > > I'd suggest to add a separate comment for the macro below, explaining that > it is supposed to be used with __VA_ARGS__ to pick a certain other macro > depending on the amount of entries in __VA_ARGS__. Yep, Eric had the same suggestion > > > +#define _MACHINE_VER_PICK(x1, x2, x3, x4, x5, x6, ...) x6 > > + > > +/* > > + * Construct a human targetted machine version string. > > s/targetted/targeted/ according to my spell checker ? > > Apart from the nits: > Reviewed-by: Thomas Huth <thuth@redhat.com> > > Thomas > With regards, Daniel
On Thu, May 02, 2024 at 09:57:21AM -0500, Eric Blake wrote: > On Wed, May 01, 2024 at 07:27:46PM +0100, Daniel P. Berrangé wrote: > > The various targets which define versioned machine types have > > a bunch of obfuscated macro code for defining unique function > > and variable names using string concatenation. > > > > This addes a couple of helpers to improve the clarity of such > > code macro. > > > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > > --- > > include/hw/boards.h | 166 ++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 166 insertions(+) > > > > diff --git a/include/hw/boards.h b/include/hw/boards.h > > index 2fa800f11a..47ca450fca 100644 > > --- a/include/hw/boards.h > > +++ b/include/hw/boards.h > > @@ -414,6 +414,172 @@ struct MachineState { > > struct NumaState *numa_state; > > }; > > > > +/* > > + * The macros which follow are intended to facilitate the > > + * definition of versioned machine types, using a somewhat > > + * similar pattern across targets: > > + * > > + * #define DEFINE_VIRT_MACHINE_IMPL(latest, ...) \ > > + * static void MACHINE_VER_SYM(class_init, virt, __VA_ARGS__)( \ > > + * ObjectClass *oc, \ > > + * void *data) \ > > + * { \ > > + * MachineClass *mc = MACHINE_CLASS(oc); \ > > + * MACHINE_VER_SYM(options, virt, __VA_ARGS__)(mc); \ > > Nice to include example usage of the macros. __VA_ARGS__ is getting > expanded quite a few times here, but I don't see that as being too > much of a problem. > > > + * // Normal 2 digit eg 'virt-9.0' > > + * #define DEFINE_VIRT_MACHINE(major, minor) \ > > + * DEFINE_VIRT_MACHINE_IMPL(false, major, minor) > > + * > > + * // Bugfix 3 digit eg 'virt-9.0.1' > > Inconsistent on whether you are using one or two spaces before 'eg'. Will fix > > > + > > +#define _MACHINE_VER_PICK(x1, x2, x3, x4, x5, x6, ...) x6 > > This helper macro is powerful; it may be worth a short comment, maybe > along the lines of: > > /* Helper macro used to pick the right macro name based on the number > * of extra arguments passed to the containing macro; usage: > * > * _MACHINE_VER_PICK(__VA_ARGS__, \ > * MACRO_FOR_5_ARGS, \ > * MACRO_FOR_4_ARGS, \ > * MACRO_FOR_3_ARGS, \ > * MACRO_FOR_2_ARGS)(optional prefix args, __VA_ARGS__) > */ Yes, will add something like this > > But once understood, I see it comes in handy in several places below. > > Reviewed-by: Eric Blake <eblake@redhat.com> > > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. > Virtualization: qemu.org | libguestfs.org > With regards, Daniel
diff --git a/include/hw/boards.h b/include/hw/boards.h index 2fa800f11a..47ca450fca 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -414,6 +414,172 @@ struct MachineState { struct NumaState *numa_state; }; +/* + * The macros which follow are intended to facilitate the + * definition of versioned machine types, using a somewhat + * similar pattern across targets: + * + * #define DEFINE_VIRT_MACHINE_IMPL(latest, ...) \ + * static void MACHINE_VER_SYM(class_init, virt, __VA_ARGS__)( \ + * ObjectClass *oc, \ + * void *data) \ + * { \ + * MachineClass *mc = MACHINE_CLASS(oc); \ + * MACHINE_VER_SYM(options, virt, __VA_ARGS__)(mc); \ + * mc->desc = "QEMU " MACHINE_VER_STR(__VA_ARGS__) " Virtual Machine"; \ + * if (latest) { \ + * mc->alias = "virt"; \ + * } \ + * } \ + * static const TypeInfo MACHINE_VER_SYM(info, virt, __VA_ARGS__) = { \ + * .name = MACHINE_VER_TYPE_NAME("virt", __VA_ARGS__), \ + * .parent = TYPE_VIRT_MACHINE, \ + * .class_init = MACHINE_VER_SYM(class_init, virt, __VA_ARGS__), \ + * }; \ + * static void MACHINE_VER_SYM(register, virt, __VA_ARGS__)(void) \ + * { \ + * type_register_static(&MACHINE_VER_SYM(info, virt, __VA_ARGS__)); \ + * } \ + * type_init(MACHINE_VER_SYM(register, virt, __VA_ARGS__)); + * + * Following this, one (or more) helpers can be added for + * whichever scenarios need to be catered for with a machine: + * + * // Normal 2 digit, marked as latest eg 'virt-9.0' + * #define DEFINE_VIRT_MACHINE_LATEST(major, minor) \ + * DEFINE_VIRT_MACHINE_IMPL(true, major, minor) + * + * // Normal 2 digit eg 'virt-9.0' + * #define DEFINE_VIRT_MACHINE(major, minor) \ + * DEFINE_VIRT_MACHINE_IMPL(false, major, minor) + * + * // Bugfix 3 digit eg 'virt-9.0.1' + * #define DEFINE_VIRT_MACHINE_BUGFIX(major, minor, micro) \ + * DEFINE_VIRT_MACHINE_IMPL(false, major, minor, micro) + * + * // Tagged 2 digit eg 'virt-9.0-extra' + * #define DEFINE_VIRT_MACHINE_TAGGED(major, minor, tag) \ + * DEFINE_VIRT_MACHINE_IMPL(false, major, minor, _, tag) + * + * // Tagged bugffix 2 digit eg 'virt-9.0.1-extra' + * #define DEFINE_VIRT_MACHINE_TAGGED(major, minor, micro, tag) \ + * DEFINE_VIRT_MACHINE_IMPL(false, major, minor, micro, _, tag) + */ + +#define _MACHINE_VER_PICK(x1, x2, x3, x4, x5, x6, ...) x6 + +/* + * Construct a human targetted machine version string. + * + * Can be invoked with various signatures + * + * MACHINE_VER_STR(sym, prefix, major, minor) + * MACHINE_VER_STR(sym, prefix, major, minor, micro) + * MACHINE_VER_STR(sym, prefix, major, minor, _, tag) + * MACHINE_VER_STR(sym, prefix, major, minor, micro, _, tag) + * + * Respectively emitting symbols with the format + * + * "{major}.{minor}" + * "{major}.{minor}-{tag}" + * "{major}.{minor}.{micro}" + * "{major}.{minor}.{micro}-{tag}" + */ +#define _MACHINE_VER_STR2(major, minor) \ + #major "." #minor + +#define _MACHINE_VER_STR3(major, minor, micro) \ + #major "." #minor "." #micro + +#define _MACHINE_VER_STR4(major, minor, _unused_, tag) \ + #major "." #minor "-" #tag + +#define _MACHINE_VER_STR5(major, minor, micro, _unused_, tag) \ + #major "." #minor "." #micro "-" #tag + +#define MACHINE_VER_STR(...) \ + _MACHINE_VER_PICK(__VA_ARGS__, \ + _MACHINE_VER_STR5, \ + _MACHINE_VER_STR4, \ + _MACHINE_VER_STR3, \ + _MACHINE_VER_STR2) (__VA_ARGS__) + + +/* + * Construct a QAPI type name for a versioned machine + * type + * + * Can be invoked with various signatures + * + * MACHINE_VER_TYPE_NAME(prefix, major, minor) + * MACHINE_VER_TYPE_NAME(prefix, major, minor, micro) + * MACHINE_VER_TYPE_NAME(prefix, major, minor, _, tag) + * MACHINE_VER_TYPE_NAME(prefix, major, minor, micro, _, tag) + * + * Respectively emitting symbols with the format + * + * "{prefix}-{major}.{minor}" + * "{prefix}-{major}.{minor}.{micro}" + * "{prefix}-{major}.{minor}-{tag}" + * "{prefix}-{major}.{minor}.{micro}-{tag}" + */ +#define _MACHINE_VER_TYPE_NAME2(prefix, major, minor) \ + prefix "-" #major "." #minor TYPE_MACHINE_SUFFIX + +#define _MACHINE_VER_TYPE_NAME3(prefix, major, minor, micro) \ + prefix "-" #major "." #minor "." #micro TYPE_MACHINE_SUFFIX + +#define _MACHINE_VER_TYPE_NAME4(prefix, major, minor, _unused_, tag) \ + prefix "-" #major "." #minor "-" #tag TYPE_MACHINE_SUFFIX + +#define _MACHINE_VER_TYPE_NAME5(prefix, major, minor, micro, _unused_, tag) \ + prefix "-" #major "." #minor "." #micro "-" #tag TYPE_MACHINE_SUFFIX + +#define MACHINE_VER_TYPE_NAME(prefix, ...) \ + _MACHINE_VER_PICK(__VA_ARGS__, \ + _MACHINE_VER_TYPE_NAME5, \ + _MACHINE_VER_TYPE_NAME4, \ + _MACHINE_VER_TYPE_NAME3, \ + _MACHINE_VER_TYPE_NAME2) (prefix, __VA_ARGS__) + +/* + * Construct a name for a versioned machine type that is + * suitable for use as a C symbol (function/variable/etc). + * + * Can be invoked with various signatures + * + * MACHINE_VER_SYM(sym, prefix, major, minor) + * MACHINE_VER_SYM(sym, prefix, major, minor, micro) + * MACHINE_VER_SYM(sym, prefix, major, minor, _, tag) + * MACHINE_VER_SYM(sym, prefix, major, minor, micro, _, tag) + * + * Respectively emitting symbols with the format + * + * {prefix}_machine_{major}_{minor}_{sym} + * {prefix}_machine_{major}_{minor}_{micro}_{sym} + * {prefix}_machine_{major}_{minor}_{tag}_{sym} + * {prefix}_machine_{major}_{minor}_{micro}_{tag}_{sym} + */ +#define _MACHINE_VER_SYM2(sym, prefix, major, minor) \ + prefix ## _machine_ ## major ## _ ## minor ## _ ## sym + +#define _MACHINE_VER_SYM3(sym, prefix, major, minor, micro) \ + prefix ## _machine_ ## major ## _ ## minor ## _ ## micro ## _ ## sym + +#define _MACHINE_VER_SYM4(sym, prefix, major, minor, _unused_, tag) \ + prefix ## _machine_ ## major ## _ ## minor ## _ ## tag ## _ ## sym + +#define _MACHINE_VER_SYM5(sym, prefix, major, minor, micro, _unused_, tag) \ + prefix ## _machine_ ## major ## _ ## minor ## _ ## micro ## _ ## tag ## _ ## sym + +#define MACHINE_VER_SYM(sym, prefix, ...) \ + _MACHINE_VER_PICK(__VA_ARGS__, \ + _MACHINE_VER_SYM5, \ + _MACHINE_VER_SYM4, \ + _MACHINE_VER_SYM3, \ + _MACHINE_VER_SYM2) (sym, prefix, __VA_ARGS__) + + #define DEFINE_MACHINE(namestr, machine_initfn) \ static void machine_initfn##_class_init(ObjectClass *oc, void *data) \ { \
The various targets which define versioned machine types have a bunch of obfuscated macro code for defining unique function and variable names using string concatenation. This addes a couple of helpers to improve the clarity of such code macro. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- include/hw/boards.h | 166 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 166 insertions(+)