* [Qemu-devel] [PATCH] xen-block: support feature-large-sector-size
@ 2019-04-09 16:40 Paul Durrant
2019-04-09 16:40 ` Paul Durrant
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Paul Durrant @ 2019-04-09 16:40 UTC (permalink / raw)
To: qemu-devel, xen-devel, qemu-block
Cc: Paul Durrant, Stefano Stabellini, Anthony Perard,
Stefan Hajnoczi, Kevin Wolf, Max Reitz
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 <paul.durrant@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
---
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 = conf->blk;
+ dataplane->blk = blk;
+ dataplane->sector_size = sector_size;
QLIST_INIT(&dataplane->inflight);
QLIST_INIT(&dataplane->freelist);
diff --git a/hw/block/dataplane/xen-block.h b/hw/block/dataplane/xen-block.h
index d6fa6d26dd..76dcd51c3d 100644
--- a/hw/block/dataplane/xen-block.h
+++ b/hw/block/dataplane/xen-block.h
@@ -15,7 +15,8 @@
typedef struct XenBlockDataPlane XenBlockDataPlane;
XenBlockDataPlane *xen_block_dataplane_create(XenDevice *xendev,
- BlockConf *conf,
+ BlockBackend *blk,
+ unsigned int sector_size,
IOThread *iothread);
void xen_block_dataplane_destroy(XenBlockDataPlane *dataplane);
void xen_block_dataplane_start(XenBlockDataPlane *dataplane,
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)
XenBlockDevice *blockdev = XEN_BLOCK_DEVICE(xendev);
const char *type = object_get_typename(OBJECT(blockdev));
XenBlockVdev *vdev = &blockdev->props.vdev;
+ BlockConf *conf = &blockdev->props.conf;
+ unsigned int feature_large_sector_size;
unsigned int order, nr_ring_ref, *ring_ref, event_channel, protocol;
char *str;
trace_xen_block_connect(type, vdev->disk, vdev->partition);
+ 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",
+ XEN_BLKIF_SECTOR_SIZE);
+ return;
+ }
+
if (xen_device_frontend_scanf(xendev, "ring-page-order", "%u",
&order) != 1) {
nr_ring_ref = 1;
@@ -149,7 +163,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) / XEN_BLKIF_SECTOR_SIZE;
+ int64_t sectors = blk_getlength(conf->blk) / conf->logical_block_size;
XenDevice *xendev = XEN_DEVICE(blockdev);
trace_xen_block_size(type, vdev->disk, vdev->partition, sectors);
@@ -184,6 +198,7 @@ static void xen_block_realize(XenDevice *xendev, Error **errp)
const char *type = object_get_typename(OBJECT(blockdev));
XenBlockVdev *vdev = &blockdev->props.vdev;
BlockConf *conf = &blockdev->props.conf;
+ BlockBackend *blk = conf->blk;
Error *local_err = NULL;
if (vdev->type == XEN_BLOCK_VDEV_TYPE_INVALID) {
@@ -205,8 +220,8 @@ static void xen_block_realize(XenDevice *xendev, Error **errp)
* The blkif protocol does not deal with removable media, so it must
* always be present, even for CDRom devices.
*/
- assert(conf->blk);
- if (!blk_is_inserted(conf->blk)) {
+ assert(blk);
+ if (!blk_is_inserted(blk)) {
error_setg(errp, "device needs media, but drive is empty");
return;
}
@@ -223,26 +238,20 @@ static void xen_block_realize(XenDevice *xendev, Error **errp)
blkconf_blocksizes(conf);
- if (conf->logical_block_size != XEN_BLKIF_SECTOR_SIZE) {
- error_setg(errp, "logical_block_size != %u not supported",
- XEN_BLKIF_SECTOR_SIZE);
- return;
- }
-
if (conf->logical_block_size > conf->physical_block_size) {
error_setg(
errp, "logical_block_size > physical_block_size not supported");
return;
}
- blk_set_dev_ops(conf->blk, &xen_block_dev_ops, blockdev);
- blk_set_guest_block_size(conf->blk, conf->logical_block_size);
+ blk_set_dev_ops(blk, &xen_block_dev_ops, blockdev);
+ blk_set_guest_block_size(blk, conf->logical_block_size);
if (conf->discard_granularity == -1) {
conf->discard_granularity = conf->physical_block_size;
}
- if (blk_get_flags(conf->blk) & BDRV_O_UNMAP) {
+ if (blk_get_flags(blk) & BDRV_O_UNMAP) {
xen_device_backend_printf(xendev, "feature-discard", "%u", 1);
xen_device_backend_printf(xendev, "discard-granularity", "%u",
conf->discard_granularity);
@@ -259,12 +268,13 @@ static void xen_block_realize(XenDevice *xendev, Error **errp)
blockdev->device_type);
xen_device_backend_printf(xendev, "sector-size", "%u",
- XEN_BLKIF_SECTOR_SIZE);
+ conf->logical_block_size);
xen_block_set_size(blockdev);
blockdev->dataplane =
- xen_block_dataplane_create(xendev, conf, blockdev->props.iothread);
+ xen_block_dataplane_create(xendev, blk, conf->logical_block_size,
+ blockdev->props.iothread);
}
static void xen_block_frontend_changed(XenDevice *xendev,
--
2.20.1.2.gb21ebb6
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH] xen-block: support feature-large-sector-size
2019-04-09 16:40 [Qemu-devel] [PATCH] xen-block: support feature-large-sector-size Paul Durrant
@ 2019-04-09 16:40 ` Paul Durrant
2019-04-10 15:52 ` Anthony PERARD
2019-06-26 16:48 ` Max Reitz
2 siblings, 0 replies; 9+ messages in thread
From: Paul Durrant @ 2019-04-09 16:40 UTC (permalink / raw)
To: qemu-devel, xen-devel, qemu-block
Cc: Kevin Wolf, Stefano Stabellini, Max Reitz, Paul Durrant,
Stefan Hajnoczi, Anthony Perard
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 <paul.durrant@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
---
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 = conf->blk;
+ dataplane->blk = blk;
+ dataplane->sector_size = sector_size;
QLIST_INIT(&dataplane->inflight);
QLIST_INIT(&dataplane->freelist);
diff --git a/hw/block/dataplane/xen-block.h b/hw/block/dataplane/xen-block.h
index d6fa6d26dd..76dcd51c3d 100644
--- a/hw/block/dataplane/xen-block.h
+++ b/hw/block/dataplane/xen-block.h
@@ -15,7 +15,8 @@
typedef struct XenBlockDataPlane XenBlockDataPlane;
XenBlockDataPlane *xen_block_dataplane_create(XenDevice *xendev,
- BlockConf *conf,
+ BlockBackend *blk,
+ unsigned int sector_size,
IOThread *iothread);
void xen_block_dataplane_destroy(XenBlockDataPlane *dataplane);
void xen_block_dataplane_start(XenBlockDataPlane *dataplane,
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)
XenBlockDevice *blockdev = XEN_BLOCK_DEVICE(xendev);
const char *type = object_get_typename(OBJECT(blockdev));
XenBlockVdev *vdev = &blockdev->props.vdev;
+ BlockConf *conf = &blockdev->props.conf;
+ unsigned int feature_large_sector_size;
unsigned int order, nr_ring_ref, *ring_ref, event_channel, protocol;
char *str;
trace_xen_block_connect(type, vdev->disk, vdev->partition);
+ 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",
+ XEN_BLKIF_SECTOR_SIZE);
+ return;
+ }
+
if (xen_device_frontend_scanf(xendev, "ring-page-order", "%u",
&order) != 1) {
nr_ring_ref = 1;
@@ -149,7 +163,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) / XEN_BLKIF_SECTOR_SIZE;
+ int64_t sectors = blk_getlength(conf->blk) / conf->logical_block_size;
XenDevice *xendev = XEN_DEVICE(blockdev);
trace_xen_block_size(type, vdev->disk, vdev->partition, sectors);
@@ -184,6 +198,7 @@ static void xen_block_realize(XenDevice *xendev, Error **errp)
const char *type = object_get_typename(OBJECT(blockdev));
XenBlockVdev *vdev = &blockdev->props.vdev;
BlockConf *conf = &blockdev->props.conf;
+ BlockBackend *blk = conf->blk;
Error *local_err = NULL;
if (vdev->type == XEN_BLOCK_VDEV_TYPE_INVALID) {
@@ -205,8 +220,8 @@ static void xen_block_realize(XenDevice *xendev, Error **errp)
* The blkif protocol does not deal with removable media, so it must
* always be present, even for CDRom devices.
*/
- assert(conf->blk);
- if (!blk_is_inserted(conf->blk)) {
+ assert(blk);
+ if (!blk_is_inserted(blk)) {
error_setg(errp, "device needs media, but drive is empty");
return;
}
@@ -223,26 +238,20 @@ static void xen_block_realize(XenDevice *xendev, Error **errp)
blkconf_blocksizes(conf);
- if (conf->logical_block_size != XEN_BLKIF_SECTOR_SIZE) {
- error_setg(errp, "logical_block_size != %u not supported",
- XEN_BLKIF_SECTOR_SIZE);
- return;
- }
-
if (conf->logical_block_size > conf->physical_block_size) {
error_setg(
errp, "logical_block_size > physical_block_size not supported");
return;
}
- blk_set_dev_ops(conf->blk, &xen_block_dev_ops, blockdev);
- blk_set_guest_block_size(conf->blk, conf->logical_block_size);
+ blk_set_dev_ops(blk, &xen_block_dev_ops, blockdev);
+ blk_set_guest_block_size(blk, conf->logical_block_size);
if (conf->discard_granularity == -1) {
conf->discard_granularity = conf->physical_block_size;
}
- if (blk_get_flags(conf->blk) & BDRV_O_UNMAP) {
+ if (blk_get_flags(blk) & BDRV_O_UNMAP) {
xen_device_backend_printf(xendev, "feature-discard", "%u", 1);
xen_device_backend_printf(xendev, "discard-granularity", "%u",
conf->discard_granularity);
@@ -259,12 +268,13 @@ static void xen_block_realize(XenDevice *xendev, Error **errp)
blockdev->device_type);
xen_device_backend_printf(xendev, "sector-size", "%u",
- XEN_BLKIF_SECTOR_SIZE);
+ conf->logical_block_size);
xen_block_set_size(blockdev);
blockdev->dataplane =
- xen_block_dataplane_create(xendev, conf, blockdev->props.iothread);
+ xen_block_dataplane_create(xendev, blk, conf->logical_block_size,
+ blockdev->props.iothread);
}
static void xen_block_frontend_changed(XenDevice *xendev,
--
2.20.1.2.gb21ebb6
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] xen-block: support feature-large-sector-size
2019-04-09 16:40 [Qemu-devel] [PATCH] xen-block: support feature-large-sector-size Paul Durrant
2019-04-09 16:40 ` Paul Durrant
@ 2019-04-10 15:52 ` Anthony PERARD
2019-04-10 15:52 ` Anthony PERARD
2019-04-10 15:55 ` Paul Durrant
2019-06-26 16:48 ` Max Reitz
2 siblings, 2 replies; 9+ messages in thread
From: Anthony PERARD @ 2019-04-10 15:52 UTC (permalink / raw)
To: Paul Durrant
Cc: qemu-devel, xen-devel, qemu-block, Stefano Stabellini,
Stefan Hajnoczi, Kevin Wolf, Max Reitz
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 <paul.durrant@citrix.com>
> ---
> 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?
> + XEN_BLKIF_SECTOR_SIZE);
> + return;
> + }
> +
With the question answered:
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] xen-block: support feature-large-sector-size
2019-04-10 15:52 ` Anthony PERARD
@ 2019-04-10 15:52 ` Anthony PERARD
2019-04-10 15:55 ` Paul Durrant
1 sibling, 0 replies; 9+ messages in thread
From: Anthony PERARD @ 2019-04-10 15:52 UTC (permalink / raw)
To: Paul Durrant
Cc: Kevin Wolf, Stefano Stabellini, qemu-block, qemu-devel,
Max Reitz, Stefan Hajnoczi, xen-devel
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 <paul.durrant@citrix.com>
> ---
> 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?
> + XEN_BLKIF_SECTOR_SIZE);
> + return;
> + }
> +
With the question answered:
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] xen-block: support feature-large-sector-size
2019-04-10 15:52 ` Anthony PERARD
2019-04-10 15:52 ` Anthony PERARD
@ 2019-04-10 15:55 ` Paul Durrant
2019-04-10 15:55 ` Paul Durrant
1 sibling, 1 reply; 9+ messages in thread
From: Paul Durrant @ 2019-04-10 15:55 UTC (permalink / raw)
To: Anthony Perard
Cc: qemu-devel, xen-devel, qemu-block, Stefano Stabellini,
Stefan Hajnoczi, Kevin Wolf, Max Reitz
> -----Original Message-----
> From: Anthony PERARD [mailto:anthony.perard@citrix.com]
> Sent: 10 April 2019 16:52
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: qemu-devel@nongnu.org; xen-devel@lists.xenproject.org; qemu-block@nongnu.org; Stefano Stabellini
> <sstabellini@kernel.org>; Stefan Hajnoczi <stefanha@redhat.com>; Kevin Wolf <kwolf@redhat.com>; Max
> Reitz <mreitz@redhat.com>
> 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 <paul.durrant@citrix.com>
> > ---
> > 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 <anthony.perard@citrix.com>
>
Thanks,
Paul
> Thanks,
>
> --
> Anthony PERARD
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] xen-block: support feature-large-sector-size
2019-04-10 15:55 ` Paul Durrant
@ 2019-04-10 15:55 ` Paul Durrant
0 siblings, 0 replies; 9+ messages in thread
From: Paul Durrant @ 2019-04-10 15:55 UTC (permalink / raw)
To: Anthony Perard
Cc: Kevin Wolf, Stefano Stabellini, qemu-block, qemu-devel,
Max Reitz, Stefan Hajnoczi, xen-devel
> -----Original Message-----
> From: Anthony PERARD [mailto:anthony.perard@citrix.com]
> Sent: 10 April 2019 16:52
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: qemu-devel@nongnu.org; xen-devel@lists.xenproject.org; qemu-block@nongnu.org; Stefano Stabellini
> <sstabellini@kernel.org>; Stefan Hajnoczi <stefanha@redhat.com>; Kevin Wolf <kwolf@redhat.com>; Max
> Reitz <mreitz@redhat.com>
> 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 <paul.durrant@citrix.com>
> > ---
> > 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 <anthony.perard@citrix.com>
>
Thanks,
Paul
> Thanks,
>
> --
> Anthony PERARD
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] xen-block: support feature-large-sector-size
2019-04-09 16:40 [Qemu-devel] [PATCH] xen-block: support feature-large-sector-size Paul Durrant
2019-04-09 16:40 ` Paul Durrant
2019-04-10 15:52 ` Anthony PERARD
@ 2019-06-26 16:48 ` Max Reitz
2019-06-26 17:19 ` Anthony PERARD
2 siblings, 1 reply; 9+ messages in thread
From: Max Reitz @ 2019-06-26 16:48 UTC (permalink / raw)
To: Paul Durrant, qemu-devel, xen-devel, qemu-block
Cc: Anthony Perard, Kevin Wolf, Stefano Stabellini, Stefan Hajnoczi
[-- Attachment #1.1: Type: text/plain, Size: 1711 bytes --]
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 <paul.durrant@citrix.com>
> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Max Reitz <mreitz@redhat.com>
> ---
> 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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] xen-block: support feature-large-sector-size
2019-06-26 16:48 ` Max Reitz
@ 2019-06-26 17:19 ` Anthony PERARD
2019-06-26 17:48 ` Max Reitz
0 siblings, 1 reply; 9+ messages in thread
From: Anthony PERARD @ 2019-06-26 17:19 UTC (permalink / raw)
To: Max Reitz
Cc: Kevin Wolf, Stefano Stabellini, qemu-block, qemu-devel,
Paul Durrant, Stefan Hajnoczi, xen-devel
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 <paul.durrant@citrix.com>
> > ---
> > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > Cc: Anthony Perard <anthony.perard@citrix.com>
> > Cc: Stefan Hajnoczi <stefanha@redhat.com>
> > Cc: Kevin Wolf <kwolf@redhat.com>
> > Cc: Max Reitz <mreitz@redhat.com>
> > ---
> > 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.perard@citrix.com/20190624153257.20163-2-anthony.perard@citrix.com/
I guess I need to start sending an email every time I've added a patch
to my queue.
Cheers,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] xen-block: support feature-large-sector-size
2019-06-26 17:19 ` Anthony PERARD
@ 2019-06-26 17:48 ` Max Reitz
0 siblings, 0 replies; 9+ messages in thread
From: Max Reitz @ 2019-06-26 17:48 UTC (permalink / raw)
To: Anthony PERARD
Cc: Kevin Wolf, Stefano Stabellini, qemu-block, qemu-devel,
Paul Durrant, Stefan Hajnoczi, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 2410 bytes --]
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 <paul.durrant@citrix.com>
>>> ---
>>> Cc: Stefano Stabellini <sstabellini@kernel.org>
>>> Cc: Anthony Perard <anthony.perard@citrix.com>
>>> Cc: Stefan Hajnoczi <stefanha@redhat.com>
>>> Cc: Kevin Wolf <kwolf@redhat.com>
>>> Cc: Max Reitz <mreitz@redhat.com>
>>> ---
>>> 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.perard@citrix.com/20190624153257.20163-2-anthony.perard@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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-06-26 17:52 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-09 16:40 [Qemu-devel] [PATCH] xen-block: support feature-large-sector-size Paul Durrant
2019-04-09 16:40 ` Paul Durrant
2019-04-10 15:52 ` Anthony PERARD
2019-04-10 15:52 ` Anthony PERARD
2019-04-10 15:55 ` Paul Durrant
2019-04-10 15:55 ` Paul Durrant
2019-06-26 16:48 ` Max Reitz
2019-06-26 17:19 ` Anthony PERARD
2019-06-26 17:48 ` Max Reitz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).