Re: [PATCH v4 1/3] virtio-blk: store opt_io_size with correct size

[Michael S. Tsirkin]

On Wed, May 20, 2020 at 11:06:55AM +0300, Roman Kagan wrote:
> The width of opt_io_size in virtio_blk_topology is 32bit.
> 
> Use the appropriate accessor to store it.
> 
> Signed-off-by: Roman Kagan <address@hidden>


Thanks for the patch!
Could you add a bit of analysis - when does this cause
bugs? I'm guessing on BE systems with legacy virtio, right?

Also, should we convert virtio_stw_p and friends to get the
pointer to the correct value type, as opposed to void *?

This will catch bugs like this ...



> ---
> v4: new patch
> 
>  hw/block/virtio-blk.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index f5f6fc925e..413083e62f 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -918,7 +918,7 @@ static void virtio_blk_update_config(VirtIODevice *vdev, 
> uint8_t *config)
>      virtio_stw_p(vdev, &blkcfg.geometry.cylinders, conf->cyls);
>      virtio_stl_p(vdev, &blkcfg.blk_size, blk_size);
>      virtio_stw_p(vdev, &blkcfg.min_io_size, conf->min_io_size / blk_size);
> -    virtio_stw_p(vdev, &blkcfg.opt_io_size, conf->opt_io_size / blk_size);
> +    virtio_stl_p(vdev, &blkcfg.opt_io_size, conf->opt_io_size / blk_size);
>      blkcfg.geometry.heads = conf->heads;
>      /*
>       * We must ensure that the block device capacity is a multiple of
> -- 
> 2.26.2