On Fri, Jan 04, 2019 at 10:12:00AM +0000, Dr. David Alan Gilbert wrote:
> * Michael S. Tsirkin (m...@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
