* [Qemu-devel] [PULL 1/5] Fix iSCSI crash on SG_IO with an iovector
2013-07-17 15:05 [Qemu-devel] [PULL 0/5] SCSI changes for 2013-07-17 Paolo Bonzini
@ 2013-07-17 15:05 ` Paolo Bonzini
2013-07-17 15:05 ` [Qemu-devel] [PULL 2/5] iscsi: fix -ENOSPC in iscsi_create() Paolo Bonzini
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2013-07-17 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Ronnie Sahlberg
From: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Don't assume that SG_IO is always invoked with a simple buffer,
check the iovec_count and if it is >= 1 then we need to pass an array
of iovectors to libiscsi instead of just a plain buffer.
Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/iscsi.c | 51 ++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 44 insertions(+), 7 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index 0bbf0b1..fa5252c 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -32,6 +32,7 @@
#include "block/block_int.h"
#include "trace.h"
#include "block/scsi.h"
+#include "qemu/iov.h"
#include <iscsi/iscsi.h>
#include <iscsi/scsi-lowlevel.h>
@@ -651,6 +652,9 @@ iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status,
{
IscsiAIOCB *acb = opaque;
+ g_free(acb->buf);
+ acb->buf = NULL;
+
if (acb->canceled != 0) {
return;
}
@@ -727,14 +731,30 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
memcpy(&acb->task->cdb[0], acb->ioh->cmdp, acb->ioh->cmd_len);
acb->task->expxferlen = acb->ioh->dxfer_len;
+ data.size = 0;
if (acb->task->xfer_dir == SCSI_XFER_WRITE) {
- data.data = acb->ioh->dxferp;
- data.size = acb->ioh->dxfer_len;
+ if (acb->ioh->iovec_count == 0) {
+ data.data = acb->ioh->dxferp;
+ data.size = acb->ioh->dxfer_len;
+ } else {
+#if defined(LIBISCSI_FEATURE_IOVECTOR)
+ scsi_task_set_iov_out(acb->task,
+ (struct scsi_iovec *) acb->ioh->dxferp,
+ acb->ioh->iovec_count);
+#else
+ struct iovec *iov = (struct iovec *)acb->ioh->dxferp;
+
+ acb->buf = g_malloc(acb->ioh->dxfer_len);
+ data.data = acb->buf;
+ data.size = iov_to_buf(iov, acb->ioh->iovec_count, 0,
+ acb->buf, acb->ioh->dxfer_len);
+#endif
+ }
}
+
if (iscsi_scsi_command_async(iscsi, iscsilun->lun, acb->task,
iscsi_aio_ioctl_cb,
- (acb->task->xfer_dir == SCSI_XFER_WRITE) ?
- &data : NULL,
+ (data.size > 0) ? &data : NULL,
acb) != 0) {
scsi_free_scsi_task(acb->task);
qemu_aio_release(acb);
@@ -743,9 +763,26 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
/* tell libiscsi to read straight into the buffer we got from ioctl */
if (acb->task->xfer_dir == SCSI_XFER_READ) {
- scsi_task_add_data_in_buffer(acb->task,
- acb->ioh->dxfer_len,
- acb->ioh->dxferp);
+ if (acb->ioh->iovec_count == 0) {
+ scsi_task_add_data_in_buffer(acb->task,
+ acb->ioh->dxfer_len,
+ acb->ioh->dxferp);
+ } else {
+#if defined(LIBISCSI_FEATURE_IOVECTOR)
+ scsi_task_set_iov_in(acb->task,
+ (struct scsi_iovec *) acb->ioh->dxferp,
+ acb->ioh->iovec_count);
+#else
+ int i;
+ for (i = 0; i < acb->ioh->iovec_count; i++) {
+ struct iovec *iov = (struct iovec *)acb->ioh->dxferp;
+
+ scsi_task_add_data_in_buffer(acb->task,
+ iov[i].iov_len,
+ iov[i].iov_base);
+ }
+#endif
+ }
}
iscsi_set_events(iscsilun);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PULL 2/5] iscsi: fix -ENOSPC in iscsi_create()
2013-07-17 15:05 [Qemu-devel] [PULL 0/5] SCSI changes for 2013-07-17 Paolo Bonzini
2013-07-17 15:05 ` [Qemu-devel] [PULL 1/5] Fix iSCSI crash on SG_IO with an iovector Paolo Bonzini
@ 2013-07-17 15:05 ` Paolo Bonzini
2013-07-17 15:05 ` [Qemu-devel] [PULL 3/5] iscsi: remove support for misaligned nb_sectors in aio_readv Paolo Bonzini
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2013-07-17 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Lieven, qemu-stable
From: Peter Lieven <pl@kamp.de>
the -ENOPSC case did not work due to the missing goto.
Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Peter Lieven <pl@kamp.de>
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/iscsi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/block/iscsi.c b/block/iscsi.c
index fa5252c..91b602c 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1272,6 +1272,7 @@ static int iscsi_create(const char *filename, QEMUOptionParameter *options)
}
if (bs.total_sectors < total_size) {
ret = -ENOSPC;
+ goto out;
}
ret = 0;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PULL 3/5] iscsi: remove support for misaligned nb_sectors in aio_readv
2013-07-17 15:05 [Qemu-devel] [PULL 0/5] SCSI changes for 2013-07-17 Paolo Bonzini
2013-07-17 15:05 ` [Qemu-devel] [PULL 1/5] Fix iSCSI crash on SG_IO with an iovector Paolo Bonzini
2013-07-17 15:05 ` [Qemu-devel] [PULL 2/5] iscsi: fix -ENOSPC in iscsi_create() Paolo Bonzini
@ 2013-07-17 15:05 ` Paolo Bonzini
2013-07-17 15:05 ` [Qemu-devel] [PULL 4/5] iscsi: assert that sectors are aligned to LUN blocksize Paolo Bonzini
2013-07-17 15:05 ` [Qemu-devel] [PULL 5/5] iscsi: factor out sector conversions Paolo Bonzini
4 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2013-07-17 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Lieven, qemu-stable
From: Peter Lieven <pl@kamp.de>
this hask is not working (anymore). support for misaligned offsets should
be handled at the block layer.
Signed-off-by: Peter Lieven <pl@kamp.de>
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/iscsi.c | 22 ++++------------------
1 file changed, 4 insertions(+), 18 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index 91b602c..df283ed 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -62,8 +62,6 @@ typedef struct IscsiAIOCB {
int status;
int canceled;
int retries;
- size_t read_size;
- size_t read_offset;
int64_t sector_num;
int nb_sectors;
#ifdef __linux__
@@ -380,6 +378,7 @@ static int
iscsi_aio_readv_acb(IscsiAIOCB *acb)
{
struct iscsi_context *iscsi = acb->iscsilun->iscsi;
+ size_t size;
uint64_t lba;
uint32_t num_sectors;
int ret;
@@ -392,20 +391,7 @@ iscsi_aio_readv_acb(IscsiAIOCB *acb)
acb->status = -EINPROGRESS;
acb->buf = NULL;
- /* If LUN blocksize is bigger than BDRV_BLOCK_SIZE a read from QEMU
- * may be misaligned to the LUN, so we may need to read some extra
- * data.
- */
- acb->read_offset = 0;
- if (acb->iscsilun->block_size > BDRV_SECTOR_SIZE) {
- uint64_t bdrv_offset = BDRV_SECTOR_SIZE * acb->sector_num;
-
- acb->read_offset = bdrv_offset % acb->iscsilun->block_size;
- }
-
- num_sectors = (acb->read_size + acb->iscsilun->block_size
- + acb->read_offset - 1)
- / acb->iscsilun->block_size;
+ size = acb->nb_sectors * BDRV_SECTOR_SIZE;
acb->task = malloc(sizeof(struct scsi_task));
if (acb->task == NULL) {
@@ -416,8 +402,9 @@ iscsi_aio_readv_acb(IscsiAIOCB *acb)
memset(acb->task, 0, sizeof(struct scsi_task));
acb->task->xfer_dir = SCSI_XFER_READ;
+ acb->task->expxferlen = size;
lba = sector_qemu2lun(acb->sector_num, acb->iscsilun);
- acb->task->expxferlen = acb->read_size;
+ num_sectors = sector_qemu2lun(acb->nb_sectors, acb->iscsilun);
switch (acb->iscsilun->type) {
case TYPE_DISK:
@@ -472,7 +459,6 @@ iscsi_aio_readv(BlockDriverState *bs, int64_t sector_num,
acb->sector_num = sector_num;
acb->iscsilun = iscsilun;
acb->qiov = qiov;
- acb->read_size = BDRV_SECTOR_SIZE * (size_t)acb->nb_sectors;
acb->retries = ISCSI_CMD_RETRIES;
if (iscsi_aio_readv_acb(acb) != 0) {
--
1.8.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PULL 4/5] iscsi: assert that sectors are aligned to LUN blocksize
2013-07-17 15:05 [Qemu-devel] [PULL 0/5] SCSI changes for 2013-07-17 Paolo Bonzini
` (2 preceding siblings ...)
2013-07-17 15:05 ` [Qemu-devel] [PULL 3/5] iscsi: remove support for misaligned nb_sectors in aio_readv Paolo Bonzini
@ 2013-07-17 15:05 ` Paolo Bonzini
2013-07-17 15:05 ` [Qemu-devel] [PULL 5/5] iscsi: factor out sector conversions Paolo Bonzini
4 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2013-07-17 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Lieven, qemu-stable
From: Peter Lieven <pl@kamp.de>
if the blocksize of an iSCSI LUN is bigger than the BDRV_SECTOR_SIZE
it is possible that sector_num or nb_sectors are not correctly
aligned.
to avoid corruption we fail requests which are misaligned.
Signed-off-by: Peter Lieven <pl@kamp.de>
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/iscsi.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/block/iscsi.c b/block/iscsi.c
index df283ed..1294fdf 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -237,6 +237,18 @@ static int64_t sector_qemu2lun(int64_t sector, IscsiLun *iscsilun)
return sector * BDRV_SECTOR_SIZE / iscsilun->block_size;
}
+static bool is_request_lun_aligned(int64_t sector_num, int nb_sectors,
+ IscsiLun *iscsilun)
+{
+ if ((sector_num * BDRV_SECTOR_SIZE) % iscsilun->block_size ||
+ (nb_sectors * BDRV_SECTOR_SIZE) % iscsilun->block_size) {
+ error_report("iSCSI misaligned request: iscsilun->block_size %u, sector_num %ld, nb_sectors %d",
+ iscsilun->block_size, sector_num, nb_sectors);
+ return 0;
+ }
+ return 1;
+}
+
static int
iscsi_aio_writev_acb(IscsiAIOCB *acb)
{
@@ -321,6 +333,10 @@ iscsi_aio_writev(BlockDriverState *bs, int64_t sector_num,
IscsiLun *iscsilun = bs->opaque;
IscsiAIOCB *acb;
+ if (!is_request_lun_aligned(sector_num, nb_sectors, iscsilun)) {
+ return NULL;
+ }
+
acb = qemu_aio_get(&iscsi_aiocb_info, bs, cb, opaque);
trace_iscsi_aio_writev(iscsilun->iscsi, sector_num, nb_sectors, opaque, acb);
@@ -452,6 +468,10 @@ iscsi_aio_readv(BlockDriverState *bs, int64_t sector_num,
IscsiLun *iscsilun = bs->opaque;
IscsiAIOCB *acb;
+ if (!is_request_lun_aligned(sector_num, nb_sectors, iscsilun)) {
+ return NULL;
+ }
+
acb = qemu_aio_get(&iscsi_aiocb_info, bs, cb, opaque);
trace_iscsi_aio_readv(iscsilun->iscsi, sector_num, nb_sectors, opaque, acb);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PULL 5/5] iscsi: factor out sector conversions
2013-07-17 15:05 [Qemu-devel] [PULL 0/5] SCSI changes for 2013-07-17 Paolo Bonzini
` (3 preceding siblings ...)
2013-07-17 15:05 ` [Qemu-devel] [PULL 4/5] iscsi: assert that sectors are aligned to LUN blocksize Paolo Bonzini
@ 2013-07-17 15:05 ` Paolo Bonzini
4 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2013-07-17 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Lieven
From: Peter Lieven <pl@kamp.de>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Peter Lieven <pl@kamp.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/iscsi.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index 1294fdf..5f28c6a 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -232,6 +232,11 @@ iscsi_aio_write16_cb(struct iscsi_context *iscsi, int status,
iscsi_schedule_bh(acb);
}
+static int64_t sector_lun2qemu(int64_t sector, IscsiLun *iscsilun)
+{
+ return sector * iscsilun->block_size / BDRV_SECTOR_SIZE;
+}
+
static int64_t sector_qemu2lun(int64_t sector, IscsiLun *iscsilun)
{
return sector * BDRV_SECTOR_SIZE / iscsilun->block_size;
@@ -296,7 +301,7 @@ iscsi_aio_writev_acb(IscsiAIOCB *acb)
lba = sector_qemu2lun(acb->sector_num, acb->iscsilun);
*(uint32_t *)&acb->task->cdb[2] = htonl(lba >> 32);
*(uint32_t *)&acb->task->cdb[6] = htonl(lba & 0xffffffff);
- num_sectors = size / acb->iscsilun->block_size;
+ num_sectors = sector_qemu2lun(acb->nb_sectors, acb->iscsilun);
*(uint32_t *)&acb->task->cdb[10] = htonl(num_sectors);
acb->task->expxferlen = size;
@@ -1161,8 +1166,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags)
if ((ret = iscsi_readcapacity_sync(iscsilun)) != 0) {
goto out;
}
- bs->total_sectors = iscsilun->num_blocks *
- iscsilun->block_size / BDRV_SECTOR_SIZE ;
+ bs->total_sectors = sector_lun2qemu(iscsilun->num_blocks, iscsilun);
/* Medium changer or tape. We dont have any emulation for this so this must
* be sg ioctl compatible. We force it to be sg, otherwise qemu will try
--
1.8.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread