[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-stable] [PATCH for-2.8] qdev: apply global properties in rever
From: |
Greg Kurz |
Subject: |
Re: [Qemu-stable] [PATCH for-2.8] qdev: apply global properties in reverse order |
Date: |
Mon, 5 Dec 2016 20:11:56 +0100 |
On Mon, 5 Dec 2016 16:42:00 +0100
Cornelia Huck <address@hidden> 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).
>
Yeah, it's unfortunate I encountered the issue this late... I usually
pass properties to -device directly. :-\ Maybe we should set global
options per virtio-pci subtype as you suggested on IRC ?
Note: sorry for the late answer but I've just received all the mails...
> >
> > Please advise.
> >
> > hw/core/qdev-properties.c | 11 ++++++++++-
> > 1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> > index 2a8276806721..1345f489d6b1 100644
> > --- a/hw/core/qdev-properties.c
> > +++ b/hw/core/qdev-properties.c
> > @@ -1119,11 +1119,20 @@ static void
> > qdev_prop_set_globals_for_type(DeviceState *dev,
> > void qdev_prop_set_globals(DeviceState *dev)
> > {
> > ObjectClass *class = object_get_class(OBJECT(dev));
> > + GSList *class_list = NULL;
> >
> > do {
> > - qdev_prop_set_globals_for_type(dev, object_class_get_name(class));
> > + class_list = g_slist_prepend(class_list, class);
> > class = object_class_get_parent(class);
> > } while (class);
> > +
> > + do {
> > + GSList *head = class_list;
> > +
> > + qdev_prop_set_globals_for_type(dev,
> > object_class_get_name(head->data));
> > + class_list = g_slist_next(head);
> > + g_slist_free_1(head);
> > + } while (class_list);
> > }
> >
> > /* --- 64bit unsigned int 'size' type --- */
> >
>
> It is a bit unfortunate that we need a double loop here, but I don't
> see any good alternative.
>
- Re: [Qemu-stable] [PATCH for-2.8] qdev: apply global properties in reverse order, (continued)
- Re: [Qemu-stable] [Qemu-devel] [PATCH for-2.8] qdev: apply global properties in reverse order, Eduardo Habkost, 2016/12/05
- Re: [Qemu-stable] [Qemu-devel] [PATCH for-2.8] qdev: apply global properties in reverse order, Cornelia Huck, 2016/12/05
- Re: [Qemu-stable] [Qemu-devel] [PATCH for-2.8] qdev: apply global properties in reverse order, Eduardo Habkost, 2016/12/05
- Re: [Qemu-stable] [Qemu-devel] [PATCH for-2.8] qdev: apply global properties in reverse order, Halil Pasic, 2016/12/05
- Re: [Qemu-stable] [Qemu-devel] [PATCH for-2.8] qdev: apply global properties in reverse order, Greg Kurz, 2016/12/06
- Re: [Qemu-stable] [Qemu-devel] [PATCH for-2.8] qdev: apply global properties in reverse order, Halil Pasic, 2016/12/06
- Re: [Qemu-stable] [Qemu-devel] [PATCH for-2.8] qdev: apply global properties in reverse order, Greg Kurz, 2016/12/06
- Re: [Qemu-stable] [Qemu-devel] [PATCH for-2.8] qdev: apply global properties in reverse order, Halil Pasic, 2016/12/06
- Re: [Qemu-stable] [Qemu-devel] [PATCH for-2.8] qdev: apply global properties in reverse order, Greg Kurz, 2016/12/05
Re: [Qemu-stable] [PATCH for-2.8] qdev: apply global properties in reverse order,
Greg Kurz <=
Re: [Qemu-stable] [Qemu-devel] [PATCH for-2.8] qdev: apply global properties in reverse order, Daniel P. Berrange, 2016/12/05
Re: [Qemu-stable] [Qemu-devel] [PATCH for-2.8] qdev: apply global properties in reverse order, Stefan Hajnoczi, 2016/12/06