[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH] xen-block: support feature-large-sector-size
From: |
Paul Durrant |
Subject: |
Re: [Qemu-block] [PATCH] xen-block: support feature-large-sector-size |
Date: |
Wed, 10 Apr 2019 15:55:24 +0000 |
> -----Original Message-----
> From: Anthony PERARD [mailto:address@hidden
> Sent: 10 April 2019 16:52
> To: Paul Durrant <address@hidden>
> Cc: address@hidden; address@hidden; address@hidden; Stefano Stabellini
> <address@hidden>; Stefan Hajnoczi <address@hidden>; Kevin Wolf
> <address@hidden>; Max
> Reitz <address@hidden>
> Subject: Re: [PATCH] xen-block: support feature-large-sector-size
>
> On Tue, Apr 09, 2019 at 05:40:38PM +0100, Paul Durrant wrote:
> > A recent Xen commit [1] clarified the semantics of sector based quantities
> > used in the blkif protocol such that it is now safe to create a xen-block
> > device with a logical_block_size != 512, as long as the device only
> > connects to a frontend advertizing 'feature-large-block-size'.
> >
> > This patch modifies xen-block accordingly. It also uses a stack variable
> > for the BlockBackend in xen_block_realize() to avoid repeated dereferencing
> > of the BlockConf pointer, and changes the parameters of
> > xen_block_dataplane_create() so that the BlockBackend pointer and sector
> > size are passed expicitly rather than implicitly via the BlockConf.
> >
> > These modifications have been tested against a recent Windows PV XENVBD
> > driver [2] using a xen-disk device with a 4kB logical block size.
> >
> > [1]
> > http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=67e1c050e36b2c9900cca83618e56189effbad98
> > [2] https://winpvdrvbuild.xenproject.org:8080/job/XENVBD-master/126
> >
> > Signed-off-by: Paul Durrant <address@hidden>
> > ---
> > diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
> > index ef635be4c2..05e890ad78 100644
> > --- a/hw/block/xen-block.c
> > +++ b/hw/block/xen-block.c
> > @@ -51,11 +51,25 @@ static void xen_block_connect(XenDevice *xendev, Error
> > **errp)
> [...]
> > + if (xen_device_frontend_scanf(xendev, "feature-large-sector-size",
> > "%u",
> > + &feature_large_sector_size) != 1) {
> > + feature_large_sector_size = 0;
> > + }
> > +
> > + if (feature_large_sector_size != 1 &&
> > + conf->logical_block_size != XEN_BLKIF_SECTOR_SIZE) {
> > + error_setg(errp, "logical_block_size != %u not supported",
>
> Maybe add "by frontend" to the error message?
Yes, I'm fine with that addition.
>
> > + XEN_BLKIF_SECTOR_SIZE);
> > + return;
> > + }
> > +
>
> With the question answered:
> Reviewed-by: Anthony PERARD <address@hidden>
>
Thanks,
Paul
> Thanks,
>
> --
> Anthony PERARD