From: "Michael S. Tsirkin" <mst@redhat.com>
To: Eduardo Habkost <ehabkost@redhat.com>
Cc: berrange@redhat.com, Stefan Hajnoczi <stefanha@gmail.com>,
jasowang@redhat.com, qemu-devel@nongnu.org,
Jean-Louis Dupond <jean-louis@dupond.be>,
pbonzini@redhat.com
Subject: Re: [PATCH] hw/qdev-core: Add compatibility for (non)-transitional devs
Date: Wed, 20 Oct 2021 11:16:09 -0400 [thread overview]
Message-ID: <20211020111227-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20211020150143.kocjozzcuz3f35b2@habkost.net>
On Wed, Oct 20, 2021 at 11:01:43AM -0400, Eduardo Habkost wrote:
> On Wed, Oct 20, 2021 at 10:55:59AM -0400, Michael S. Tsirkin wrote:
> > On Wed, Oct 20, 2021 at 09:57:37AM -0400, Eduardo Habkost wrote:
> > > On Wed, Oct 20, 2021 at 03:41:38AM -0400, Michael S. Tsirkin wrote:
> > > > On Tue, Oct 19, 2021 at 12:56:11PM -0400, Eduardo Habkost wrote:
> > > > > On Tue, Oct 19, 2021 at 12:13:17PM -0400, Michael S. Tsirkin wrote:
> > > > > > On Tue, Oct 19, 2021 at 11:29:13AM -0400, Eduardo Habkost wrote:
> > > > > > > On Tue, Oct 19, 2021 at 06:59:09AM -0400, Michael S. Tsirkin wrote:
> > > > > > > > On Tue, Oct 19, 2021 at 11:46:17AM +0100, Stefan Hajnoczi wrote:
> > > > > > > > > On Tue, Oct 12, 2021 at 10:36:01AM +0200, Jean-Louis Dupond wrote:
> > > > > > > > > > Forgot to CC maintainers.
> > > > > > > > >
> > > > > > > > > Also CCing Jason Wang and Michael Tsirkin for VIRTIO.
> > > > > > > > >
> > > > > > > > > Stefan
> > > > > > > >
> > > > > > > > OMG
> > > > > > > > where all compat properties broken all the time?
> > > > > > >
> > > > > > > Compat properties that existed when commit f6e501a28ef9 ("virtio:
> > > > > > > Provide version-specific variants of virtio PCI devices") was
> > > > > > > merged are not broken, because virtio-*-transitional and
> > > > > > > virtio-*-non-transitional were brand new QOM types (so there's no
> > > > > > > compatibility to be kept with old QEMU versions).
> > > > > > >
> > > > > > > Compat properties referencing "virtio-*-pci" instead of
> > > > > > > "virtio-*-pci-base" added after commit f6e501a28ef9 are probably
> > > > > > > broken, yes.
> > > > > > >
> > > > > > > --
> > > > > > > Eduardo
> > > > > >
> > > > > > Oh. So just this one:
> > > > > > { "virtio-net-pci", "vectors", "3"},
> > > > > >
> > > > > > right?
> > > > >
> > > > > I think so. That's the only post-4.0 virtio-*-pci compat property I see in
> > > > > hw/core/machine.c.
> > > > >
> > > > > pc.c doesn't have any post-4.0 virtio-*-pci compat props. I didn't see any
> > > > > virtio compat props on spapr.c and s390-virtio-ccw.c.
> > > > >
> > > > > >
> > > > > > about the patch: how do people feel about virtio specific
> > > > > > stuff in qdev core? Ok by everyone?
> > > > >
> > > > > Not OK, if we have a mechanism to avoid that, already (the
> > > > > "virtio-net-pci-base" type name). I wonder what we can do to
> > > > > make this kind of mistake less likely, though.
> > > > >
> > > > > Jean-Louis, Jason, does the following fix work?
> > > > >
> > > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > > > > ---
> > > > > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > > > > index b8d95eec32d..bd9c6156c1a 100644
> > > > > --- a/hw/core/machine.c
> > > > > +++ b/hw/core/machine.c
> > > > > @@ -56,7 +56,7 @@ GlobalProperty hw_compat_5_2[] = {
> > > > > { "ICH9-LPC", "smm-compat", "on"},
> > > > > { "PIIX4_PM", "smm-compat", "on"},
> > > > > { "virtio-blk-device", "report-discard-granularity", "off" },
> > > > > - { "virtio-net-pci", "vectors", "3"},
> > > > > + { "virtio-net-pci-base", "vectors", "3"},
> > > > > };
> > > > > const size_t hw_compat_5_2_len = G_N_ELEMENTS(hw_compat_5_2);
> > > >
> > > > Hmm I'm a bit confused at this point, as to why does
> > > > specifying properties for virtio-net-pci on command
> > > > line with -global work, but in compat list doesn't. Do others
> > > > understand?
> > >
> > > I don't think that's the case. -global behaves similarly to compat_props.
> > >
> > > Running an unpatched QEMU 6.1.0 binary:
> > >
> > > $ echo -e 'info qtree\nquit' | qemu-system-x86_64 -device virtio-net-pci -machine pc-q35-5.2 -monitor stdio | grep vectors
> > > vectors = 3 (0x3)
> > > $ echo -e 'info qtree\nquit' | qemu-system-x86_64 -device virtio-net-pci-non-transitional -machine pc-q35-5.2 -monitor stdio | grep vectors
> > > vectors = 4 (0x4)
> > > $ echo -e 'info qtree\nquit' | qemu-system-x86_64 -device virtio-net-pci-non-transitional -global virtio-net-pci.vectors=3 -monitor stdio | grep vectors
> > > vectors = 4 (0x4)
> > > $ echo -e 'info qtree\nquit' | qemu-system-x86_64 -device virtio-net-pci-non-transitional -global virtio-net-pci-base.vectors=3 -monitor stdio | grep vectors
> > > vectors = 3 (0x3)
> >
> > OK so ... that's another breakage then. Suggestions how to fix?
>
> What exactly is another breakage? virtio-net-pci,
> virtio-net-pci-non-transitional, and virtio-net-pci-transitional
> are three distinct device types.
Hmm. I guess ... good point.
> --
> Eduardo
next prev parent reply other threads:[~2021-10-20 15:17 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-12 8:24 [PATCH] hw/qdev-core: Add compatibility for (non)-transitional devs Jean-Louis Dupond
2021-10-12 8:36 ` Jean-Louis Dupond
2021-10-19 10:46 ` Stefan Hajnoczi
2021-10-19 10:59 ` Michael S. Tsirkin
2021-10-19 15:29 ` Eduardo Habkost
2021-10-19 16:13 ` Michael S. Tsirkin
2021-10-19 16:56 ` Eduardo Habkost
2021-10-20 1:31 ` Jason Wang
2021-10-20 5:02 ` Jason Wang
2021-10-20 14:09 ` Eduardo Habkost
2021-10-20 14:58 ` Michael S. Tsirkin
2021-10-20 15:46 ` Eduardo Habkost
2021-10-20 7:00 ` Jean-Louis Dupond
2021-10-20 7:41 ` Michael S. Tsirkin
2021-10-20 13:57 ` Eduardo Habkost
2021-10-20 14:55 ` Michael S. Tsirkin
2021-10-20 15:01 ` Eduardo Habkost
2021-10-20 15:16 ` Michael S. Tsirkin [this message]
2021-10-19 15:27 ` Eduardo Habkost
2021-10-20 6:58 ` Jean-Louis Dupond
2021-11-01 22:26 ` Michael S. Tsirkin
2021-11-03 7:51 ` Jean-Louis Dupond
2021-11-03 7:58 ` Michael S. Tsirkin
-- strict thread matches above, loose matches on Subject: below --
2021-10-12 7:49 Jean-Louis Dupond
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=20211020111227-mutt-send-email-mst@kernel.org \
--to=mst@redhat.com \
--cc=berrange@redhat.com \
--cc=ehabkost@redhat.com \
--cc=jasowang@redhat.com \
--cc=jean-louis@dupond.be \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@gmail.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).