On Tue, Mar 10, 2020 at 04:24:45PM +0200, Liran Alon wrote: > > On 10/03/2020 13:22, Michael S. Tsirkin wrote: > > On Tue, Mar 10, 2020 at 01:13:21PM +0200, Liran Alon wrote: > > > On 10/03/2020 11:34, Michael S. Tsirkin wrote: > > > > On Tue, Mar 10, 2020 at 01:54:04AM +0200, Liran Alon wrote: > > > > > This is VMware documented functionallity that some guests rely on. > > > > > Returns the BIOS UUID of the current virtual machine. > > > > > > > > > > Reviewed-by: Nikita Leshenko <nikita.leshche...@oracle.com> > > > > > Signed-off-by: Liran Alon <liran.a...@oracle.com> > > > > So this at least seems guest-visible. > > > > > > > > So I suspect you need to add properties to > > > > disable this for old machine types, to avoid > > > > breaking compatibility with live-migration. > > > It is indeed guest visible. > > > In theory, you are right that for every guest-visible change, we should > > > make > > > sure to expose it to only new machine-types. > > > > > > However, in this case, I feel it just unnecessary over-complicates the > > > code. > > > I don't see how a guest which previously failed to use this command, will > > > fail because after Live-Migration it could succeed. > > The reverse can happen, start guest on a new qemu, command seems to > > work, then we migrate and it fails. > > > > And I guess this applies to the version right? > > > > > If you insist, I will add such functionality. In that case, do you think a > > > single flag will suffice for the addition of all new commands > > > (i.e. "commands-version" that it's number specifies set of commands to > > > expose), or you want to have a per-command flag? > > > > > > -Liran > > Can be a single flag but I'd just do it a boolean that enables a group > > of commands. E.g. "commands-v2". > > > Re-thinking about this... > > QEMU VMPort interface was quite broken already (See first patch in series > "hw/i386/vmport: Propagate IOPort read to vCPU EAX register"). > The introduction of that fix already changes the result of all existing > commands from guest perspective which relied on return-value from > vmport_ioport_read(). > E.g. CMD_GETVERSION and CMD_GETRAMSIZE. > > In theory, we should have also made that bug-fix be tied to machine-type. To > similarly avoid the issue of migrating a VM from a working VMPort command > implementation to a non-working one. > i.e. In case of migrating from new QEMU to old QEMU. Do we wish to create a > property-flag for that fix as-well?
Yes, I meants that too. Just include everything in the same property. > Or can we just drop all the machine-type > flags alltogether (Including the suggested "commands-v2") > and declare this the first actually working VMPort implementation? > > -Liran It's hard to be sure no one used this, and the overhead isn't big. But that would be a maintainer call. In any case you need to call this out explicitly in the commit log, and I guess block migration for old machine types.