* [Qemu-devel] [PATCH v2 0/2] dataplane: Enable "scsi=on"
@ 2014-05-22 7:37 Fam Zheng
2014-05-22 7:37 ` [Qemu-devel] [PATCH v2 1/2] virtio-blk: Factor out virtio_blk_handle_scsi_req from virtio_blk_handle_scsi Fam Zheng
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Fam Zheng @ 2014-05-22 7:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Stefan Hajnoczi
This makes the SG_IO code of non-dataplane available to dataplane, so that
dataplane can use to allow scsi=on.
v2:
[1/2] Fix scsi=off case and drop VirtIOBlockReq.scsi.
[2/2] Pass conf to virtio_blk_handle_scsi_req.
Fam
Fam Zheng (2):
virtio-blk: Factor out virtio_blk_handle_scsi_req from
virtio_blk_handle_scsi
dataplane: Support VIRTIO_BLK_T_SCSI_CMD
hw/block/dataplane/virtio-blk.c | 18 +++++----
hw/block/virtio-blk.c | 83 +++++++++++++++++++++++------------------
include/hw/virtio/virtio-blk.h | 4 ++
3 files changed, 60 insertions(+), 45 deletions(-)
--
1.9.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH v2 1/2] virtio-blk: Factor out virtio_blk_handle_scsi_req from virtio_blk_handle_scsi
2014-05-22 7:37 [Qemu-devel] [PATCH v2 0/2] dataplane: Enable "scsi=on" Fam Zheng
@ 2014-05-22 7:37 ` Fam Zheng
2014-05-22 7:58 ` Paolo Bonzini
2014-05-22 7:37 ` [Qemu-devel] [PATCH v2 2/2] dataplane: Support VIRTIO_BLK_T_SCSI_CMD Fam Zheng
2014-05-22 12:48 ` [Qemu-devel] [PATCH v2 0/2] dataplane: Enable "scsi=on" Stefan Hajnoczi
2 siblings, 1 reply; 5+ messages in thread
From: Fam Zheng @ 2014-05-22 7:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Stefan Hajnoczi
The common logic to process a scsi request in a VirtQueueElement is
extracted to a function to share with dataplane.
This makes VirtIOBlockReq.scsi unused, so drop it.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
hw/block/virtio-blk.c | 83 +++++++++++++++++++++++-------------------
include/hw/virtio/virtio-blk.h | 4 ++
2 files changed, 50 insertions(+), 37 deletions(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 5e9433d..78e81a6 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -33,7 +33,6 @@ typedef struct VirtIOBlockReq
VirtQueueElement elem;
struct virtio_blk_inhdr *in;
struct virtio_blk_outhdr *out;
- struct virtio_scsi_inhdr *scsi;
QEMUIOVector qiov;
struct VirtIOBlockReq *next;
BlockAcctCookie acct;
@@ -125,13 +124,16 @@ static VirtIOBlockReq *virtio_blk_get_request(VirtIOBlock *s)
return req;
}
-static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
+int virtio_blk_handle_scsi_req(BlockDriverState *bs,
+ VirtIOBlkConf *conf,
+ VirtQueueElement *elem)
{
+ int status = VIRTIO_BLK_S_OK;
+ struct virtio_scsi_inhdr *scsi = NULL;
#ifdef __linux__
- int ret;
int i;
+ struct sg_io_hdr hdr;
#endif
- int status = VIRTIO_BLK_S_OK;
/*
* We require at least one output segment each for the virtio_blk_outhdr
@@ -140,63 +142,61 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
* We also at least require the virtio_blk_inhdr, the virtio_scsi_inhdr
* and the sense buffer pointer in the input segments.
*/
- if (req->elem.out_num < 2 || req->elem.in_num < 3) {
- virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR);
- g_free(req);
- return;
+ if (elem->out_num < 2 || elem->in_num < 3) {
+ status = VIRTIO_BLK_S_IOERR;
+ goto fail;
}
/*
* The scsi inhdr is placed in the second-to-last input segment, just
* before the regular inhdr.
*/
- req->scsi = (void *)req->elem.in_sg[req->elem.in_num - 2].iov_base;
+ scsi = (void *)elem->in_sg[elem->in_num - 2].iov_base;
- if (!req->dev->blk.scsi) {
+ /*
+ * No support for bidirection commands yet.
+ */
+ if (elem->out_num > 2 && elem->in_num > 3) {
status = VIRTIO_BLK_S_UNSUPP;
goto fail;
}
- /*
- * No support for bidirection commands yet.
- */
- if (req->elem.out_num > 2 && req->elem.in_num > 3) {
+ if (!conf->scsi) {
status = VIRTIO_BLK_S_UNSUPP;
goto fail;
}
#ifdef __linux__
- struct sg_io_hdr hdr;
memset(&hdr, 0, sizeof(struct sg_io_hdr));
hdr.interface_id = 'S';
- hdr.cmd_len = req->elem.out_sg[1].iov_len;
- hdr.cmdp = req->elem.out_sg[1].iov_base;
+ hdr.cmd_len = elem->out_sg[1].iov_len;
+ hdr.cmdp = elem->out_sg[1].iov_base;
hdr.dxfer_len = 0;
- if (req->elem.out_num > 2) {
+ if (elem->out_num > 2) {
/*
* If there are more than the minimally required 2 output segments
* there is write payload starting from the third iovec.
*/
hdr.dxfer_direction = SG_DXFER_TO_DEV;
- hdr.iovec_count = req->elem.out_num - 2;
+ hdr.iovec_count = elem->out_num - 2;
for (i = 0; i < hdr.iovec_count; i++)
- hdr.dxfer_len += req->elem.out_sg[i + 2].iov_len;
+ hdr.dxfer_len += elem->out_sg[i + 2].iov_len;
- hdr.dxferp = req->elem.out_sg + 2;
+ hdr.dxferp = elem->out_sg + 2;
- } else if (req->elem.in_num > 3) {
+ } else if (elem->in_num > 3) {
/*
* If we have more than 3 input segments the guest wants to actually
* read data.
*/
hdr.dxfer_direction = SG_DXFER_FROM_DEV;
- hdr.iovec_count = req->elem.in_num - 3;
+ hdr.iovec_count = elem->in_num - 3;
for (i = 0; i < hdr.iovec_count; i++)
- hdr.dxfer_len += req->elem.in_sg[i].iov_len;
+ hdr.dxfer_len += elem->in_sg[i].iov_len;
- hdr.dxferp = req->elem.in_sg;
+ hdr.dxferp = elem->in_sg;
} else {
/*
* Some SCSI commands don't actually transfer any data.
@@ -204,11 +204,11 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
hdr.dxfer_direction = SG_DXFER_NONE;
}
- hdr.sbp = req->elem.in_sg[req->elem.in_num - 3].iov_base;
- hdr.mx_sb_len = req->elem.in_sg[req->elem.in_num - 3].iov_len;
+ hdr.sbp = elem->in_sg[elem->in_num - 3].iov_base;
+ hdr.mx_sb_len = elem->in_sg[elem->in_num - 3].iov_len;
- ret = bdrv_ioctl(req->dev->bs, SG_IO, &hdr);
- if (ret) {
+ status = bdrv_ioctl(bs, SG_IO, &hdr);
+ if (status) {
status = VIRTIO_BLK_S_UNSUPP;
goto fail;
}
@@ -224,23 +224,32 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
hdr.status = CHECK_CONDITION;
}
- stl_p(&req->scsi->errors,
+ stl_p(&scsi->errors,
hdr.status | (hdr.msg_status << 8) |
(hdr.host_status << 16) | (hdr.driver_status << 24));
- stl_p(&req->scsi->residual, hdr.resid);
- stl_p(&req->scsi->sense_len, hdr.sb_len_wr);
- stl_p(&req->scsi->data_len, hdr.dxfer_len);
+ stl_p(&scsi->residual, hdr.resid);
+ stl_p(&scsi->sense_len, hdr.sb_len_wr);
+ stl_p(&scsi->data_len, hdr.dxfer_len);
- virtio_blk_req_complete(req, status);
- g_free(req);
- return;
+ return status;
#else
abort();
#endif
fail:
/* Just put anything nonzero so that the ioctl fails in the guest. */
- stl_p(&req->scsi->errors, 255);
+ if (scsi) {
+ stl_p(&scsi->errors, 255);
+ }
+ return status;
+}
+
+static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
+{
+ int status;
+
+ status = virtio_blk_handle_scsi_req(req->dev->bs, &req->dev->blk,
+ &req->elem);
virtio_blk_req_complete(req, status);
g_free(req);
}
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index e4c41ff..1b00b48 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -155,4 +155,8 @@ typedef struct VirtIOBlock {
void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk);
+int virtio_blk_handle_scsi_req(BlockDriverState *bs,
+ VirtIOBlkConf *conf,
+ VirtQueueElement *elem);
+
#endif
--
1.9.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH v2 2/2] dataplane: Support VIRTIO_BLK_T_SCSI_CMD
2014-05-22 7:37 [Qemu-devel] [PATCH v2 0/2] dataplane: Enable "scsi=on" Fam Zheng
2014-05-22 7:37 ` [Qemu-devel] [PATCH v2 1/2] virtio-blk: Factor out virtio_blk_handle_scsi_req from virtio_blk_handle_scsi Fam Zheng
@ 2014-05-22 7:37 ` Fam Zheng
2014-05-22 12:48 ` [Qemu-devel] [PATCH v2 0/2] dataplane: Enable "scsi=on" Stefan Hajnoczi
2 siblings, 0 replies; 5+ messages in thread
From: Fam Zheng @ 2014-05-22 7:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Stefan Hajnoczi
Signed-off-by: Fam Zheng <famz@redhat.com>
---
hw/block/dataplane/virtio-blk.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 46a6824..e833045 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -201,6 +201,15 @@ static void do_flush_cmd(VirtIOBlockDataPlane *s, VirtQueueElement *elem,
bdrv_aio_flush(s->blk->conf.bs, complete_flush, req);
}
+static void do_scsi_cmd(VirtIOBlockDataPlane *s, VirtQueueElement *elem,
+ QEMUIOVector *inhdr)
+{
+ int status;
+
+ status = virtio_blk_handle_scsi_req(s->blk->conf.bs, s->blk, elem);
+ complete_request_early(s, elem, inhdr, status);
+}
+
static int process_request(VirtIOBlockDataPlane *s, VirtQueueElement *elem)
{
struct iovec *iov = elem->out_sg;
@@ -249,8 +258,7 @@ static int process_request(VirtIOBlockDataPlane *s, VirtQueueElement *elem)
return 0;
case VIRTIO_BLK_T_SCSI_CMD:
- /* TODO support SCSI commands */
- complete_request_early(s, elem, inhdr, VIRTIO_BLK_S_UNSUPP);
+ do_scsi_cmd(s, elem, inhdr);
return 0;
case VIRTIO_BLK_T_FLUSH:
@@ -326,12 +334,6 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk,
return;
}
- if (blk->scsi) {
- error_setg(errp,
- "device is incompatible with x-data-plane, use scsi=off");
- return;
- }
-
/* If dataplane is (re-)enabled while the guest is running there could be
* block jobs that can conflict.
*/
--
1.9.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] virtio-blk: Factor out virtio_blk_handle_scsi_req from virtio_blk_handle_scsi
2014-05-22 7:37 ` [Qemu-devel] [PATCH v2 1/2] virtio-blk: Factor out virtio_blk_handle_scsi_req from virtio_blk_handle_scsi Fam Zheng
@ 2014-05-22 7:58 ` Paolo Bonzini
0 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2014-05-22 7:58 UTC (permalink / raw)
To: Fam Zheng, qemu-devel; +Cc: Stefan Hajnoczi
Il 22/05/2014 09:37, Fam Zheng ha scritto:
> -static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
> +int virtio_blk_handle_scsi_req(BlockDriverState *bs,
> + VirtIOBlkConf *conf,
> + VirtQueueElement *elem)
Two more comments...
please pass the VirtIOBlock * instead of bs/conf here.
> {
> + int status = VIRTIO_BLK_S_OK;
> + struct virtio_scsi_inhdr *scsi = NULL;
> #ifdef __linux__
> - int ret;
> int i;
> + struct sg_io_hdr hdr;
> #endif
> - int status = VIRTIO_BLK_S_OK;
>
> /*
> * We require at least one output segment each for the virtio_blk_outhdr
> @@ -140,63 +142,61 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
> * We also at least require the virtio_blk_inhdr, the virtio_scsi_inhdr
> * and the sense buffer pointer in the input segments.
> */
> - if (req->elem.out_num < 2 || req->elem.in_num < 3) {
> - virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR);
> - g_free(req);
> - return;
> + if (elem->out_num < 2 || elem->in_num < 3) {
> + status = VIRTIO_BLK_S_IOERR;
> + goto fail;
> }
>
> /*
> * The scsi inhdr is placed in the second-to-last input segment, just
> * before the regular inhdr.
> */
> - req->scsi = (void *)req->elem.in_sg[req->elem.in_num - 2].iov_base;
> + scsi = (void *)elem->in_sg[elem->in_num - 2].iov_base;
>
> - if (!req->dev->blk.scsi) {
> + /*
> + * No support for bidirection commands yet.
> + */
> + if (elem->out_num > 2 && elem->in_num > 3) {
> status = VIRTIO_BLK_S_UNSUPP;
> goto fail;
> }
>
> - /*
> - * No support for bidirection commands yet.
> - */
> - if (req->elem.out_num > 2 && req->elem.in_num > 3) {
> + if (!conf->scsi) {
> status = VIRTIO_BLK_S_UNSUPP;
> goto fail;
> }
Swapping the two conditionals unnecessarily makes the patch bigger.
Paolo
> #ifdef __linux__
> - struct sg_io_hdr hdr;
> memset(&hdr, 0, sizeof(struct sg_io_hdr));
> hdr.interface_id = 'S';
> - hdr.cmd_len = req->elem.out_sg[1].iov_len;
> - hdr.cmdp = req->elem.out_sg[1].iov_base;
> + hdr.cmd_len = elem->out_sg[1].iov_len;
> + hdr.cmdp = elem->out_sg[1].iov_base;
> hdr.dxfer_len = 0;
>
> - if (req->elem.out_num > 2) {
> + if (elem->out_num > 2) {
> /*
> * If there are more than the minimally required 2 output segments
> * there is write payload starting from the third iovec.
> */
> hdr.dxfer_direction = SG_DXFER_TO_DEV;
> - hdr.iovec_count = req->elem.out_num - 2;
> + hdr.iovec_count = elem->out_num - 2;
>
> for (i = 0; i < hdr.iovec_count; i++)
> - hdr.dxfer_len += req->elem.out_sg[i + 2].iov_len;
> + hdr.dxfer_len += elem->out_sg[i + 2].iov_len;
>
> - hdr.dxferp = req->elem.out_sg + 2;
> + hdr.dxferp = elem->out_sg + 2;
>
> - } else if (req->elem.in_num > 3) {
> + } else if (elem->in_num > 3) {
> /*
> * If we have more than 3 input segments the guest wants to actually
> * read data.
> */
> hdr.dxfer_direction = SG_DXFER_FROM_DEV;
> - hdr.iovec_count = req->elem.in_num - 3;
> + hdr.iovec_count = elem->in_num - 3;
> for (i = 0; i < hdr.iovec_count; i++)
> - hdr.dxfer_len += req->elem.in_sg[i].iov_len;
> + hdr.dxfer_len += elem->in_sg[i].iov_len;
>
> - hdr.dxferp = req->elem.in_sg;
> + hdr.dxferp = elem->in_sg;
> } else {
> /*
> * Some SCSI commands don't actually transfer any data.
> @@ -204,11 +204,11 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
> hdr.dxfer_direction = SG_DXFER_NONE;
> }
>
> - hdr.sbp = req->elem.in_sg[req->elem.in_num - 3].iov_base;
> - hdr.mx_sb_len = req->elem.in_sg[req->elem.in_num - 3].iov_len;
> + hdr.sbp = elem->in_sg[elem->in_num - 3].iov_base;
> + hdr.mx_sb_len = elem->in_sg[elem->in_num - 3].iov_len;
>
> - ret = bdrv_ioctl(req->dev->bs, SG_IO, &hdr);
> - if (ret) {
> + status = bdrv_ioctl(bs, SG_IO, &hdr);
> + if (status) {
> status = VIRTIO_BLK_S_UNSUPP;
> goto fail;
> }
> @@ -224,23 +224,32 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
> hdr.status = CHECK_CONDITION;
> }
>
> - stl_p(&req->scsi->errors,
> + stl_p(&scsi->errors,
> hdr.status | (hdr.msg_status << 8) |
> (hdr.host_status << 16) | (hdr.driver_status << 24));
> - stl_p(&req->scsi->residual, hdr.resid);
> - stl_p(&req->scsi->sense_len, hdr.sb_len_wr);
> - stl_p(&req->scsi->data_len, hdr.dxfer_len);
> + stl_p(&scsi->residual, hdr.resid);
> + stl_p(&scsi->sense_len, hdr.sb_len_wr);
> + stl_p(&scsi->data_len, hdr.dxfer_len);
>
> - virtio_blk_req_complete(req, status);
> - g_free(req);
> - return;
> + return status;
> #else
> abort();
> #endif
>
> fail:
> /* Just put anything nonzero so that the ioctl fails in the guest. */
> - stl_p(&req->scsi->errors, 255);
> + if (scsi) {
> + stl_p(&scsi->errors, 255);
> + }
> + return status;
> +}
> +
> +static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
> +{
> + int status;
> +
> + status = virtio_blk_handle_scsi_req(req->dev->bs, &req->dev->blk,
> + &req->elem);
> virtio_blk_req_complete(req, status);
> g_free(req);
> }
> diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
> index e4c41ff..1b00b48 100644
> --- a/include/hw/virtio/virtio-blk.h
> +++ b/include/hw/virtio/virtio-blk.h
> @@ -155,4 +155,8 @@ typedef struct VirtIOBlock {
>
> void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk);
>
> +int virtio_blk_handle_scsi_req(BlockDriverState *bs,
> + VirtIOBlkConf *conf,
> + VirtQueueElement *elem);
> +
> #endif
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/2] dataplane: Enable "scsi=on"
2014-05-22 7:37 [Qemu-devel] [PATCH v2 0/2] dataplane: Enable "scsi=on" Fam Zheng
2014-05-22 7:37 ` [Qemu-devel] [PATCH v2 1/2] virtio-blk: Factor out virtio_blk_handle_scsi_req from virtio_blk_handle_scsi Fam Zheng
2014-05-22 7:37 ` [Qemu-devel] [PATCH v2 2/2] dataplane: Support VIRTIO_BLK_T_SCSI_CMD Fam Zheng
@ 2014-05-22 12:48 ` Stefan Hajnoczi
2 siblings, 0 replies; 5+ messages in thread
From: Stefan Hajnoczi @ 2014-05-22 12:48 UTC (permalink / raw)
To: Fam Zheng; +Cc: Paolo Bonzini, qemu-devel
On Thu, May 22, 2014 at 03:37:10PM +0800, Fam Zheng wrote:
> This makes the SG_IO code of non-dataplane available to dataplane, so that
> dataplane can use to allow scsi=on.
>
> v2:
> [1/2] Fix scsi=off case and drop VirtIOBlockReq.scsi.
> [2/2] Pass conf to virtio_blk_handle_scsi_req.
>
> Fam
>
>
> Fam Zheng (2):
> virtio-blk: Factor out virtio_blk_handle_scsi_req from
> virtio_blk_handle_scsi
> dataplane: Support VIRTIO_BLK_T_SCSI_CMD
>
> hw/block/dataplane/virtio-blk.c | 18 +++++----
> hw/block/virtio-blk.c | 83 +++++++++++++++++++++++------------------
> include/hw/virtio/virtio-blk.h | 4 ++
> 3 files changed, 60 insertions(+), 45 deletions(-)
I wonder if it's okay to use synchronous bdrv_ioctl() but the existing
code already does it. It would be bad to block the thread.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-05-22 12:49 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-22 7:37 [Qemu-devel] [PATCH v2 0/2] dataplane: Enable "scsi=on" Fam Zheng
2014-05-22 7:37 ` [Qemu-devel] [PATCH v2 1/2] virtio-blk: Factor out virtio_blk_handle_scsi_req from virtio_blk_handle_scsi Fam Zheng
2014-05-22 7:58 ` Paolo Bonzini
2014-05-22 7:37 ` [Qemu-devel] [PATCH v2 2/2] dataplane: Support VIRTIO_BLK_T_SCSI_CMD Fam Zheng
2014-05-22 12:48 ` [Qemu-devel] [PATCH v2 0/2] dataplane: Enable "scsi=on" Stefan Hajnoczi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.