Re: [Qemu-devel] [PATCH] xen-block: support feature-large-sector-size
On 26.06.19 19:19, Anthony PERARD wrote: > On Wed, Jun 26, 2019 at 06:48:50PM +0200, Max Reitz wrote: >> On 09.04.19 18:40, 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 >>> --- >>> Cc: Stefano Stabellini >>> Cc: Anthony Perard >>> Cc: Stefan Hajnoczi >>> Cc: Kevin Wolf >>> Cc: Max Reitz >>> --- >>> hw/block/dataplane/xen-block.c | 25 -- >>> hw/block/dataplane/xen-block.h | 3 ++- >>> hw/block/xen-block.c | 38 +- >>> 3 files changed, 40 insertions(+), 26 deletions(-) >> >> Thanks, added “by frontend” to the error message and applied to my block >> branch: >> >> https://git.xanclic.moe/XanClic/qemu/commits/branch/block > > :(, I've just sent a pull request with that patch: > https://patchew.org/QEMU/20190624153257.20163-1-anthony.per...@citrix.com/20190624153257.20163-2-anthony.per...@citrix.com/ That’s just as well, then. :-) > I guess I need to start sending an email every time I've added a patch > to my queue. Well, it certainly won’t hurt. Although in this cases it’s just a bit of an unfortunate coincidence that I looked at this patch now when Peter seems to be away (otherwise I’d have seen it in master). Max signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] xen-block: support feature-large-sector-size
On Wed, Jun 26, 2019 at 06:48:50PM +0200, Max Reitz wrote: > On 09.04.19 18:40, 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 > > --- > > Cc: Stefano Stabellini > > Cc: Anthony Perard > > Cc: Stefan Hajnoczi > > Cc: Kevin Wolf > > Cc: Max Reitz > > --- > > hw/block/dataplane/xen-block.c | 25 -- > > hw/block/dataplane/xen-block.h | 3 ++- > > hw/block/xen-block.c | 38 +- > > 3 files changed, 40 insertions(+), 26 deletions(-) > > Thanks, added “by frontend” to the error message and applied to my block > branch: > > https://git.xanclic.moe/XanClic/qemu/commits/branch/block :(, I've just sent a pull request with that patch: https://patchew.org/QEMU/20190624153257.20163-1-anthony.per...@citrix.com/20190624153257.20163-2-anthony.per...@citrix.com/ I guess I need to start sending an email every time I've added a patch to my queue. Cheers, -- Anthony PERARD
Re: [Qemu-devel] [PATCH] xen-block: support feature-large-sector-size
On 09.04.19 18:40, 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 > --- > Cc: Stefano Stabellini > Cc: Anthony Perard > Cc: Stefan Hajnoczi > Cc: Kevin Wolf > Cc: Max Reitz > --- > hw/block/dataplane/xen-block.c | 25 -- > hw/block/dataplane/xen-block.h | 3 ++- > hw/block/xen-block.c | 38 +- > 3 files changed, 40 insertions(+), 26 deletions(-) Thanks, added “by frontend” to the error message and applied to my block branch: https://git.xanclic.moe/XanClic/qemu/commits/branch/block Max signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] xen-block: support feature-large-sector-size
> -Original Message-
> From: Anthony PERARD [mailto:anthony.per...@citrix.com]
> Sent: 10 April 2019 16:52
> To: Paul Durrant
> Cc: qemu-devel@nongnu.org; xen-de...@lists.xenproject.org;
> qemu-bl...@nongnu.org; Stefano Stabellini
> ; Stefan Hajnoczi ; Kevin Wolf
> ; Max
> Reitz
> 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
> > ---
> > 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",
> > + _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
>
Thanks,
Paul
> Thanks,
>
> --
> Anthony PERARD
Re: [Qemu-devel] [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
> ---
> 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",
> + _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?
> + XEN_BLKIF_SECTOR_SIZE);
> +return;
> +}
> +
With the question answered:
Reviewed-by: Anthony PERARD
Thanks,
--
Anthony PERARD
[Qemu-devel] [PATCH] xen-block: support feature-large-sector-size
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
---
Cc: Stefano Stabellini
Cc: Anthony Perard
Cc: Stefan Hajnoczi
Cc: Kevin Wolf
Cc: Max Reitz
---
hw/block/dataplane/xen-block.c | 25 --
hw/block/dataplane/xen-block.h | 3 ++-
hw/block/xen-block.c | 38 +-
3 files changed, 40 insertions(+), 26 deletions(-)
diff --git a/hw/block/dataplane/xen-block.c b/hw/block/dataplane/xen-block.c
index 908bd27bbd..50094a886b 100644
--- a/hw/block/dataplane/xen-block.c
+++ b/hw/block/dataplane/xen-block.c
@@ -58,6 +58,7 @@ struct XenBlockDataPlane {
int requests_inflight;
unsigned int max_requests;
BlockBackend *blk;
+unsigned int sector_size;
QEMUBH *bh;
IOThread *iothread;
AioContext *ctx;
@@ -167,7 +168,7 @@ static int xen_block_parse_request(XenBlockRequest *request)
goto err;
}
-request->start = request->req.sector_number * XEN_BLKIF_SECTOR_SIZE;
+request->start = request->req.sector_number * dataplane->sector_size;
for (i = 0; i < request->req.nr_segments; i++) {
if (i == BLKIF_MAX_SEGMENTS_PER_REQUEST) {
error_report("error: nr_segments too big");
@@ -177,14 +178,14 @@ static int xen_block_parse_request(XenBlockRequest
*request)
error_report("error: first > last sector");
goto err;
}
-if (request->req.seg[i].last_sect * XEN_BLKIF_SECTOR_SIZE >=
+if (request->req.seg[i].last_sect * dataplane->sector_size >=
XC_PAGE_SIZE) {
error_report("error: page crossing");
goto err;
}
len = (request->req.seg[i].last_sect -
- request->req.seg[i].first_sect + 1) * XEN_BLKIF_SECTOR_SIZE;
+ request->req.seg[i].first_sect + 1) * dataplane->sector_size;
request->size += len;
}
if (request->start + request->size > blk_getlength(dataplane->blk)) {
@@ -218,17 +219,17 @@ static int xen_block_copy_request(XenBlockRequest
*request)
if (to_domain) {
segs[i].dest.foreign.ref = request->req.seg[i].gref;
segs[i].dest.foreign.offset = request->req.seg[i].first_sect *
-XEN_BLKIF_SECTOR_SIZE;
+dataplane->sector_size;
segs[i].source.virt = virt;
} else {
segs[i].source.foreign.ref = request->req.seg[i].gref;
segs[i].source.foreign.offset = request->req.seg[i].first_sect *
-XEN_BLKIF_SECTOR_SIZE;
+dataplane->sector_size;
segs[i].dest.virt = virt;
}
segs[i].len = (request->req.seg[i].last_sect -
request->req.seg[i].first_sect + 1) *
- XEN_BLKIF_SECTOR_SIZE;
+ dataplane->sector_size;
virt += segs[i].len;
}
@@ -338,12 +339,12 @@ static bool xen_block_split_discard(XenBlockRequest
*request,
/* Wrap around, or overflowing byte limit? */
if (sec_start + sec_count < sec_count ||
-sec_start + sec_count > INT64_MAX / XEN_BLKIF_SECTOR_SIZE) {
+sec_start + sec_count > INT64_MAX / dataplane->sector_size) {
return false;
}
-byte_offset = sec_start * XEN_BLKIF_SECTOR_SIZE;
-byte_remaining = sec_count * XEN_BLKIF_SECTOR_SIZE;
+byte_offset = sec_start * dataplane->sector_size;
+byte_remaining = sec_count * dataplane->sector_size;
do {
byte_chunk = byte_remaining > BDRV_REQUEST_MAX_BYTES ?
@@ -626,13 +627,15 @@ static bool xen_block_dataplane_event(void *opaque)
}
XenBlockDataPlane *xen_block_dataplane_create(XenDevice *xendev,
- BlockConf *conf,
+ BlockBackend *blk,
+ unsigned int sector_size,
IOThread *iothread)
{
XenBlockDataPlane *dataplane = g_new0(XenBlockDataPlane, 1);
dataplane->xendev = xendev;
-dataplane->blk =
