| Message ID | 20181107123652.23417-14-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Generalize machine compatibility properties | expand |
Hi On Wed, Nov 7, 2018 at 4:49 PM Marc-André Lureau <marcandre.lureau@redhat.com> wrote: > > The following patch is going to add compatiblity parameters for > qemu <= 3.1. > I realize this may be good enough for x86 i440/q35 machines, but what about other machines & architectures? What do we officially support, for migration, across different versions? It seems we have versionized: - arm "virt" machines - "s390-ccw-virtio" machines - ppc "pseries" machines - x86 piix/q35 machines At least, I think I should update this patch to add new 3.2 machines for those. Is there any way to check compat properties are handled properly for those various machines? It looks like it is generally lacking. For example, if there is a new HW_COMPAT, it would be nice if something failed if a corresponding machine hasn't been added. It also looks like there is a bit of code duplication and a bit too much macros :) unfortunately, I don't yet have a good idea how to improve things... > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > include/hw/compat.h | 3 +++ > include/hw/i386/pc.h | 3 +++ > hw/i386/pc_piix.c | 21 ++++++++++++++++++--- > hw/i386/pc_q35.c | 19 +++++++++++++++++-- > 4 files changed, 41 insertions(+), 5 deletions(-) > > diff --git a/include/hw/compat.h b/include/hw/compat.h > index 6f4d5fc647..70958328fe 100644 > --- a/include/hw/compat.h > +++ b/include/hw/compat.h > @@ -1,6 +1,9 @@ > #ifndef HW_COMPAT_H > #define HW_COMPAT_H > > +#define HW_COMPAT_3_1 \ > + /* empty */ > + > #define HW_COMPAT_3_0 \ > /* empty */ > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 136fe497b6..c37d4333a0 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -294,6 +294,9 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t); > int e820_get_num_entries(void); > bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *); > > +#define PC_COMPAT_3_1 \ > + HW_COMPAT_3_1 > + > #define PC_COMPAT_3_0 \ > HW_COMPAT_3_0 \ > {\ > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > index dc09466b3e..ba371bfcd7 100644 > --- a/hw/i386/pc_piix.c > +++ b/hw/i386/pc_piix.c > @@ -428,21 +428,36 @@ static void pc_i440fx_machine_options(MachineClass *m) > machine_class_allow_dynamic_sysbus_dev(m, TYPE_RAMFB_DEVICE); > } > > -static void pc_i440fx_3_0_machine_options(MachineClass *m) > +static void pc_i440fx_3_2_machine_options(MachineClass *m) > { > pc_i440fx_machine_options(m); > m->alias = "pc"; > m->is_default = 1; > } > > +DEFINE_I440FX_MACHINE(v3_2, "pc-i440fx-3.2", NULL, > + pc_i440fx_3_2_machine_options); > + > +static void pc_i440fx_3_1_machine_options(MachineClass *m) > +{ > + pc_i440fx_3_2_machine_options(m); > + m->is_default = 0; > + m->alias = NULL; > + SET_MACHINE_COMPAT(m, PC_COMPAT_3_1); > +} > + > +static void pc_i440fx_3_0_machine_options(MachineClass *m) > +{ > + pc_i440fx_3_1_machine_options(m); > + SET_MACHINE_COMPAT(m, PC_COMPAT_3_0); > +} > + > DEFINE_I440FX_MACHINE(v3_0, "pc-i440fx-3.0", NULL, > pc_i440fx_3_0_machine_options); > > static void pc_i440fx_2_12_machine_options(MachineClass *m) > { > pc_i440fx_3_0_machine_options(m); > - m->is_default = 0; > - m->alias = NULL; > SET_MACHINE_COMPAT(m, PC_COMPAT_2_12); > } > > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c > index 532241e3f8..64d6ea65d5 100644 > --- a/hw/i386/pc_q35.c > +++ b/hw/i386/pc_q35.c > @@ -311,19 +311,34 @@ static void pc_q35_machine_options(MachineClass *m) > m->max_cpus = 288; > } > > -static void pc_q35_3_0_machine_options(MachineClass *m) > +static void pc_q35_3_2_machine_options(MachineClass *m) > { > pc_q35_machine_options(m); > m->alias = "q35"; > } > > +DEFINE_Q35_MACHINE(v3_2, "pc-q35-3.2", NULL, > + pc_q35_3_2_machine_options); > + > +static void pc_q35_3_1_machine_options(MachineClass *m) > +{ > + pc_q35_3_2_machine_options(m); > + m->alias = NULL; > + SET_MACHINE_COMPAT(m, PC_COMPAT_3_1); > +} > + > +static void pc_q35_3_0_machine_options(MachineClass *m) > +{ > + pc_q35_3_1_machine_options(m); > + SET_MACHINE_COMPAT(m, PC_COMPAT_3_0); > +} > + > DEFINE_Q35_MACHINE(v3_0, "pc-q35-3.0", NULL, > pc_q35_3_0_machine_options); > > static void pc_q35_2_12_machine_options(MachineClass *m) > { > pc_q35_3_0_machine_options(m); > - m->alias = NULL; > SET_MACHINE_COMPAT(m, PC_COMPAT_2_12); > } > > -- > 2.19.1.708.g4ede3d42df > >
On Wed, Nov 07, 2018 at 07:49:54PM +0400, Marc-André Lureau wrote: > Hi > > On Wed, Nov 7, 2018 at 4:49 PM Marc-André Lureau > <marcandre.lureau@redhat.com> wrote: > > > > The following patch is going to add compatiblity parameters for > > qemu <= 3.1. > > > > I realize this may be good enough for x86 i440/q35 machines, but what > about other machines & architectures? > > What do we officially support, for migration, across different versions? > > It seems we have versionized: > - arm "virt" machines > - "s390-ccw-virtio" machines > - ppc "pseries" machines > - x86 piix/q35 machines > > At least, I think I should update this patch to add new 3.2 machines for those. This QEMU release seems to be unusual: normally we add features that require adding new machine-types long before soft freeze. If we didn't add new machine-types until now, this means we didn't introduce any guest ABI changes that require them. Now, this is an interesting case: we probably don't _need_ the new machine-types, but users might be confused if they don't see new machine-types. Should we add them anyway? > > Is there any way to check compat properties are handled properly for > those various machines? It looks like it is generally lacking. For > example, if there is a new HW_COMPAT, it would be nice if something > failed if a corresponding machine hasn't been added. > > It also looks like there is a bit of code duplication and a bit too > much macros :) unfortunately, I don't yet have a good idea how to > improve things... I dream with the day all this compatibility data will be treated like what it is: just data. The ugly macro trickery needs to go away eventually, but this requires making the QOM/machine-type APIs more practical. Removing very old machine-types will probably make this task easier, because they are the main source of compatibility code that is not represented as <driver.property=value> tuples (grep for "static void pc_compat_" and you'll see them).
diff --git a/include/hw/compat.h b/include/hw/compat.h index 6f4d5fc647..70958328fe 100644 --- a/include/hw/compat.h +++ b/include/hw/compat.h @@ -1,6 +1,9 @@ #ifndef HW_COMPAT_H #define HW_COMPAT_H +#define HW_COMPAT_3_1 \ + /* empty */ + #define HW_COMPAT_3_0 \ /* empty */ diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 136fe497b6..c37d4333a0 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -294,6 +294,9 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t); int e820_get_num_entries(void); bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *); +#define PC_COMPAT_3_1 \ + HW_COMPAT_3_1 + #define PC_COMPAT_3_0 \ HW_COMPAT_3_0 \ {\ diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index dc09466b3e..ba371bfcd7 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -428,21 +428,36 @@ static void pc_i440fx_machine_options(MachineClass *m) machine_class_allow_dynamic_sysbus_dev(m, TYPE_RAMFB_DEVICE); } -static void pc_i440fx_3_0_machine_options(MachineClass *m) +static void pc_i440fx_3_2_machine_options(MachineClass *m) { pc_i440fx_machine_options(m); m->alias = "pc"; m->is_default = 1; } +DEFINE_I440FX_MACHINE(v3_2, "pc-i440fx-3.2", NULL, + pc_i440fx_3_2_machine_options); + +static void pc_i440fx_3_1_machine_options(MachineClass *m) +{ + pc_i440fx_3_2_machine_options(m); + m->is_default = 0; + m->alias = NULL; + SET_MACHINE_COMPAT(m, PC_COMPAT_3_1); +} + +static void pc_i440fx_3_0_machine_options(MachineClass *m) +{ + pc_i440fx_3_1_machine_options(m); + SET_MACHINE_COMPAT(m, PC_COMPAT_3_0); +} + DEFINE_I440FX_MACHINE(v3_0, "pc-i440fx-3.0", NULL, pc_i440fx_3_0_machine_options); static void pc_i440fx_2_12_machine_options(MachineClass *m) { pc_i440fx_3_0_machine_options(m); - m->is_default = 0; - m->alias = NULL; SET_MACHINE_COMPAT(m, PC_COMPAT_2_12); } diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index 532241e3f8..64d6ea65d5 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -311,19 +311,34 @@ static void pc_q35_machine_options(MachineClass *m) m->max_cpus = 288; } -static void pc_q35_3_0_machine_options(MachineClass *m) +static void pc_q35_3_2_machine_options(MachineClass *m) { pc_q35_machine_options(m); m->alias = "q35"; } +DEFINE_Q35_MACHINE(v3_2, "pc-q35-3.2", NULL, + pc_q35_3_2_machine_options); + +static void pc_q35_3_1_machine_options(MachineClass *m) +{ + pc_q35_3_2_machine_options(m); + m->alias = NULL; + SET_MACHINE_COMPAT(m, PC_COMPAT_3_1); +} + +static void pc_q35_3_0_machine_options(MachineClass *m) +{ + pc_q35_3_1_machine_options(m); + SET_MACHINE_COMPAT(m, PC_COMPAT_3_0); +} + DEFINE_Q35_MACHINE(v3_0, "pc-q35-3.0", NULL, pc_q35_3_0_machine_options); static void pc_q35_2_12_machine_options(MachineClass *m) { pc_q35_3_0_machine_options(m); - m->alias = NULL; SET_MACHINE_COMPAT(m, PC_COMPAT_2_12); }
The following patch is going to add compatiblity parameters for qemu <= 3.1. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- include/hw/compat.h | 3 +++ include/hw/i386/pc.h | 3 +++ hw/i386/pc_piix.c | 21 ++++++++++++++++++--- hw/i386/pc_q35.c | 19 +++++++++++++++++-- 4 files changed, 41 insertions(+), 5 deletions(-)