* Re: [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6
2019-01-04 4:23 ` Michael S. Tsirkin
@ 2019-01-04 9:37 ` Cornelia Huck
2019-01-04 10:12 ` Dr. David Alan Gilbert
2019-01-04 16:08 ` Eduardo Habkost
2 siblings, 0 replies; 15+ messages in thread
From: Cornelia Huck @ 2019-01-04 9:37 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Eduardo Habkost, qemu-devel, Thomas Huth, Dr. David Alan Gilbert
On Thu, 3 Jan 2019 23:23:45 -0500
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Fri, Jan 04, 2019 at 01:22:26AM -0200, Eduardo Habkost wrote:
> > The virtio-pci entries in HW_COMPAT_2_6 had an implicit
> > assumption: that all virtio-pci subclasses support the
> > disable-legacy and disable-modern options.
> >
> > That assumption was broken by commit f6e501a28ef9 ("virtio:
> > Provide version-specific variants of virtio PCI devices"). This
> > caused QEMU to crash if using the new -non-transitional or
> > -transitional device types:
> >
> > $ qemu-system-x86_64 -machine pc-i440fx-2.6 \
> > -device virtio-net-pci-non-transitional
> > Unexpected error in object_property_find() at qom/object.c:1092:
> > qemu-system-x86_64: -device virtio-net-pci-non-transitional: can't apply \
> > global virtio-pci.disable-modern=on: Property '.disable-modern' not found
> > Aborted (core dumped)
> >
> > Replace the virtio-pci.disable-legacy=off and
> > virtio-pci.disable-modern=on entries on HW_COMPAT_2_6 with
> > explicit entries for each generic virtio device type.
> >
> > The full list of generic virtio device types was extracted by
> > just grepping for ".generic_name". Note that we don't need to
> > worry about listing new virtio-pci devices in HW_COMPAT_2_6 in
> > the future, because new devices won't require QEMU 2.6
> > compatibility.
>
> I fully expect that e.g. packed ring support will need
> to affect all virtio devices too. And it's likely
> that we'll have some new virtio-pci transport features too.
Yes, but as this post-dates the introduction of the version-specific
devices, it will all go into the base class and therefore not be
problematic.
>
> > This makes the compat entries annoyingly verbose, but is simpler
> > than the alternative of making the virtio-pci type inheritance
> > rules even more complex.
>
> God forbid we forgot something, the only way to notice is to
> run a cross version migration with an old qemu.
> I think we need to come up with something less verbose and fragile.
I'd rather prefer to do this change now, so that we don't need to drag
around some complicated infrastructure forever.
As Eduardo said, we don't need to care about new devices, so this
grep-based approach sounds like it would catch everything.
>
>
> > Reported-by: Thomas Huth <thuth@redhat.com>
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > ---
> > include/hw/compat.h | 140 +++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 138 insertions(+), 2 deletions(-)
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6
2019-01-04 4:23 ` Michael S. Tsirkin
2019-01-04 9:37 ` Cornelia Huck
@ 2019-01-04 10:12 ` Dr. David Alan Gilbert
2019-01-04 17:54 ` Eduardo Habkost
2019-01-04 16:08 ` Eduardo Habkost
2 siblings, 1 reply; 15+ messages in thread
From: Dr. David Alan Gilbert @ 2019-01-04 10:12 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Eduardo Habkost, qemu-devel, Thomas Huth, Cornelia Huck
* Michael S. Tsirkin (mst@redhat.com) wrote:
> On Fri, Jan 04, 2019 at 01:22:26AM -0200, Eduardo Habkost wrote:
> > The virtio-pci entries in HW_COMPAT_2_6 had an implicit
> > assumption: that all virtio-pci subclasses support the
> > disable-legacy and disable-modern options.
> >
> > That assumption was broken by commit f6e501a28ef9 ("virtio:
> > Provide version-specific variants of virtio PCI devices"). This
> > caused QEMU to crash if using the new -non-transitional or
> > -transitional device types:
> >
> > $ qemu-system-x86_64 -machine pc-i440fx-2.6 \
> > -device virtio-net-pci-non-transitional
> > Unexpected error in object_property_find() at qom/object.c:1092:
> > qemu-system-x86_64: -device virtio-net-pci-non-transitional: can't apply \
> > global virtio-pci.disable-modern=on: Property '.disable-modern' not found
> > Aborted (core dumped)
> >
> > Replace the virtio-pci.disable-legacy=off and
> > virtio-pci.disable-modern=on entries on HW_COMPAT_2_6 with
> > explicit entries for each generic virtio device type.
> >
> > The full list of generic virtio device types was extracted by
> > just grepping for ".generic_name". Note that we don't need to
> > worry about listing new virtio-pci devices in HW_COMPAT_2_6 in
> > the future, because new devices won't require QEMU 2.6
> > compatibility.
>
> I fully expect that e.g. packed ring support will need
> to affect all virtio devices too. And it's likely
> that we'll have some new virtio-pci transport features too.
>
> > This makes the compat entries annoyingly verbose, but is simpler
> > than the alternative of making the virtio-pci type inheritance
> > rules even more complex.
>
> God forbid we forgot something, the only way to notice is to
> run a cross version migration with an old qemu.
> I think we need to come up with something less verbose and fragile.
I guess we could use a script like tests/acceptance/virtio_version.py to
do a check?
As for something less verbose, I guess something is doable with a few
macros (more complex but less verbose); or I guess you could add these
properties to the other devices but just refuse to operate when they
were set the wrong way.
Dave
>
> > Reported-by: Thomas Huth <thuth@redhat.com>
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > ---
> > include/hw/compat.h | 140 +++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 138 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/hw/compat.h b/include/hw/compat.h
> > index 3ca85b037c..fbb6c1138a 100644
> > --- a/include/hw/compat.h
> > +++ b/include/hw/compat.h
> > @@ -163,11 +163,147 @@
> > .property = "format_transport_address",\
> > .value = "off",\
> > },{\
> > - .driver = "virtio-pci",\
> > + .driver = "vhost-scsi-pci",\
> > .property = "disable-modern",\
> > .value = "on",\
> > },{\
> > - .driver = "virtio-pci",\
> > + .driver = "vhost-scsi-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "vhost-user-blk-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "vhost-user-blk-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "vhost-user-scsi-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "vhost-user-scsi-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "vhost-vsock-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "vhost-vsock-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-9p-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-9p-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-balloon-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-balloon-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-blk-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-blk-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-crypto-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-crypto-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-gpu-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-gpu-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-input-host-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-input-host-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-keyboard-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-keyboard-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-mouse-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-mouse-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-net-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-net-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-rng-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-rng-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-scsi-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-scsi-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-serial-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-serial-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-tablet-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-tablet-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-vga",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-vga",\
> > .property = "disable-legacy",\
> > .value = "off",\
> > },
> > --
> > 2.18.0.rc1.1.g3f1ff2140
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6
2019-01-04 10:12 ` Dr. David Alan Gilbert
@ 2019-01-04 17:54 ` Eduardo Habkost
2019-01-04 20:09 ` Eduardo Habkost
0 siblings, 1 reply; 15+ messages in thread
From: Eduardo Habkost @ 2019-01-04 17:54 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: Michael S. Tsirkin, qemu-devel, Thomas Huth, Cornelia Huck
On Fri, Jan 04, 2019 at 10:12:00AM +0000, Dr. David Alan Gilbert wrote:
> * Michael S. Tsirkin (mst@redhat.com) wrote:
> > On Fri, Jan 04, 2019 at 01:22:26AM -0200, Eduardo Habkost wrote:
> > > The virtio-pci entries in HW_COMPAT_2_6 had an implicit
> > > assumption: that all virtio-pci subclasses support the
> > > disable-legacy and disable-modern options.
> > >
> > > That assumption was broken by commit f6e501a28ef9 ("virtio:
> > > Provide version-specific variants of virtio PCI devices"). This
> > > caused QEMU to crash if using the new -non-transitional or
> > > -transitional device types:
> > >
> > > $ qemu-system-x86_64 -machine pc-i440fx-2.6 \
> > > -device virtio-net-pci-non-transitional
> > > Unexpected error in object_property_find() at qom/object.c:1092:
> > > qemu-system-x86_64: -device virtio-net-pci-non-transitional: can't apply \
> > > global virtio-pci.disable-modern=on: Property '.disable-modern' not found
> > > Aborted (core dumped)
> > >
> > > Replace the virtio-pci.disable-legacy=off and
> > > virtio-pci.disable-modern=on entries on HW_COMPAT_2_6 with
> > > explicit entries for each generic virtio device type.
> > >
> > > The full list of generic virtio device types was extracted by
> > > just grepping for ".generic_name". Note that we don't need to
> > > worry about listing new virtio-pci devices in HW_COMPAT_2_6 in
> > > the future, because new devices won't require QEMU 2.6
> > > compatibility.
> >
> > I fully expect that e.g. packed ring support will need
> > to affect all virtio devices too. And it's likely
> > that we'll have some new virtio-pci transport features too.
> >
> > > This makes the compat entries annoyingly verbose, but is simpler
> > > than the alternative of making the virtio-pci type inheritance
> > > rules even more complex.
> >
> > God forbid we forgot something, the only way to notice is to
> > run a cross version migration with an old qemu.
> > I think we need to come up with something less verbose and fragile.
>
> I guess we could use a script like tests/acceptance/virtio_version.py to
> do a check?
That's a good idea. On test code we can try additional tricks to
detect the hybrid virtio devices without increasing the
complexity of QEMU code. I'll give it a try.
>
> As for something less verbose, I guess something is doable with a few
> macros (more complex but less verbose); or I guess you could add these
> properties to the other devices but just refuse to operate when they
> were set the wrong way.
Adding the properties to all virtio-pci devices is the kind of
complexity I would like to avoid. If we need to define rules for
when disable-modern/disable-legacy is set to unexpected values,
we'll have to maintain the code that implement those rules
forever.
--
Eduardo
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6
2019-01-04 17:54 ` Eduardo Habkost
@ 2019-01-04 20:09 ` Eduardo Habkost
2019-01-04 20:48 ` Michael S. Tsirkin
0 siblings, 1 reply; 15+ messages in thread
From: Eduardo Habkost @ 2019-01-04 20:09 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: Michael S. Tsirkin, qemu-devel, Thomas Huth, Cornelia Huck
On Fri, Jan 04, 2019 at 03:54:39PM -0200, Eduardo Habkost wrote:
> On Fri, Jan 04, 2019 at 10:12:00AM +0000, Dr. David Alan Gilbert wrote:
> > * Michael S. Tsirkin (mst@redhat.com) wrote:
> > > On Fri, Jan 04, 2019 at 01:22:26AM -0200, Eduardo Habkost wrote:
> > > > The virtio-pci entries in HW_COMPAT_2_6 had an implicit
> > > > assumption: that all virtio-pci subclasses support the
> > > > disable-legacy and disable-modern options.
> > > >
> > > > That assumption was broken by commit f6e501a28ef9 ("virtio:
> > > > Provide version-specific variants of virtio PCI devices"). This
> > > > caused QEMU to crash if using the new -non-transitional or
> > > > -transitional device types:
> > > >
> > > > $ qemu-system-x86_64 -machine pc-i440fx-2.6 \
> > > > -device virtio-net-pci-non-transitional
> > > > Unexpected error in object_property_find() at qom/object.c:1092:
> > > > qemu-system-x86_64: -device virtio-net-pci-non-transitional: can't apply \
> > > > global virtio-pci.disable-modern=on: Property '.disable-modern' not found
> > > > Aborted (core dumped)
> > > >
> > > > Replace the virtio-pci.disable-legacy=off and
> > > > virtio-pci.disable-modern=on entries on HW_COMPAT_2_6 with
> > > > explicit entries for each generic virtio device type.
> > > >
> > > > The full list of generic virtio device types was extracted by
> > > > just grepping for ".generic_name". Note that we don't need to
> > > > worry about listing new virtio-pci devices in HW_COMPAT_2_6 in
> > > > the future, because new devices won't require QEMU 2.6
> > > > compatibility.
> > >
> > > I fully expect that e.g. packed ring support will need
> > > to affect all virtio devices too. And it's likely
> > > that we'll have some new virtio-pci transport features too.
> > >
> > > > This makes the compat entries annoyingly verbose, but is simpler
> > > > than the alternative of making the virtio-pci type inheritance
> > > > rules even more complex.
> > >
> > > God forbid we forgot something, the only way to notice is to
> > > run a cross version migration with an old qemu.
> > > I think we need to come up with something less verbose and fragile.
> >
> > I guess we could use a script like tests/acceptance/virtio_version.py to
> > do a check?
>
> That's a good idea. On test code we can try additional tricks to
> detect the hybrid virtio devices without increasing the
> complexity of QEMU code. I'll give it a try.
I did it but I'm not happy with the result: many of the virtio
devices can't be tested without extra arguments. Some of them
(like vhost-*) require extra privileges on the host that might be
unavailable.
Anyway, while writing this I noticed another issue: many of the
virtio devices in QEMU 2.6 were already modern-only!
Setting disable-modern=off on modern-only devices like virtio-vga
or virtio-tablet-pci doesn't make sense. This means setting
virtio-pci.disable-modern=off on HW_COMPAT_2_6 was incorrect even
before the -non-transitional and -transitional device types were
introduced.
---
diff --git a/tests/acceptance/virtio_version.py b/tests/acceptance/virtio_version.py
index ce990250d8..9157a1b173 100644
--- a/tests/acceptance/virtio_version.py
+++ b/tests/acceptance/virtio_version.py
@@ -55,6 +55,18 @@ def get_pci_interfaces(vm, devtype):
interfaces = ('pci-express-device', 'conventional-pci-device')
return [i for i in interfaces if devtype_implements(vm, devtype, i)]
+def is_hybrid_dev(vm, devtype):
+ props = [p['name'] for p in vm.command('device-list-properties',
+ typename=devtype)]
+ return 'disable-legacy' in props
+
+def get_all_hybrid_devs(vm):
+ """Return list of all hybrid virtio device types (the ones that can be
+ configured using the disable-legacy & disable-modern properties)"""
+ alldevs = [d['name'] for d in vm.command('qom-list-types',
+ implements='virtio-pci')]
+ return [d for d in alldevs if is_hybrid_dev(vm, d)]
+
class VirtioVersionCheck(Test):
"""
Check if virtio-version-specific device types result in the
@@ -174,3 +186,33 @@ class VirtioVersionCheck(Test):
self.check_modern_only('virtio-mouse-pci', VIRTIO_INPUT)
self.check_modern_only('virtio-tablet-pci', VIRTIO_INPUT)
self.check_modern_only('virtio-keyboard-pci', VIRTIO_INPUT)
+
+ def get_all_hybrid_devs(self):
+ with QEMUMachine(self.qemu_bin) as vm:
+ vm.set_machine('none')
+ vm.add_args('-S')
+ vm.launch()
+ return get_all_hybrid_devs(vm)
+
+ def check_legacy_compat(self, qemu_devtype):
+ # these device types can't be run with extra arguments, so we can't
+ # test them directly:
+ if qemu_devtype in ('virtio-input-host-pci', 'virtio-blk-pci',
+ 'virtio-crypto-pci', 'virtio-9p-pci',
+ 'vhost-vsock-pci', 'vhost-scsi-pci',
+ 'vhost-user-blk-pci', 'vhost-user-scsi-pci'):
+ return
+
+ # Force legacy mode:
+ dev_legacy, _ = self.run_device(qemu_devtype,
+ 'disable-modern=on,disable-legacy=off',
+ machine='pc-i440fx-2.6')
+
+ # No options: default to legacy on <= 2.6 machine-types:
+ no_opts_pc, _ = self.run_device(qemu_devtype, machine='pc-i440fx-2.6')
+ self.assertEqual(dev_legacy, no_opts_pc)
+
+ def test_legacy_compat(self):
+ devs = self.get_all_hybrid_devs()
+ for d in devs:
+ self.check_legacy_compat(d)
--
Eduardo
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6
2019-01-04 20:09 ` Eduardo Habkost
@ 2019-01-04 20:48 ` Michael S. Tsirkin
2019-01-04 21:06 ` Eduardo Habkost
0 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2019-01-04 20:48 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Dr. David Alan Gilbert, qemu-devel, Thomas Huth, Cornelia Huck
On Fri, Jan 04, 2019 at 06:09:52PM -0200, Eduardo Habkost wrote:
> On Fri, Jan 04, 2019 at 03:54:39PM -0200, Eduardo Habkost wrote:
> > On Fri, Jan 04, 2019 at 10:12:00AM +0000, Dr. David Alan Gilbert wrote:
> > > * Michael S. Tsirkin (mst@redhat.com) wrote:
> > > > On Fri, Jan 04, 2019 at 01:22:26AM -0200, Eduardo Habkost wrote:
> > > > > The virtio-pci entries in HW_COMPAT_2_6 had an implicit
> > > > > assumption: that all virtio-pci subclasses support the
> > > > > disable-legacy and disable-modern options.
> > > > >
> > > > > That assumption was broken by commit f6e501a28ef9 ("virtio:
> > > > > Provide version-specific variants of virtio PCI devices"). This
> > > > > caused QEMU to crash if using the new -non-transitional or
> > > > > -transitional device types:
> > > > >
> > > > > $ qemu-system-x86_64 -machine pc-i440fx-2.6 \
> > > > > -device virtio-net-pci-non-transitional
> > > > > Unexpected error in object_property_find() at qom/object.c:1092:
> > > > > qemu-system-x86_64: -device virtio-net-pci-non-transitional: can't apply \
> > > > > global virtio-pci.disable-modern=on: Property '.disable-modern' not found
> > > > > Aborted (core dumped)
> > > > >
> > > > > Replace the virtio-pci.disable-legacy=off and
> > > > > virtio-pci.disable-modern=on entries on HW_COMPAT_2_6 with
> > > > > explicit entries for each generic virtio device type.
> > > > >
> > > > > The full list of generic virtio device types was extracted by
> > > > > just grepping for ".generic_name". Note that we don't need to
> > > > > worry about listing new virtio-pci devices in HW_COMPAT_2_6 in
> > > > > the future, because new devices won't require QEMU 2.6
> > > > > compatibility.
> > > >
> > > > I fully expect that e.g. packed ring support will need
> > > > to affect all virtio devices too. And it's likely
> > > > that we'll have some new virtio-pci transport features too.
> > > >
> > > > > This makes the compat entries annoyingly verbose, but is simpler
> > > > > than the alternative of making the virtio-pci type inheritance
> > > > > rules even more complex.
> > > >
> > > > God forbid we forgot something, the only way to notice is to
> > > > run a cross version migration with an old qemu.
> > > > I think we need to come up with something less verbose and fragile.
> > >
> > > I guess we could use a script like tests/acceptance/virtio_version.py to
> > > do a check?
> >
> > That's a good idea. On test code we can try additional tricks to
> > detect the hybrid virtio devices without increasing the
> > complexity of QEMU code. I'll give it a try.
>
> I did it but I'm not happy with the result: many of the virtio
> devices can't be tested without extra arguments. Some of them
> (like vhost-*) require extra privileges on the host that might be
> unavailable.
>
> Anyway, while writing this I noticed another issue: many of the
> virtio devices in QEMU 2.6 were already modern-only!
>
> Setting disable-modern=off on modern-only devices like virtio-vga
> or virtio-tablet-pci doesn't make sense. This means setting
> virtio-pci.disable-modern=off on HW_COMPAT_2_6 was incorrect even
> before the -non-transitional and -transitional device types were
> introduced.
It did create an opportunity to create non working devices.
Whether that's incorrect as such I'm not sure.
> ---
> diff --git a/tests/acceptance/virtio_version.py b/tests/acceptance/virtio_version.py
> index ce990250d8..9157a1b173 100644
> --- a/tests/acceptance/virtio_version.py
> +++ b/tests/acceptance/virtio_version.py
> @@ -55,6 +55,18 @@ def get_pci_interfaces(vm, devtype):
> interfaces = ('pci-express-device', 'conventional-pci-device')
> return [i for i in interfaces if devtype_implements(vm, devtype, i)]
>
> +def is_hybrid_dev(vm, devtype):
> + props = [p['name'] for p in vm.command('device-list-properties',
> + typename=devtype)]
> + return 'disable-legacy' in props
> +
> +def get_all_hybrid_devs(vm):
> + """Return list of all hybrid virtio device types (the ones that can be
> + configured using the disable-legacy & disable-modern properties)"""
> + alldevs = [d['name'] for d in vm.command('qom-list-types',
> + implements='virtio-pci')]
> + return [d for d in alldevs if is_hybrid_dev(vm, d)]
> +
> class VirtioVersionCheck(Test):
> """
> Check if virtio-version-specific device types result in the
> @@ -174,3 +186,33 @@ class VirtioVersionCheck(Test):
> self.check_modern_only('virtio-mouse-pci', VIRTIO_INPUT)
> self.check_modern_only('virtio-tablet-pci', VIRTIO_INPUT)
> self.check_modern_only('virtio-keyboard-pci', VIRTIO_INPUT)
> +
> + def get_all_hybrid_devs(self):
> + with QEMUMachine(self.qemu_bin) as vm:
> + vm.set_machine('none')
> + vm.add_args('-S')
> + vm.launch()
> + return get_all_hybrid_devs(vm)
> +
> + def check_legacy_compat(self, qemu_devtype):
> + # these device types can't be run with extra arguments, so we can't
> + # test them directly:
> + if qemu_devtype in ('virtio-input-host-pci', 'virtio-blk-pci',
> + 'virtio-crypto-pci', 'virtio-9p-pci',
> + 'vhost-vsock-pci', 'vhost-scsi-pci',
> + 'vhost-user-blk-pci', 'vhost-user-scsi-pci'):
> + return
> +
> + # Force legacy mode:
> + dev_legacy, _ = self.run_device(qemu_devtype,
> + 'disable-modern=on,disable-legacy=off',
> + machine='pc-i440fx-2.6')
> +
> + # No options: default to legacy on <= 2.6 machine-types:
> + no_opts_pc, _ = self.run_device(qemu_devtype, machine='pc-i440fx-2.6')
> + self.assertEqual(dev_legacy, no_opts_pc)
> +
> + def test_legacy_compat(self):
> + devs = self.get_all_hybrid_devs()
> + for d in devs:
> + self.check_legacy_compat(d)
>
> --
> Eduardo
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6
2019-01-04 20:48 ` Michael S. Tsirkin
@ 2019-01-04 21:06 ` Eduardo Habkost
2019-01-04 21:13 ` Michael S. Tsirkin
0 siblings, 1 reply; 15+ messages in thread
From: Eduardo Habkost @ 2019-01-04 21:06 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Dr. David Alan Gilbert, qemu-devel, Thomas Huth, Cornelia Huck
On Fri, Jan 04, 2019 at 03:48:02PM -0500, Michael S. Tsirkin wrote:
> On Fri, Jan 04, 2019 at 06:09:52PM -0200, Eduardo Habkost wrote:
> > On Fri, Jan 04, 2019 at 03:54:39PM -0200, Eduardo Habkost wrote:
> > > On Fri, Jan 04, 2019 at 10:12:00AM +0000, Dr. David Alan Gilbert wrote:
> > > > * Michael S. Tsirkin (mst@redhat.com) wrote:
> > > > > On Fri, Jan 04, 2019 at 01:22:26AM -0200, Eduardo Habkost wrote:
> > > > > > The virtio-pci entries in HW_COMPAT_2_6 had an implicit
> > > > > > assumption: that all virtio-pci subclasses support the
> > > > > > disable-legacy and disable-modern options.
> > > > > >
> > > > > > That assumption was broken by commit f6e501a28ef9 ("virtio:
> > > > > > Provide version-specific variants of virtio PCI devices"). This
> > > > > > caused QEMU to crash if using the new -non-transitional or
> > > > > > -transitional device types:
> > > > > >
> > > > > > $ qemu-system-x86_64 -machine pc-i440fx-2.6 \
> > > > > > -device virtio-net-pci-non-transitional
> > > > > > Unexpected error in object_property_find() at qom/object.c:1092:
> > > > > > qemu-system-x86_64: -device virtio-net-pci-non-transitional: can't apply \
> > > > > > global virtio-pci.disable-modern=on: Property '.disable-modern' not found
> > > > > > Aborted (core dumped)
> > > > > >
> > > > > > Replace the virtio-pci.disable-legacy=off and
> > > > > > virtio-pci.disable-modern=on entries on HW_COMPAT_2_6 with
> > > > > > explicit entries for each generic virtio device type.
> > > > > >
> > > > > > The full list of generic virtio device types was extracted by
> > > > > > just grepping for ".generic_name". Note that we don't need to
> > > > > > worry about listing new virtio-pci devices in HW_COMPAT_2_6 in
> > > > > > the future, because new devices won't require QEMU 2.6
> > > > > > compatibility.
> > > > >
> > > > > I fully expect that e.g. packed ring support will need
> > > > > to affect all virtio devices too. And it's likely
> > > > > that we'll have some new virtio-pci transport features too.
> > > > >
> > > > > > This makes the compat entries annoyingly verbose, but is simpler
> > > > > > than the alternative of making the virtio-pci type inheritance
> > > > > > rules even more complex.
> > > > >
> > > > > God forbid we forgot something, the only way to notice is to
> > > > > run a cross version migration with an old qemu.
> > > > > I think we need to come up with something less verbose and fragile.
> > > >
> > > > I guess we could use a script like tests/acceptance/virtio_version.py to
> > > > do a check?
> > >
> > > That's a good idea. On test code we can try additional tricks to
> > > detect the hybrid virtio devices without increasing the
> > > complexity of QEMU code. I'll give it a try.
> >
> > I did it but I'm not happy with the result: many of the virtio
> > devices can't be tested without extra arguments. Some of them
> > (like vhost-*) require extra privileges on the host that might be
> > unavailable.
> >
> > Anyway, while writing this I noticed another issue: many of the
> > virtio devices in QEMU 2.6 were already modern-only!
> >
> > Setting disable-modern=off on modern-only devices like virtio-vga
> > or virtio-tablet-pci doesn't make sense. This means setting
> > virtio-pci.disable-modern=off on HW_COMPAT_2_6 was incorrect even
> > before the -non-transitional and -transitional device types were
> > introduced.
>
>
> It did create an opportunity to create non working devices.
>
> Whether that's incorrect as such I'm not sure.
This is not just creating the opportunity for an user to
disable-modern=on. HW_COMPAT_2_6 is actually setting
disable-modern=on on virtio-vga and other modern-only devices.
Sounds like a mistake to me.
Luckily those modern-only devices silently ignore the
disable-modern/disable-legacy properties, but this might change
in the future.
--
Eduardo
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6
2019-01-04 21:06 ` Eduardo Habkost
@ 2019-01-04 21:13 ` Michael S. Tsirkin
2019-01-04 22:00 ` Eduardo Habkost
0 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2019-01-04 21:13 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Dr. David Alan Gilbert, qemu-devel, Thomas Huth, Cornelia Huck
On Fri, Jan 04, 2019 at 07:06:56PM -0200, Eduardo Habkost wrote:
> On Fri, Jan 04, 2019 at 03:48:02PM -0500, Michael S. Tsirkin wrote:
> > On Fri, Jan 04, 2019 at 06:09:52PM -0200, Eduardo Habkost wrote:
> > > On Fri, Jan 04, 2019 at 03:54:39PM -0200, Eduardo Habkost wrote:
> > > > On Fri, Jan 04, 2019 at 10:12:00AM +0000, Dr. David Alan Gilbert wrote:
> > > > > * Michael S. Tsirkin (mst@redhat.com) wrote:
> > > > > > On Fri, Jan 04, 2019 at 01:22:26AM -0200, Eduardo Habkost wrote:
> > > > > > > The virtio-pci entries in HW_COMPAT_2_6 had an implicit
> > > > > > > assumption: that all virtio-pci subclasses support the
> > > > > > > disable-legacy and disable-modern options.
> > > > > > >
> > > > > > > That assumption was broken by commit f6e501a28ef9 ("virtio:
> > > > > > > Provide version-specific variants of virtio PCI devices"). This
> > > > > > > caused QEMU to crash if using the new -non-transitional or
> > > > > > > -transitional device types:
> > > > > > >
> > > > > > > $ qemu-system-x86_64 -machine pc-i440fx-2.6 \
> > > > > > > -device virtio-net-pci-non-transitional
> > > > > > > Unexpected error in object_property_find() at qom/object.c:1092:
> > > > > > > qemu-system-x86_64: -device virtio-net-pci-non-transitional: can't apply \
> > > > > > > global virtio-pci.disable-modern=on: Property '.disable-modern' not found
> > > > > > > Aborted (core dumped)
> > > > > > >
> > > > > > > Replace the virtio-pci.disable-legacy=off and
> > > > > > > virtio-pci.disable-modern=on entries on HW_COMPAT_2_6 with
> > > > > > > explicit entries for each generic virtio device type.
> > > > > > >
> > > > > > > The full list of generic virtio device types was extracted by
> > > > > > > just grepping for ".generic_name". Note that we don't need to
> > > > > > > worry about listing new virtio-pci devices in HW_COMPAT_2_6 in
> > > > > > > the future, because new devices won't require QEMU 2.6
> > > > > > > compatibility.
> > > > > >
> > > > > > I fully expect that e.g. packed ring support will need
> > > > > > to affect all virtio devices too. And it's likely
> > > > > > that we'll have some new virtio-pci transport features too.
> > > > > >
> > > > > > > This makes the compat entries annoyingly verbose, but is simpler
> > > > > > > than the alternative of making the virtio-pci type inheritance
> > > > > > > rules even more complex.
> > > > > >
> > > > > > God forbid we forgot something, the only way to notice is to
> > > > > > run a cross version migration with an old qemu.
> > > > > > I think we need to come up with something less verbose and fragile.
> > > > >
> > > > > I guess we could use a script like tests/acceptance/virtio_version.py to
> > > > > do a check?
> > > >
> > > > That's a good idea. On test code we can try additional tricks to
> > > > detect the hybrid virtio devices without increasing the
> > > > complexity of QEMU code. I'll give it a try.
> > >
> > > I did it but I'm not happy with the result: many of the virtio
> > > devices can't be tested without extra arguments. Some of them
> > > (like vhost-*) require extra privileges on the host that might be
> > > unavailable.
> > >
> > > Anyway, while writing this I noticed another issue: many of the
> > > virtio devices in QEMU 2.6 were already modern-only!
> > >
> > > Setting disable-modern=off on modern-only devices like virtio-vga
> > > or virtio-tablet-pci doesn't make sense. This means setting
> > > virtio-pci.disable-modern=off on HW_COMPAT_2_6 was incorrect even
> > > before the -non-transitional and -transitional device types were
> > > introduced.
> >
> >
> > It did create an opportunity to create non working devices.
> >
> > Whether that's incorrect as such I'm not sure.
>
> This is not just creating the opportunity for an user to
> disable-modern=on. HW_COMPAT_2_6 is actually setting
> disable-modern=on on virtio-vga and other modern-only devices.
> Sounds like a mistake to me.
>
> Luckily those modern-only devices silently ignore the
> disable-modern/disable-legacy properties, but this might change
> in the future.
Worry about it then?
--
MST
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6
2019-01-04 21:13 ` Michael S. Tsirkin
@ 2019-01-04 22:00 ` Eduardo Habkost
2019-01-07 8:12 ` Cornelia Huck
0 siblings, 1 reply; 15+ messages in thread
From: Eduardo Habkost @ 2019-01-04 22:00 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Dr. David Alan Gilbert, qemu-devel, Thomas Huth, Cornelia Huck
On Fri, Jan 04, 2019 at 04:13:15PM -0500, Michael S. Tsirkin wrote:
> On Fri, Jan 04, 2019 at 07:06:56PM -0200, Eduardo Habkost wrote:
> > On Fri, Jan 04, 2019 at 03:48:02PM -0500, Michael S. Tsirkin wrote:
> > > On Fri, Jan 04, 2019 at 06:09:52PM -0200, Eduardo Habkost wrote:
> > > > On Fri, Jan 04, 2019 at 03:54:39PM -0200, Eduardo Habkost wrote:
> > > > > On Fri, Jan 04, 2019 at 10:12:00AM +0000, Dr. David Alan Gilbert wrote:
> > > > > > * Michael S. Tsirkin (mst@redhat.com) wrote:
> > > > > > > On Fri, Jan 04, 2019 at 01:22:26AM -0200, Eduardo Habkost wrote:
> > > > > > > > The virtio-pci entries in HW_COMPAT_2_6 had an implicit
> > > > > > > > assumption: that all virtio-pci subclasses support the
> > > > > > > > disable-legacy and disable-modern options.
> > > > > > > >
> > > > > > > > That assumption was broken by commit f6e501a28ef9 ("virtio:
> > > > > > > > Provide version-specific variants of virtio PCI devices"). This
> > > > > > > > caused QEMU to crash if using the new -non-transitional or
> > > > > > > > -transitional device types:
> > > > > > > >
> > > > > > > > $ qemu-system-x86_64 -machine pc-i440fx-2.6 \
> > > > > > > > -device virtio-net-pci-non-transitional
> > > > > > > > Unexpected error in object_property_find() at qom/object.c:1092:
> > > > > > > > qemu-system-x86_64: -device virtio-net-pci-non-transitional: can't apply \
> > > > > > > > global virtio-pci.disable-modern=on: Property '.disable-modern' not found
> > > > > > > > Aborted (core dumped)
> > > > > > > >
> > > > > > > > Replace the virtio-pci.disable-legacy=off and
> > > > > > > > virtio-pci.disable-modern=on entries on HW_COMPAT_2_6 with
> > > > > > > > explicit entries for each generic virtio device type.
> > > > > > > >
> > > > > > > > The full list of generic virtio device types was extracted by
> > > > > > > > just grepping for ".generic_name". Note that we don't need to
> > > > > > > > worry about listing new virtio-pci devices in HW_COMPAT_2_6 in
> > > > > > > > the future, because new devices won't require QEMU 2.6
> > > > > > > > compatibility.
> > > > > > >
> > > > > > > I fully expect that e.g. packed ring support will need
> > > > > > > to affect all virtio devices too. And it's likely
> > > > > > > that we'll have some new virtio-pci transport features too.
> > > > > > >
> > > > > > > > This makes the compat entries annoyingly verbose, but is simpler
> > > > > > > > than the alternative of making the virtio-pci type inheritance
> > > > > > > > rules even more complex.
> > > > > > >
> > > > > > > God forbid we forgot something, the only way to notice is to
> > > > > > > run a cross version migration with an old qemu.
> > > > > > > I think we need to come up with something less verbose and fragile.
> > > > > >
> > > > > > I guess we could use a script like tests/acceptance/virtio_version.py to
> > > > > > do a check?
> > > > >
> > > > > That's a good idea. On test code we can try additional tricks to
> > > > > detect the hybrid virtio devices without increasing the
> > > > > complexity of QEMU code. I'll give it a try.
> > > >
> > > > I did it but I'm not happy with the result: many of the virtio
> > > > devices can't be tested without extra arguments. Some of them
> > > > (like vhost-*) require extra privileges on the host that might be
> > > > unavailable.
> > > >
> > > > Anyway, while writing this I noticed another issue: many of the
> > > > virtio devices in QEMU 2.6 were already modern-only!
> > > >
> > > > Setting disable-modern=off on modern-only devices like virtio-vga
> > > > or virtio-tablet-pci doesn't make sense. This means setting
> > > > virtio-pci.disable-modern=off on HW_COMPAT_2_6 was incorrect even
> > > > before the -non-transitional and -transitional device types were
> > > > introduced.
> > >
> > >
> > > It did create an opportunity to create non working devices.
> > >
> > > Whether that's incorrect as such I'm not sure.
> >
> > This is not just creating the opportunity for an user to
> > disable-modern=on. HW_COMPAT_2_6 is actually setting
> > disable-modern=on on virtio-vga and other modern-only devices.
> > Sounds like a mistake to me.
> >
> > Luckily those modern-only devices silently ignore the
> > disable-modern/disable-legacy properties, but this might change
> > in the future.
>
> Worry about it then?
Right, we don't need to worry about it today. But if a solution
to the crash reported by Thomas will make the problem go away,
that's even better.
--
Eduardo
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6
2019-01-04 22:00 ` Eduardo Habkost
@ 2019-01-07 8:12 ` Cornelia Huck
2019-01-07 19:26 ` Eduardo Habkost
0 siblings, 1 reply; 15+ messages in thread
From: Cornelia Huck @ 2019-01-07 8:12 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Michael S. Tsirkin, Dr. David Alan Gilbert, qemu-devel,
Thomas Huth
On Fri, 4 Jan 2019 20:00:50 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Fri, Jan 04, 2019 at 04:13:15PM -0500, Michael S. Tsirkin wrote:
> > On Fri, Jan 04, 2019 at 07:06:56PM -0200, Eduardo Habkost wrote:
> > > On Fri, Jan 04, 2019 at 03:48:02PM -0500, Michael S. Tsirkin wrote:
> > > > On Fri, Jan 04, 2019 at 06:09:52PM -0200, Eduardo Habkost wrote:
> > > > > Anyway, while writing this I noticed another issue: many of the
> > > > > virtio devices in QEMU 2.6 were already modern-only!
> > > > >
> > > > > Setting disable-modern=off on modern-only devices like virtio-vga
> > > > > or virtio-tablet-pci doesn't make sense. This means setting
> > > > > virtio-pci.disable-modern=off on HW_COMPAT_2_6 was incorrect even
> > > > > before the -non-transitional and -transitional device types were
> > > > > introduced.
> > > >
> > > >
> > > > It did create an opportunity to create non working devices.
> > > >
> > > > Whether that's incorrect as such I'm not sure.
> > >
> > > This is not just creating the opportunity for an user to
> > > disable-modern=on. HW_COMPAT_2_6 is actually setting
> > > disable-modern=on on virtio-vga and other modern-only devices.
> > > Sounds like a mistake to me.
> > >
> > > Luckily those modern-only devices silently ignore the
> > > disable-modern/disable-legacy properties, but this might change
> > > in the future.
> >
> > Worry about it then?
>
> Right, we don't need to worry about it today. But if a solution
> to the crash reported by Thomas will make the problem go away,
> that's even better.
It seems your patch with the modern-only devices removed from the list
would achieve that?
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6
2019-01-07 8:12 ` Cornelia Huck
@ 2019-01-07 19:26 ` Eduardo Habkost
0 siblings, 0 replies; 15+ messages in thread
From: Eduardo Habkost @ 2019-01-07 19:26 UTC (permalink / raw)
To: Cornelia Huck
Cc: Michael S. Tsirkin, Dr. David Alan Gilbert, qemu-devel,
Thomas Huth
On Mon, Jan 07, 2019 at 09:12:06AM +0100, Cornelia Huck wrote:
> On Fri, 4 Jan 2019 20:00:50 -0200
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> > On Fri, Jan 04, 2019 at 04:13:15PM -0500, Michael S. Tsirkin wrote:
> > > On Fri, Jan 04, 2019 at 07:06:56PM -0200, Eduardo Habkost wrote:
> > > > On Fri, Jan 04, 2019 at 03:48:02PM -0500, Michael S. Tsirkin wrote:
> > > > > On Fri, Jan 04, 2019 at 06:09:52PM -0200, Eduardo Habkost wrote:
>
> > > > > > Anyway, while writing this I noticed another issue: many of the
> > > > > > virtio devices in QEMU 2.6 were already modern-only!
> > > > > >
> > > > > > Setting disable-modern=off on modern-only devices like virtio-vga
> > > > > > or virtio-tablet-pci doesn't make sense. This means setting
> > > > > > virtio-pci.disable-modern=off on HW_COMPAT_2_6 was incorrect even
> > > > > > before the -non-transitional and -transitional device types were
> > > > > > introduced.
> > > > >
> > > > >
> > > > > It did create an opportunity to create non working devices.
> > > > >
> > > > > Whether that's incorrect as such I'm not sure.
> > > >
> > > > This is not just creating the opportunity for an user to
> > > > disable-modern=on. HW_COMPAT_2_6 is actually setting
> > > > disable-modern=on on virtio-vga and other modern-only devices.
> > > > Sounds like a mistake to me.
> > > >
> > > > Luckily those modern-only devices silently ignore the
> > > > disable-modern/disable-legacy properties, but this might change
> > > > in the future.
> > >
> > > Worry about it then?
> >
> > Right, we don't need to worry about it today. But if a solution
> > to the crash reported by Thomas will make the problem go away,
> > that's even better.
>
> It seems your patch with the modern-only devices removed from the list
> would achieve that?
I think so. But I think I'll do the removal of modern-only
devices from the list in a separate patch, just to be safe.
--
Eduardo
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6
2019-01-04 4:23 ` Michael S. Tsirkin
2019-01-04 9:37 ` Cornelia Huck
2019-01-04 10:12 ` Dr. David Alan Gilbert
@ 2019-01-04 16:08 ` Eduardo Habkost
2 siblings, 0 replies; 15+ messages in thread
From: Eduardo Habkost @ 2019-01-04 16:08 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: qemu-devel, Thomas Huth, Dr. David Alan Gilbert, Cornelia Huck
On Thu, Jan 03, 2019 at 11:23:45PM -0500, Michael S. Tsirkin wrote:
> On Fri, Jan 04, 2019 at 01:22:26AM -0200, Eduardo Habkost wrote:
> > The virtio-pci entries in HW_COMPAT_2_6 had an implicit
> > assumption: that all virtio-pci subclasses support the
> > disable-legacy and disable-modern options.
> >
> > That assumption was broken by commit f6e501a28ef9 ("virtio:
> > Provide version-specific variants of virtio PCI devices"). This
> > caused QEMU to crash if using the new -non-transitional or
> > -transitional device types:
> >
> > $ qemu-system-x86_64 -machine pc-i440fx-2.6 \
> > -device virtio-net-pci-non-transitional
> > Unexpected error in object_property_find() at qom/object.c:1092:
> > qemu-system-x86_64: -device virtio-net-pci-non-transitional: can't apply \
> > global virtio-pci.disable-modern=on: Property '.disable-modern' not found
> > Aborted (core dumped)
> >
> > Replace the virtio-pci.disable-legacy=off and
> > virtio-pci.disable-modern=on entries on HW_COMPAT_2_6 with
> > explicit entries for each generic virtio device type.
> >
> > The full list of generic virtio device types was extracted by
> > just grepping for ".generic_name". Note that we don't need to
> > worry about listing new virtio-pci devices in HW_COMPAT_2_6 in
> > the future, because new devices won't require QEMU 2.6
> > compatibility.
>
> I fully expect that e.g. packed ring support will need
> to affect all virtio devices too. And it's likely
> that we'll have some new virtio-pci transport features too.
If you need to affect all virtio devices, you can still use
.driver="virtio-pci" on compat_props. This is just about the
disable-legacy and disable-modern properties.
>
> > This makes the compat entries annoyingly verbose, but is simpler
> > than the alternative of making the virtio-pci type inheritance
> > rules even more complex.
>
> God forbid we forgot something, the only way to notice is to
> run a cross version migration with an old qemu.
> I think we need to come up with something less verbose and fragile.
Both approaches can break if we forget something or do something
wrong. I don't think this one is more fragile than the others.
The difference is that this one lets us list the device types
once on compat_props and never think about it again. The others
would require us to worry forever about subtle interactions
between the disable-modern/disable-legacy properties and QOM
inheritance.
>
>
> > Reported-by: Thomas Huth <thuth@redhat.com>
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > ---
> > include/hw/compat.h | 140 +++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 138 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/hw/compat.h b/include/hw/compat.h
> > index 3ca85b037c..fbb6c1138a 100644
> > --- a/include/hw/compat.h
> > +++ b/include/hw/compat.h
> > @@ -163,11 +163,147 @@
> > .property = "format_transport_address",\
> > .value = "off",\
> > },{\
> > - .driver = "virtio-pci",\
> > + .driver = "vhost-scsi-pci",\
> > .property = "disable-modern",\
> > .value = "on",\
> > },{\
> > - .driver = "virtio-pci",\
> > + .driver = "vhost-scsi-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "vhost-user-blk-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "vhost-user-blk-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "vhost-user-scsi-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "vhost-user-scsi-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "vhost-vsock-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "vhost-vsock-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-9p-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-9p-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-balloon-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-balloon-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-blk-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-blk-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-crypto-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-crypto-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-gpu-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-gpu-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-input-host-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-input-host-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-keyboard-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-keyboard-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-mouse-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-mouse-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-net-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-net-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-rng-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-rng-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-scsi-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-scsi-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-serial-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-serial-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-tablet-pci",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-tablet-pci",\
> > + .property = "disable-legacy",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-vga",\
> > + .property = "disable-modern",\
> > + .value = "on",\
> > + },{\
> > + .driver = "virtio-vga",\
> > .property = "disable-legacy",\
> > .value = "off",\
> > },
> > --
> > 2.18.0.rc1.1.g3f1ff2140
--
Eduardo
^ permalink raw reply [flat|nested] 15+ messages in thread