qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Cornelia Huck <cohuck@redhat.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>,
	qemu-devel@nongnu.org, Thomas Huth <thuth@redhat.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6
Date: Fri, 4 Jan 2019 10:37:08 +0100	[thread overview]
Message-ID: <20190104103708.4d91b342.cohuck@redhat.com> (raw)
In-Reply-To: <20190103232012-mutt-send-email-mst@kernel.org>

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(-)

  reply	other threads:[~2019-01-04  9:37 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-04  3:22 [Qemu-devel] [PATCH] compat: Use explicit type names on HW_COMPAT_2_6 Eduardo Habkost
2019-01-04  4:23 ` Michael S. Tsirkin
2019-01-04  9:37   ` Cornelia Huck [this message]
2019-01-04 10:12   ` Dr. David Alan Gilbert
2019-01-04 17:54     ` Eduardo Habkost
2019-01-04 20:09       ` Eduardo Habkost
2019-01-04 20:48         ` Michael S. Tsirkin
2019-01-04 21:06           ` Eduardo Habkost
2019-01-04 21:13             ` Michael S. Tsirkin
2019-01-04 22:00               ` Eduardo Habkost
2019-01-07  8:12                 ` Cornelia Huck
2019-01-07 19:26                   ` Eduardo Habkost
2019-01-04 16:08   ` Eduardo Habkost
2019-01-04  9:49 ` Cornelia Huck
2019-01-04 16:11   ` Eduardo Habkost

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190104103708.4d91b342.cohuck@redhat.com \
    --to=cohuck@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).