On Wed, May 20, 2020 at 06:44:44AM -0400, Michael S. Tsirkin wrote:
> 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 <rvka...@yandex-team.ru>
>
>
> 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?
I guess so too. It was found just by eye inspection, trying to figure
out the potential truncation of opt_io_size in virtio-blk and why it's
different from scsi. I don't have any analysis to add :(
> Also, should we convert virtio_stw_p and friends to get the
> pointer to the correct value type, as opposed to void *?
I dunno. I guess they were designed to be used with untyped buffers and
modeled after virtio_{st,ld}*_phys. The same question applies to the
underlying {st,ld}_{b,l}e_p.
> This will catch bugs like this ...
I'll try and see if this change doesn't cause too much churn / pain.
But I suggest to decouple it from the simple patch at hand.
Thanks,
Roman.
> > ---
> > 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
>
