[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6 |
Date: |
Fri, 4 Jan 2019 16:13:15 -0500 |
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 (address@hidden) 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
- [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6, Eduardo Habkost, 2019/01/03
- Re: [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6, Michael S. Tsirkin, 2019/01/03
- Re: [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6, Cornelia Huck, 2019/01/04
- Re: [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6, Dr. David Alan Gilbert, 2019/01/04
- Re: [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6, Eduardo Habkost, 2019/01/04
- Re: [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6, Eduardo Habkost, 2019/01/04
- Re: [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6, Michael S. Tsirkin, 2019/01/04
- Re: [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6, Eduardo Habkost, 2019/01/04
- Re: [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6,
Michael S. Tsirkin <=
- Re: [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6, Eduardo Habkost, 2019/01/04
- Re: [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6, Cornelia Huck, 2019/01/07
- Re: [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6, Eduardo Habkost, 2019/01/07
Re: [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6, Eduardo Habkost, 2019/01/04
Re: [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6, Cornelia Huck, 2019/01/04