Re: [Qemu-devel] [PATCH] xen-block: scale sector based quantities correctly

[Paul Durrant]

> -----Original Message-----
> From: Anthony PERARD [mailto:address@hidden
> Sent: 21 March 2019 15:00
> To: Paul Durrant <address@hidden>
> Cc: address@hidden; address@hidden; address@hidden; Stefan Hajnoczi
> <address@hidden>; Stefano Stabellini <address@hidden>; Kevin Wolf 
> <address@hidden>; Max
> Reitz <address@hidden>
> Subject: Re: [PATCH] xen-block: scale sector based quantities correctly
> 
> On Wed, Mar 20, 2019 at 04:26:32PM +0000, Paul Durrant wrote:
> > The mail thread at [1] clarifies that the Xen blkif protocol requires that
> > sector based quantities should be interpreted strictly as multiples of
> > 512 bytes. This patch modifies the xen-block code accordingly.
> >
> > [1] 
> > https://lists.xenproject.org/archives/html/xen-devel/2019-03/msg01600.html
> 
> So, that patch [1] only makes one change: how to interpret the "sectors"
> node. It doesn't change the unit of a sector in blkif_request and
> blkif_request_segment.
> 
> For a request, there is a comment in blkif.h:
>     NB. first_sect and last_sect in blkif_request_segment, as well as
>     sector_number in blkif_request, are always expressed in 512-byte units.
> 
> I think most of this QEMU patch is to comply with that comment and would
> be a bug fix, but the single line change in hw/block/xen-block.c is
> different.
> 
> > diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
> > index a848849f48..57e9da7e1c 100644
> > --- a/hw/block/xen-block.c
> > +++ b/hw/block/xen-block.c
> > @@ -149,7 +149,7 @@ static void xen_block_set_size(XenBlockDevice *blockdev)
> >      const char *type = object_get_typename(OBJECT(blockdev));
> >      XenBlockVdev *vdev = &blockdev->props.vdev;
> >      BlockConf *conf = &blockdev->props.conf;
> > -    int64_t sectors = blk_getlength(conf->blk) / conf->logical_block_size;
> > +    int64_t sectors = blk_getlength(conf->blk) / XEN_BLKIF_SECTOR_SIZE;
> 
> That the only thing that the thread [1] is changing.
> 

Ok, true. I'll split this up into two patches then.

  Paul

> >      XenDevice *xendev = XEN_DEVICE(blockdev);
> >
> >      trace_xen_block_size(type, vdev->disk, vdev->partition, sectors);
> 
> Thanks,
> 
> --
> Anthony PERARD