[PATCH v2 01/12] vhost-user-blk: fix blkcfg->num_queues endianness

Stefan Hajnoczi posted 12 patches 4 years, 11 months ago
There is a newer version of this series
[PATCH v2 01/12] vhost-user-blk: fix blkcfg->num_queues endianness
Posted by Stefan Hajnoczi 4 years, 11 months ago
Treat the num_queues field as virtio-endian. On big-endian hosts the
vhost-user-blk num_queues field was in the wrong endianness.

Move the blkcfg.num_queues store operation from realize to
vhost_user_blk_update_config() so feature negotiation has finished and
we know the endianness of the device. VIRTIO 1.0 devices are
little-endian, but in case someone wants to use legacy VIRTIO we support
all endianness cases.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/block/vhost-user-blk.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 2dd3d93ca0..d9d9dc8a89 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -53,6 +53,9 @@ static void vhost_user_blk_update_config(VirtIODevice *vdev, uint8_t *config)
 {
     VHostUserBlk *s = VHOST_USER_BLK(vdev);
 
+    /* Our num_queues overrides the device backend */
+    virtio_stw_p(vdev, &s->blkcfg.num_queues, s->num_queues);
+
     memcpy(config, &s->blkcfg, sizeof(struct virtio_blk_config));
 }
 
@@ -490,10 +493,6 @@ reconnect:
         goto reconnect;
     }
 
-    if (s->blkcfg.num_queues != s->num_queues) {
-        s->blkcfg.num_queues = s->num_queues;
-    }
-
     return;
 
 virtio_err:
-- 
2.28.0

Re: [PATCH v2 01/12] vhost-user-blk: fix blkcfg->num_queues endianness
Posted by Michael S. Tsirkin 4 years, 10 months ago
On Mon, Dec 07, 2020 at 05:20:19PM +0000, Stefan Hajnoczi wrote:
> Treat the num_queues field as virtio-endian. On big-endian hosts the
> vhost-user-blk num_queues field was in the wrong endianness.
> 
> Move the blkcfg.num_queues store operation from realize to
> vhost_user_blk_update_config() so feature negotiation has finished and
> we know the endianness of the device. VIRTIO 1.0 devices are
> little-endian, but in case someone wants to use legacy VIRTIO we support
> all endianness cases.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

Pls merge with rest of the series.
And maybe CC stable?



> ---
>  hw/block/vhost-user-blk.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index 2dd3d93ca0..d9d9dc8a89 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -53,6 +53,9 @@ static void vhost_user_blk_update_config(VirtIODevice *vdev, uint8_t *config)
>  {
>      VHostUserBlk *s = VHOST_USER_BLK(vdev);
>  
> +    /* Our num_queues overrides the device backend */
> +    virtio_stw_p(vdev, &s->blkcfg.num_queues, s->num_queues);
> +
>      memcpy(config, &s->blkcfg, sizeof(struct virtio_blk_config));
>  }
>  
> @@ -490,10 +493,6 @@ reconnect:
>          goto reconnect;
>      }
>  
> -    if (s->blkcfg.num_queues != s->num_queues) {
> -        s->blkcfg.num_queues = s->num_queues;
> -    }
> -
>      return;
>  
>  virtio_err:
> -- 
> 2.28.0
> 


Re: [PATCH v2 01/12] vhost-user-blk: fix blkcfg->num_queues endianness
Posted by Raphael Norwitz 4 years, 10 months ago
Apologies for the late review - will get to the rest of the series as
soon as I can.

On Mon, Dec 7, 2020 at 12:31 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> Treat the num_queues field as virtio-endian. On big-endian hosts the
> vhost-user-blk num_queues field was in the wrong endianness.
>
> Move the blkcfg.num_queues store operation from realize to
> vhost_user_blk_update_config() so feature negotiation has finished and
> we know the endianness of the device. VIRTIO 1.0 devices are
> little-endian, but in case someone wants to use legacy VIRTIO we support
> all endianness cases.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>


> ---
>  hw/block/vhost-user-blk.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index 2dd3d93ca0..d9d9dc8a89 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -53,6 +53,9 @@ static void vhost_user_blk_update_config(VirtIODevice *vdev, uint8_t *config)
>  {
>      VHostUserBlk *s = VHOST_USER_BLK(vdev);
>
> +    /* Our num_queues overrides the device backend */
> +    virtio_stw_p(vdev, &s->blkcfg.num_queues, s->num_queues);
> +
>      memcpy(config, &s->blkcfg, sizeof(struct virtio_blk_config));
>  }
>
> @@ -490,10 +493,6 @@ reconnect:
>          goto reconnect;
>      }
>
> -    if (s->blkcfg.num_queues != s->num_queues) {
> -        s->blkcfg.num_queues = s->num_queues;
> -    }
> -
>      return;
>
>  virtio_err:
> --
> 2.28.0
>