* [PATCH RESEND v3 0/2] virtio: remove unnecessary host_features in ->get_features() @ 2022-11-21 15:50 Stefan Hajnoczi 2022-11-21 15:50 ` [PATCH RESEND v3 1/2] virtio: document ->host_features usage in vdc->get_features() callback Stefan Hajnoczi 2022-11-21 15:50 ` [PATCH RESEND v3 2/2] virtio: remove unnecessary host_features in ->get_features() Stefan Hajnoczi 0 siblings, 2 replies; 6+ messages in thread From: Stefan Hajnoczi @ 2022-11-21 15:50 UTC (permalink / raw) To: qemu-devel Cc: Laurent Vivier, Cédric Le Goater, David Gibson, Hanna Reitz, qemu-block, Michael S. Tsirkin, qemu-ppc, Stefan Hajnoczi, Fam Zheng, Paolo Bonzini, Daniel Henrique Barboza, Greg Kurz, Alex Bennée, Marc-André Lureau, Jason Wang, Amit Shah, Kevin Wolf, David Hildenbrand v3 (resend): - On top of master instead of the qemu.git/staging tree :) v3: - Use an informal comment instead of a full doc comment [Cornelia] v2: - Document vdv->get_features() callback [Cornelia] The vdc->get_features() callbacks are a little inconsistent in how they use vdev->host_features. This is because the function's behavior changed over time. Clean things up. Stefan Hajnoczi (2): virtio: document ->host_features usage in vdc->get_features() callback virtio: remove unnecessary host_features in ->get_features() include/hw/virtio/virtio.h | 7 +++++++ hw/block/virtio-blk.c | 3 --- hw/char/virtio-serial-bus.c | 1 - hw/net/virtio-net.c | 3 --- hw/scsi/vhost-scsi-common.c | 3 --- hw/scsi/virtio-scsi.c | 4 ---- hw/virtio/virtio-balloon.c | 2 -- 7 files changed, 7 insertions(+), 16 deletions(-) -- 2.38.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH RESEND v3 1/2] virtio: document ->host_features usage in vdc->get_features() callback 2022-11-21 15:50 [PATCH RESEND v3 0/2] virtio: remove unnecessary host_features in ->get_features() Stefan Hajnoczi @ 2022-11-21 15:50 ` Stefan Hajnoczi 2022-11-21 16:11 ` Cornelia Huck 2022-11-21 15:50 ` [PATCH RESEND v3 2/2] virtio: remove unnecessary host_features in ->get_features() Stefan Hajnoczi 1 sibling, 1 reply; 6+ messages in thread From: Stefan Hajnoczi @ 2022-11-21 15:50 UTC (permalink / raw) To: qemu-devel Cc: Laurent Vivier, Cédric Le Goater, David Gibson, Hanna Reitz, qemu-block, Michael S. Tsirkin, qemu-ppc, Stefan Hajnoczi, Fam Zheng, Paolo Bonzini, Daniel Henrique Barboza, Greg Kurz, Alex Bennée, Marc-André Lureau, Jason Wang, Amit Shah, Kevin Wolf, David Hildenbrand, Cornelia Huck Suggested-by: Cornelia Huck <cohuck@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- include/hw/virtio/virtio.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index a973811cbf..b6e09c6d4b 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -138,9 +138,16 @@ struct VirtioDeviceClass { /* This is what a VirtioDevice must implement */ DeviceRealize realize; DeviceUnrealize unrealize; + + /* + * Called with vdev->host_features in requested_features. Returns device + * feature bits to be stored in vdev->host_features after factoring in + * device-specific feature bits. + */ uint64_t (*get_features)(VirtIODevice *vdev, uint64_t requested_features, Error **errp); + uint64_t (*bad_features)(VirtIODevice *vdev); void (*set_features)(VirtIODevice *vdev, uint64_t val); int (*validate_features)(VirtIODevice *vdev); -- 2.38.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH RESEND v3 1/2] virtio: document ->host_features usage in vdc->get_features() callback 2022-11-21 15:50 ` [PATCH RESEND v3 1/2] virtio: document ->host_features usage in vdc->get_features() callback Stefan Hajnoczi @ 2022-11-21 16:11 ` Cornelia Huck 0 siblings, 0 replies; 6+ messages in thread From: Cornelia Huck @ 2022-11-21 16:11 UTC (permalink / raw) To: Stefan Hajnoczi, qemu-devel Cc: Laurent Vivier, Cédric Le Goater, David Gibson, Hanna Reitz, qemu-block, Michael S. Tsirkin, qemu-ppc, Stefan Hajnoczi, Fam Zheng, Paolo Bonzini, Daniel Henrique Barboza, Greg Kurz, Alex Bennée, Marc-André Lureau, Jason Wang, Amit Shah, Kevin Wolf, David Hildenbrand On Mon, Nov 21 2022, Stefan Hajnoczi <stefanha@redhat.com> wrote: > Suggested-by: Cornelia Huck <cohuck@redhat.com> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > include/hw/virtio/virtio.h | 7 +++++++ > 1 file changed, 7 insertions(+) Reviewed-by: Cornelia Huck <cohuck@redhat.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH RESEND v3 2/2] virtio: remove unnecessary host_features in ->get_features() 2022-11-21 15:50 [PATCH RESEND v3 0/2] virtio: remove unnecessary host_features in ->get_features() Stefan Hajnoczi 2022-11-21 15:50 ` [PATCH RESEND v3 1/2] virtio: document ->host_features usage in vdc->get_features() callback Stefan Hajnoczi @ 2022-11-21 15:50 ` Stefan Hajnoczi 2022-11-21 16:12 ` Cornelia Huck 2022-12-20 23:08 ` Michael S. Tsirkin 1 sibling, 2 replies; 6+ messages in thread From: Stefan Hajnoczi @ 2022-11-21 15:50 UTC (permalink / raw) To: qemu-devel Cc: Laurent Vivier, Cédric Le Goater, David Gibson, Hanna Reitz, qemu-block, Michael S. Tsirkin, qemu-ppc, Stefan Hajnoczi, Fam Zheng, Paolo Bonzini, Daniel Henrique Barboza, Greg Kurz, Alex Bennée, Marc-André Lureau, Jason Wang, Amit Shah, Kevin Wolf, David Hildenbrand, Cornelia Huck Since at least commit 6b8f1020540c27246277377aa2c3331ad2bfb160 ("virtio: move host_features") the ->get_features() function has been called with host_features as an argument. Some devices manually add host_features in ->get_features() although the features argument already contains host_features. Make all devices consistent by dropping the unnecessary code. Cc: Cornelia Huck <cohuck@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- hw/block/virtio-blk.c | 3 --- hw/char/virtio-serial-bus.c | 1 - hw/net/virtio-net.c | 3 --- hw/scsi/vhost-scsi-common.c | 3 --- hw/scsi/virtio-scsi.c | 4 ---- hw/virtio/virtio-balloon.c | 2 -- 6 files changed, 16 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index f717550fdc..a744012fec 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -983,9 +983,6 @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features, { VirtIOBlock *s = VIRTIO_BLK(vdev); - /* Firstly sync all virtio-blk possible supported features */ - features |= s->host_features; - virtio_add_feature(&features, VIRTIO_BLK_F_SEG_MAX); virtio_add_feature(&features, VIRTIO_BLK_F_GEOMETRY); virtio_add_feature(&features, VIRTIO_BLK_F_TOPOLOGY); diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c index 7d4601cb5d..1414fb85ae 100644 --- a/hw/char/virtio-serial-bus.c +++ b/hw/char/virtio-serial-bus.c @@ -557,7 +557,6 @@ static uint64_t get_features(VirtIODevice *vdev, uint64_t features, vser = VIRTIO_SERIAL(vdev); - features |= vser->host_features; if (vser->bus.max_nr_ports > 1) { virtio_add_feature(&features, VIRTIO_CONSOLE_F_MULTIPORT); } diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index aba12759d5..35b45eb6e3 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -777,9 +777,6 @@ static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features, VirtIONet *n = VIRTIO_NET(vdev); NetClientState *nc = qemu_get_queue(n->nic); - /* Firstly sync all virtio-net possible supported features */ - features |= n->host_features; - virtio_add_feature(&features, VIRTIO_NET_F_MAC); if (!peer_has_vnet_hdr(n)) { diff --git a/hw/scsi/vhost-scsi-common.c b/hw/scsi/vhost-scsi-common.c index 767f827e55..8b26f90aa1 100644 --- a/hw/scsi/vhost-scsi-common.c +++ b/hw/scsi/vhost-scsi-common.c @@ -124,9 +124,6 @@ uint64_t vhost_scsi_common_get_features(VirtIODevice *vdev, uint64_t features, { VHostSCSICommon *vsc = VHOST_SCSI_COMMON(vdev); - /* Turn on predefined features supported by this device */ - features |= vsc->host_features; - return vhost_get_features(&vsc->dev, vsc->feature_bits, features); } diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 6f6e2e32ba..40e10322fb 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -817,10 +817,6 @@ static uint64_t virtio_scsi_get_features(VirtIODevice *vdev, uint64_t requested_features, Error **errp) { - VirtIOSCSI *s = VIRTIO_SCSI(vdev); - - /* Firstly sync all virtio-scsi possible supported features */ - requested_features |= s->host_features; return requested_features; } diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index 73ac5eb675..0e9ca71b15 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -796,8 +796,6 @@ static void virtio_balloon_set_config(VirtIODevice *vdev, static uint64_t virtio_balloon_get_features(VirtIODevice *vdev, uint64_t f, Error **errp) { - VirtIOBalloon *dev = VIRTIO_BALLOON(vdev); - f |= dev->host_features; virtio_add_feature(&f, VIRTIO_BALLOON_F_STATS_VQ); return f; -- 2.38.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH RESEND v3 2/2] virtio: remove unnecessary host_features in ->get_features() 2022-11-21 15:50 ` [PATCH RESEND v3 2/2] virtio: remove unnecessary host_features in ->get_features() Stefan Hajnoczi @ 2022-11-21 16:12 ` Cornelia Huck 2022-12-20 23:08 ` Michael S. Tsirkin 1 sibling, 0 replies; 6+ messages in thread From: Cornelia Huck @ 2022-11-21 16:12 UTC (permalink / raw) To: Stefan Hajnoczi, qemu-devel Cc: Laurent Vivier, Cédric Le Goater, David Gibson, Hanna Reitz, qemu-block, Michael S. Tsirkin, qemu-ppc, Stefan Hajnoczi, Fam Zheng, Paolo Bonzini, Daniel Henrique Barboza, Greg Kurz, Alex Bennée, Marc-André Lureau, Jason Wang, Amit Shah, Kevin Wolf, David Hildenbrand On Mon, Nov 21 2022, Stefan Hajnoczi <stefanha@redhat.com> wrote: > Since at least commit 6b8f1020540c27246277377aa2c3331ad2bfb160 ("virtio: > move host_features") the ->get_features() function has been called with > host_features as an argument. > > Some devices manually add host_features in ->get_features() although the > features argument already contains host_features. Make all devices > consistent by dropping the unnecessary code. > > Cc: Cornelia Huck <cohuck@redhat.com> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > hw/block/virtio-blk.c | 3 --- > hw/char/virtio-serial-bus.c | 1 - > hw/net/virtio-net.c | 3 --- > hw/scsi/vhost-scsi-common.c | 3 --- > hw/scsi/virtio-scsi.c | 4 ---- > hw/virtio/virtio-balloon.c | 2 -- > 6 files changed, 16 deletions(-) Reviewed-by: Cornelia Huck <cohuck@redhat.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RESEND v3 2/2] virtio: remove unnecessary host_features in ->get_features() 2022-11-21 15:50 ` [PATCH RESEND v3 2/2] virtio: remove unnecessary host_features in ->get_features() Stefan Hajnoczi 2022-11-21 16:12 ` Cornelia Huck @ 2022-12-20 23:08 ` Michael S. Tsirkin 1 sibling, 0 replies; 6+ messages in thread From: Michael S. Tsirkin @ 2022-12-20 23:08 UTC (permalink / raw) To: Stefan Hajnoczi Cc: qemu-devel, Laurent Vivier, Cédric Le Goater, David Gibson, Hanna Reitz, qemu-block, qemu-ppc, Fam Zheng, Paolo Bonzini, Daniel Henrique Barboza, Greg Kurz, Alex Bennée, Marc-André Lureau, Jason Wang, Amit Shah, Kevin Wolf, David Hildenbrand, Cornelia Huck On Mon, Nov 21, 2022 at 10:50:20AM -0500, Stefan Hajnoczi wrote: > Since at least commit 6b8f1020540c27246277377aa2c3331ad2bfb160 ("virtio: > move host_features") the ->get_features() function has been called with > host_features as an argument. > > Some devices manually add host_features in ->get_features() although the > features argument already contains host_features. Make all devices > consistent by dropping the unnecessary code. > > Cc: Cornelia Huck <cohuck@redhat.com> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Causes qos-test failures: ok 65 /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-net-pci/virtio-net/virtio-net-tests/vhost-user/flags-mismatch ** ERROR:../tests/qtest/vhost-user-test.c:889:wait_for_rings_started: assertion failed (ctpop64(s->rings) == count): (2 == 4) # child process (/x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-net-pci/virtio-net/virtio-net-tests/vhost-user/multiqueue/subprocess [856811]) exit status: 1 (error) # child process (/x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-net-pci/virtio-net/virtio-net-tests/vhost-user/multiqueue/subprocess [856811]) stdout: "" # child process (/x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-net-pci/virtio-net/virtio-net-tests/vhost-user/multiqueue/subprocess [856811]) stderr: "**\nERROR:../tests/qtest/vhost-user-test.c:889:wait_for_rings_started: assertion failed (ctpop64(s->rings) == count): (2 == 4)\n" ** ERROR:../tests/qtest/qos-test.c:191:subprocess_run_one_test: child process (/x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-net-pci/virtio-net/virtio-net-tests/vhost-user/multiqueue/subprocess [856811]) failed unexpectedly Bail out! ERROR:../tests/qtest/qos-test.c:191:subprocess_run_one_test: child process (/x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-net-pci/virtio-net/virtio-net-tests/vhost-user/multiqueue/subprocess [856811]) failed unexpectedly runone.sh: line 10: 856419 Aborted (core dumped) QTEST_QEMU_STORAGE_DAEMON_BINARY=./build/storage-daemon/qemu-storage-daemon QTEST_QEMU_BINARY=build/x86_64-softmmu/qemu-system-x86_64 "$@" > --- > hw/block/virtio-blk.c | 3 --- > hw/char/virtio-serial-bus.c | 1 - > hw/net/virtio-net.c | 3 --- > hw/scsi/vhost-scsi-common.c | 3 --- > hw/scsi/virtio-scsi.c | 4 ---- > hw/virtio/virtio-balloon.c | 2 -- > 6 files changed, 16 deletions(-) > > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > index f717550fdc..a744012fec 100644 > --- a/hw/block/virtio-blk.c > +++ b/hw/block/virtio-blk.c > @@ -983,9 +983,6 @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features, > { > VirtIOBlock *s = VIRTIO_BLK(vdev); > > - /* Firstly sync all virtio-blk possible supported features */ > - features |= s->host_features; > - > virtio_add_feature(&features, VIRTIO_BLK_F_SEG_MAX); > virtio_add_feature(&features, VIRTIO_BLK_F_GEOMETRY); > virtio_add_feature(&features, VIRTIO_BLK_F_TOPOLOGY); > diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c > index 7d4601cb5d..1414fb85ae 100644 > --- a/hw/char/virtio-serial-bus.c > +++ b/hw/char/virtio-serial-bus.c > @@ -557,7 +557,6 @@ static uint64_t get_features(VirtIODevice *vdev, uint64_t features, > > vser = VIRTIO_SERIAL(vdev); > > - features |= vser->host_features; > if (vser->bus.max_nr_ports > 1) { > virtio_add_feature(&features, VIRTIO_CONSOLE_F_MULTIPORT); > } > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index aba12759d5..35b45eb6e3 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -777,9 +777,6 @@ static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features, > VirtIONet *n = VIRTIO_NET(vdev); > NetClientState *nc = qemu_get_queue(n->nic); > > - /* Firstly sync all virtio-net possible supported features */ > - features |= n->host_features; > - > virtio_add_feature(&features, VIRTIO_NET_F_MAC); > > if (!peer_has_vnet_hdr(n)) { > diff --git a/hw/scsi/vhost-scsi-common.c b/hw/scsi/vhost-scsi-common.c > index 767f827e55..8b26f90aa1 100644 > --- a/hw/scsi/vhost-scsi-common.c > +++ b/hw/scsi/vhost-scsi-common.c > @@ -124,9 +124,6 @@ uint64_t vhost_scsi_common_get_features(VirtIODevice *vdev, uint64_t features, > { > VHostSCSICommon *vsc = VHOST_SCSI_COMMON(vdev); > > - /* Turn on predefined features supported by this device */ > - features |= vsc->host_features; > - > return vhost_get_features(&vsc->dev, vsc->feature_bits, features); > } > > diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c > index 6f6e2e32ba..40e10322fb 100644 > --- a/hw/scsi/virtio-scsi.c > +++ b/hw/scsi/virtio-scsi.c > @@ -817,10 +817,6 @@ static uint64_t virtio_scsi_get_features(VirtIODevice *vdev, > uint64_t requested_features, > Error **errp) > { > - VirtIOSCSI *s = VIRTIO_SCSI(vdev); > - > - /* Firstly sync all virtio-scsi possible supported features */ > - requested_features |= s->host_features; > return requested_features; > } > > diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c > index 73ac5eb675..0e9ca71b15 100644 > --- a/hw/virtio/virtio-balloon.c > +++ b/hw/virtio/virtio-balloon.c > @@ -796,8 +796,6 @@ static void virtio_balloon_set_config(VirtIODevice *vdev, > static uint64_t virtio_balloon_get_features(VirtIODevice *vdev, uint64_t f, > Error **errp) > { > - VirtIOBalloon *dev = VIRTIO_BALLOON(vdev); > - f |= dev->host_features; > virtio_add_feature(&f, VIRTIO_BALLOON_F_STATS_VQ); > > return f; > -- > 2.38.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-12-20 23:09 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-11-21 15:50 [PATCH RESEND v3 0/2] virtio: remove unnecessary host_features in ->get_features() Stefan Hajnoczi 2022-11-21 15:50 ` [PATCH RESEND v3 1/2] virtio: document ->host_features usage in vdc->get_features() callback Stefan Hajnoczi 2022-11-21 16:11 ` Cornelia Huck 2022-11-21 15:50 ` [PATCH RESEND v3 2/2] virtio: remove unnecessary host_features in ->get_features() Stefan Hajnoczi 2022-11-21 16:12 ` Cornelia Huck 2022-12-20 23:08 ` Michael S. Tsirkin
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).