* [Qemu-devel] [PATCH] virtio-scsi: fix cdb/sense
@ 2015-03-11 9:18 Michael S. Tsirkin
2015-03-11 12:46 ` Alexey Kardashevskiy
0 siblings, 1 reply; 2+ messages in thread
From: Michael S. Tsirkin @ 2015-03-11 9:18 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Fam Zheng
Commit "virtio-scsi: use standard-headers" added
cdb and sense into req/rep structures, which
breaks uses of sizeof for these structures,
since qemu adds its own arrays on top.
To fix, replace sizeof with offsetof everywhere.
Reported-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Note: this is very lightly tested.
Help with testing will be very much appreciated!
include/hw/virtio/virtio-scsi.h | 6 +++---
hw/scsi/virtio-scsi.c | 28 ++++++++++++++++++----------
2 files changed, 21 insertions(+), 13 deletions(-)
diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index de2c739..25d96f3 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -136,15 +136,15 @@ typedef struct VirtIOSCSIReq {
union {
struct {
VirtIOSCSICmdReq cmd;
- uint8_t cdb[];
} QEMU_PACKED;
VirtIOSCSICtrlTMFReq tmf;
VirtIOSCSICtrlANReq an;
} req;
} VirtIOSCSIReq;
-QEMU_BUILD_BUG_ON(offsetof(VirtIOSCSIReq, req.cdb) !=
- offsetof(VirtIOSCSIReq, req.cmd) + sizeof(VirtIOSCSICmdReq));
+QEMU_BUILD_BUG_ON(offsetof(VirtIOSCSIReq, req.cmd.cdb) !=
+ offsetof(VirtIOSCSIReq, req.cmd) +
+ offsetof(VirtIOSCSICmdReq, cdb));
#define DEFINE_VIRTIO_SCSI_PROPERTIES(_state, _conf_field) \
DEFINE_PROP_UINT32("num_queues", _state, _conf_field.num_queues, 1), \
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index cfb52e8..52bc00c 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -47,7 +47,8 @@ VirtIOSCSIReq *virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq)
const size_t zero_skip = offsetof(VirtIOSCSIReq, elem)
+ sizeof(VirtQueueElement);
- req = g_slice_alloc(sizeof(*req) + vs->cdb_size);
+ req = g_slice_alloc(sizeof(*req) + MAX(vs->cdb_size, VIRTIO_SCSI_CDB_SIZE) -
+ VIRTIO_SCSI_CDB_SIZE);
req->vq = vq;
req->dev = s;
qemu_sglist_init(&req->qsgl, DEVICE(s), 8, &address_space_memory);
@@ -62,7 +63,8 @@ void virtio_scsi_free_req(VirtIOSCSIReq *req)
qemu_iovec_destroy(&req->resp_iov);
qemu_sglist_destroy(&req->qsgl);
- g_slice_free1(sizeof(*req) + vs->cdb_size, req);
+ g_slice_free1(sizeof(*req) + MAX(vs->cdb_size, VIRTIO_SCSI_CDB_SIZE) -
+ VIRTIO_SCSI_CDB_SIZE, req);
}
static void virtio_scsi_complete_req(VirtIOSCSIReq *req)
@@ -213,8 +215,10 @@ static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq)
assert(req->elem.in_num <= ARRAY_SIZE(req->elem.in_sg));
assert(req->elem.out_num <= ARRAY_SIZE(req->elem.out_sg));
- if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICmdReq) + vs->cdb_size,
- sizeof(VirtIOSCSICmdResp) + vs->sense_size) < 0) {
+ if (virtio_scsi_parse_req(req, offsetof(VirtIOSCSICmdReq, cdb) +
+ vs->cdb_size,
+ offsetof(VirtIOSCSICmdResp, sense) +
+ vs->sense_size) < 0) {
error_report("invalid SCSI request migration data");
exit(1);
}
@@ -439,7 +443,7 @@ static void virtio_scsi_complete_cmd_req(VirtIOSCSIReq *req)
/* Sense data is not in req->resp and is copied separately
* in virtio_scsi_command_complete.
*/
- req->resp_size = sizeof(VirtIOSCSICmdResp);
+ req->resp_size = offsetof(VirtIOSCSICmdResp, sense);
virtio_scsi_complete_req(req);
}
@@ -462,8 +466,10 @@ static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status,
} else {
req->resp.cmd.resid = 0;
sense_len = scsi_req_get_sense(r, sense, sizeof(sense));
- sense_len = MIN(sense_len, req->resp_iov.size - sizeof(req->resp.cmd));
- qemu_iovec_from_buf(&req->resp_iov, sizeof(req->resp.cmd),
+ sense_len = MIN(sense_len, req->resp_iov.size -
+ offsetof(typeof(req->resp.cmd), sense));
+ qemu_iovec_from_buf(&req->resp_iov,
+ offsetof(typeof(req->resp.cmd), sense),
sense, sense_len);
req->resp.cmd.sense_len = virtio_tswap32(vdev, sense_len);
}
@@ -522,8 +528,10 @@ bool virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req)
SCSIDevice *d;
int rc;
- rc = virtio_scsi_parse_req(req, sizeof(VirtIOSCSICmdReq) + vs->cdb_size,
- sizeof(VirtIOSCSICmdResp) + vs->sense_size);
+ rc = virtio_scsi_parse_req(req, offsetof(VirtIOSCSICmdReq, cdb) +
+ vs->cdb_size,
+ offsetof(VirtIOSCSICmdResp, sense) +
+ vs->sense_size);
if (rc < 0) {
if (rc == -ENOTSUP) {
virtio_scsi_fail_cmd_req(req);
@@ -544,7 +552,7 @@ bool virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req)
}
req->sreq = scsi_req_new(d, req->req.cmd.tag,
virtio_scsi_get_lun(req->req.cmd.lun),
- req->req.cdb, req);
+ req->req.cmd.cdb, req);
if (req->sreq->cmd.mode != SCSI_XFER_NONE
&& (req->sreq->cmd.mode != req->mode ||
--
MST
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [Qemu-devel] [PATCH] virtio-scsi: fix cdb/sense
2015-03-11 9:18 [Qemu-devel] [PATCH] virtio-scsi: fix cdb/sense Michael S. Tsirkin
@ 2015-03-11 12:46 ` Alexey Kardashevskiy
0 siblings, 0 replies; 2+ messages in thread
From: Alexey Kardashevskiy @ 2015-03-11 12:46 UTC (permalink / raw)
To: Michael S. Tsirkin, qemu-devel
Cc: Paolo Bonzini, Fam Zheng, Nikunj A Dadhania
On 03/11/2015 08:18 PM, Michael S. Tsirkin wrote:
> Commit "virtio-scsi: use standard-headers" added
> cdb and sense into req/rep structures, which
> breaks uses of sizeof for these structures,
> since qemu adds its own arrays on top.
>
> To fix, replace sizeof with offsetof everywhere.
This fixes breakage in SLOF on PPC64.
>
> Reported-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> Note: this is very lightly tested.
> Help with testing will be very much appreciated!
>
> include/hw/virtio/virtio-scsi.h | 6 +++---
> hw/scsi/virtio-scsi.c | 28 ++++++++++++++++++----------
> 2 files changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
> index de2c739..25d96f3 100644
> --- a/include/hw/virtio/virtio-scsi.h
> +++ b/include/hw/virtio/virtio-scsi.h
> @@ -136,15 +136,15 @@ typedef struct VirtIOSCSIReq {
> union {
> struct {
> VirtIOSCSICmdReq cmd;
> - uint8_t cdb[];
> } QEMU_PACKED;
> VirtIOSCSICtrlTMFReq tmf;
> VirtIOSCSICtrlANReq an;
> } req;
> } VirtIOSCSIReq;
>
> -QEMU_BUILD_BUG_ON(offsetof(VirtIOSCSIReq, req.cdb) !=
> - offsetof(VirtIOSCSIReq, req.cmd) + sizeof(VirtIOSCSICmdReq));
> +QEMU_BUILD_BUG_ON(offsetof(VirtIOSCSIReq, req.cmd.cdb) !=
> + offsetof(VirtIOSCSIReq, req.cmd) +
> + offsetof(VirtIOSCSICmdReq, cdb));
>
> #define DEFINE_VIRTIO_SCSI_PROPERTIES(_state, _conf_field) \
> DEFINE_PROP_UINT32("num_queues", _state, _conf_field.num_queues, 1), \
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index cfb52e8..52bc00c 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -47,7 +47,8 @@ VirtIOSCSIReq *virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq)
> const size_t zero_skip = offsetof(VirtIOSCSIReq, elem)
> + sizeof(VirtQueueElement);
>
> - req = g_slice_alloc(sizeof(*req) + vs->cdb_size);
> + req = g_slice_alloc(sizeof(*req) + MAX(vs->cdb_size, VIRTIO_SCSI_CDB_SIZE) -
> + VIRTIO_SCSI_CDB_SIZE);
> req->vq = vq;
> req->dev = s;
> qemu_sglist_init(&req->qsgl, DEVICE(s), 8, &address_space_memory);
> @@ -62,7 +63,8 @@ void virtio_scsi_free_req(VirtIOSCSIReq *req)
>
> qemu_iovec_destroy(&req->resp_iov);
> qemu_sglist_destroy(&req->qsgl);
> - g_slice_free1(sizeof(*req) + vs->cdb_size, req);
> + g_slice_free1(sizeof(*req) + MAX(vs->cdb_size, VIRTIO_SCSI_CDB_SIZE) -
> + VIRTIO_SCSI_CDB_SIZE, req);
> }
>
> static void virtio_scsi_complete_req(VirtIOSCSIReq *req)
> @@ -213,8 +215,10 @@ static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq)
> assert(req->elem.in_num <= ARRAY_SIZE(req->elem.in_sg));
> assert(req->elem.out_num <= ARRAY_SIZE(req->elem.out_sg));
>
> - if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICmdReq) + vs->cdb_size,
> - sizeof(VirtIOSCSICmdResp) + vs->sense_size) < 0) {
> + if (virtio_scsi_parse_req(req, offsetof(VirtIOSCSICmdReq, cdb) +
> + vs->cdb_size,
> + offsetof(VirtIOSCSICmdResp, sense) +
> + vs->sense_size) < 0) {
> error_report("invalid SCSI request migration data");
> exit(1);
> }
> @@ -439,7 +443,7 @@ static void virtio_scsi_complete_cmd_req(VirtIOSCSIReq *req)
> /* Sense data is not in req->resp and is copied separately
> * in virtio_scsi_command_complete.
> */
> - req->resp_size = sizeof(VirtIOSCSICmdResp);
> + req->resp_size = offsetof(VirtIOSCSICmdResp, sense);
> virtio_scsi_complete_req(req);
> }
>
> @@ -462,8 +466,10 @@ static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status,
> } else {
> req->resp.cmd.resid = 0;
> sense_len = scsi_req_get_sense(r, sense, sizeof(sense));
> - sense_len = MIN(sense_len, req->resp_iov.size - sizeof(req->resp.cmd));
> - qemu_iovec_from_buf(&req->resp_iov, sizeof(req->resp.cmd),
> + sense_len = MIN(sense_len, req->resp_iov.size -
> + offsetof(typeof(req->resp.cmd), sense));
> + qemu_iovec_from_buf(&req->resp_iov,
> + offsetof(typeof(req->resp.cmd), sense),
> sense, sense_len);
> req->resp.cmd.sense_len = virtio_tswap32(vdev, sense_len);
> }
> @@ -522,8 +528,10 @@ bool virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req)
> SCSIDevice *d;
> int rc;
>
> - rc = virtio_scsi_parse_req(req, sizeof(VirtIOSCSICmdReq) + vs->cdb_size,
> - sizeof(VirtIOSCSICmdResp) + vs->sense_size);
> + rc = virtio_scsi_parse_req(req, offsetof(VirtIOSCSICmdReq, cdb) +
> + vs->cdb_size,
> + offsetof(VirtIOSCSICmdResp, sense) +
> + vs->sense_size);
> if (rc < 0) {
> if (rc == -ENOTSUP) {
> virtio_scsi_fail_cmd_req(req);
> @@ -544,7 +552,7 @@ bool virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req)
> }
> req->sreq = scsi_req_new(d, req->req.cmd.tag,
> virtio_scsi_get_lun(req->req.cmd.lun),
> - req->req.cdb, req);
> + req->req.cmd.cdb, req);
>
> if (req->sreq->cmd.mode != SCSI_XFER_NONE
> && (req->sreq->cmd.mode != req->mode ||
>
--
Alexey
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-03-11 12:46 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-11 9:18 [Qemu-devel] [PATCH] virtio-scsi: fix cdb/sense Michael S. Tsirkin
2015-03-11 12:46 ` Alexey Kardashevskiy
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).