Code review comment for ~rafaeldtinoco/ubuntu/+source/qemu:lp1828495-bionic-devel-qemu

> => Both changes to pc_compat_3_1 mean that on anything 3.1 and early MPX is on
> So if you run (even on a newer qemu) such a type and get it from an earlier
> machine then it would e.g. have MPX on even it was dropped.
> I think the right way to do this is to add all these changes that were done to
> pc_compat_3_1 in the upstream patches to the respective newest types of the
> release.
> Because we change the code with the patches, and that means we'd otherwise
> change behavior.
> Only >3.1 is it ok to really have MPX off by default.
>
> That would mean we move those changes to
> Bionic - pc_compat_2_11
> Cosmic - pc_compat_2_12
> Disco - we keep 3.1 as that is Disco atm.
>
> That would ensure that the type doesn't change until a release post >=4.x
> And even on a migration between those the old Bionic qemu would have MPX off
> (a 2.11 type) and the receiving qemu would know that (it knows all <4.0 are
> that way).
>
> TODO:
> - add ecb85fe48cacb2f8740186e81f2f38a2e02bd963 to the patches that we add for
> this
> - do not drop the changes to pc_compat_3_1 but instead move it to the most
> recent compat of the qemu versions used
> - 4c257911dcc7 / #15
> - b0a1980384fc / #16

Alright, just a quick observation.. since PC_COMPAT_2_11 doesn't exist yet, the biggest COMPAT definition being used in 2.11 is "2.10", we should use it, instead of using "2.11", no ? Because defining a PC_COMPAT_2_11 in Bionic's 2.11 won't have any effect without changes to the 2 functions bellow. Changing PC_COMPAT_2_10 would mean that ANYTHING before our 2.11 has mpx on, right ?

This can be seen in:

static void pc_i440fx_2_10_machine_options(MachineClass *m)

and

static void pc_q35_2_10_machine_options(MachineClass *m)

as

    SET_MACHINE_COMPAT(m, PC_COMPAT_2_10);

So, if we are saying that "mlx" is "off" from 2.11 release and beyond, with included patch ecb85fe48cac, then it should be "on" for the previous releases (2.10 and before), declared in Bionic, Cosmic and Disco as:

BIONIC: PC_COMPAT_2_10: mpx = on.
COSMIC: PC_COMPAT_2_10: mpx = on.
DISCO: PC_COMPAT_2_10: mpx = on.

Is that so ? Could you confirm ? I'm preparing the patches like this but I can change later if needed.

POSSIBLE SCENARIOS

- START VM in Bionic (2.11), mpx=off by default.
- MIGRATE VM to Cosmic (2.12) mpx=off by default.

- START VM in Artful (2.10) or before, mpx=on by default.
- MIGRATE VM to Bionic. PC_COMPAT_2_10 takes a play here. mpx stays off.

- START VM in Cosmic (2.12), mpx=off by default.
- MIGRATE VM to Artful (2.10) or before <- can't do.