qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH for-2.8] qdev: apply global properties in revers


From: Cornelia Huck
Subject: Re: [Qemu-devel] [PATCH for-2.8] qdev: apply global properties in reverse order
Date: Mon, 5 Dec 2016 18:25:55 +0100

On Mon, 5 Dec 2016 14:48:29 -0200
Eduardo Habkost <address@hidden> 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 <address@hidden> 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 <address@hidden>
> > > ---
> > > 
> > > 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.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]