On Mon, 5 Dec 2016 14:48:29 -0200 Eduardo Habkost <ehabk...@redhat.com> wrote:
> On Mon, Dec 05, 2016 at 04:42:00PM +0100, Cornelia Huck wrote: > > On Mon, 05 Dec 2016 16:21:22 +0100 > > Greg Kurz <gr...@kaod.org> wrote: > > > > > The current code recursively applies global properties from child up to > > > parent. So, if you have: > > > > > > -global virtio-pci.disable-modern=on > > > -global virtio-blk-pci.disable-modern=off > > > > > > Then the default value of disable-modern for a virtio-blk-pci device is > > > on, > > > which looks wrong from an OOP perspective. > > > > > > This patch reverses the logic, so that a child property always prevail. > > > > This sounds reasonable... > > > > > > > > This fixes a subtle bug that got introduced in 2.7 with commit > > > "9a4c0e220d8a > > > hw/virtio-pci: fix virtio behaviour" for older (< 2.7) machine types: the > > > HW_COMPAT_2_6 macro contains global virtio-pci.disable-* properties which > > > would silently override global properties passed on the command line for > > > virtio subtypes. > > > > > > Signed-off-by: Greg Kurz <gr...@kaod.org> > > > --- > > > > > > AFAIK, libvirt's XML doesn't know about modern/legacy modes for virtio > > > devices. Early adopters of virtio 1.0 had to rely on the > > > <qemu:commandline> > > > tag to pass global properties to QEMU. This patch ensures that XML files > > > used with older machine types remain valid with newer versions of QEMU. > > > > > > FWIW I guess it could help to have this fix in 2.8, and also probably in > > > 2.7.1. > > > > ...but I'm a bit worried about doing that change this late in the > > cycle, as we may introduce subtle changes for other configurations. At > > the very least, we should look over the existing backwards compat > > properties (I'll look at those I'm familiar with). > > This patch would change the behavior for: > -global virtio-blk-pci.disable-modern=on > -global virtio-pci.disable-modern=off > > And I am not sure the new behavior would be correct. Shouldn't we > apply the properties in the order specified in the command-line? Probably; but how should this interact with compat props? > > On either case, changing the semantics of the command-line can > break existing configurations. Let's do it more carefully in the > 2.9 cycle, and fix the existing bug by changing the HW_COMPAT_* > macros? Changing the compat props is probably the best option at this point in time. Let's take this slowly so we can come up with a reasonable solution for 2.9.