qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] virtio: remove unnecessary host_features in ->get_features()
@ 2022-08-02 13:24 Stefan Hajnoczi
  0 siblings, 0 replies; 3+ messages in thread
From: Stefan Hajnoczi @ 2022-08-02 13:24 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Marc-André Lureau, Fam Zheng, Kevin Wolf,
	qemu-block, Amit Shah, Michael S. Tsirkin, Jason Wang,
	Paolo Bonzini, Stefan Hajnoczi, David Hildenbrand, Hanna Reitz,
	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 e9ba752f6b..429aedcf2b 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -996,9 +996,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 dd0d056fde..8ecdc1cd83 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -715,9 +715,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 4141dddd51..f754611dfe 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -816,10 +816,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.37.1



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH] virtio: remove unnecessary host_features in ->get_features()
@ 2022-08-02 13:28 Stefan Hajnoczi
  2022-08-02 15:31 ` Cornelia Huck
  0 siblings, 1 reply; 3+ messages in thread
From: Stefan Hajnoczi @ 2022-08-02 13:28 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael S. Tsirkin, Jason Wang, Paolo Bonzini, Laurent Vivier,
	Hanna Reitz, Stefan Hajnoczi, qemu-block, David Hildenbrand,
	Marc-André Lureau, Kevin Wolf, Fam Zheng, Amit Shah,
	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 e9ba752f6b..429aedcf2b 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -996,9 +996,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 dd0d056fde..8ecdc1cd83 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -715,9 +715,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 4141dddd51..f754611dfe 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -816,10 +816,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.37.1



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] virtio: remove unnecessary host_features in ->get_features()
  2022-08-02 13:28 [PATCH] virtio: remove unnecessary host_features in ->get_features() Stefan Hajnoczi
@ 2022-08-02 15:31 ` Cornelia Huck
  0 siblings, 0 replies; 3+ messages in thread
From: Cornelia Huck @ 2022-08-02 15:31 UTC (permalink / raw)
  To: Stefan Hajnoczi, qemu-devel
  Cc: Michael S. Tsirkin, Jason Wang, Paolo Bonzini, Laurent Vivier,
	Hanna Reitz, Stefan Hajnoczi, qemu-block, David Hildenbrand,
	Marc-André Lureau, Kevin Wolf, Fam Zheng, Amit Shah

On Tue, Aug 02 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.

I wonder whether we should add that explicitly to the contract for
get_features()?

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

The change seems sane to me.



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-08-02 15:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-02 13:28 [PATCH] virtio: remove unnecessary host_features in ->get_features() Stefan Hajnoczi
2022-08-02 15:31 ` Cornelia Huck
  -- strict thread matches above, loose matches on Subject: below --
2022-08-02 13:24 Stefan Hajnoczi

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