* [Qemu-devel] [PATCH] iSCSI fix crash when using virtio and libiscsi V3 @ 2013-06-23 15:07 Ronnie Sahlberg 2013-06-23 15:07 ` [Qemu-devel] [PATCH] Fix iSCSI crash on SG_IO with an iovector Ronnie Sahlberg 0 siblings, 1 reply; 10+ messages in thread From: Ronnie Sahlberg @ 2013-06-23 15:07 UTC (permalink / raw) To: qemu-devel; +Cc: stefanha, 1191606, lersek, pbonzini List, Please find a new version of the patch to fix the iSCSI crash when ioctl with iovector is sent. Updated to fix the commit message as per lerseks suggestion. Also added an explicit cast to suppress a compiler warning when we dont have iovector support available. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH] Fix iSCSI crash on SG_IO with an iovector 2013-06-23 15:07 [Qemu-devel] [PATCH] iSCSI fix crash when using virtio and libiscsi V3 Ronnie Sahlberg @ 2013-06-23 15:07 ` Ronnie Sahlberg 2013-06-23 15:23 ` Laszlo Ersek 2013-06-24 14:40 ` Paolo Bonzini 0 siblings, 2 replies; 10+ messages in thread From: Ronnie Sahlberg @ 2013-06-23 15:07 UTC (permalink / raw) To: qemu-devel; +Cc: stefanha, 1191606, lersek, Ronnie Sahlberg, pbonzini 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> --- block/iscsi.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++------- 1 files changed, 49 insertions(+), 7 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index 0bbf0b1..dca38c4 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -651,6 +651,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 +730,36 @@ 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 + int i; + char *buf; + struct scsi_iovec *iov = (struct scsi_iovec *)acb->ioh->dxferp; + + acb->buf = g_malloc(acb->ioh->dxfer_len); + buf = (char *)acb->buf; + for (i = 0; i < acb->ioh->iovec_count; i++) { + memcpy(buf, iov[i].iov_base, iov[i].iov_len); + buf += iov[i].iov_len; + } + data.data = acb->buf; + data.size = 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 +768,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 scsi_iovec *iov = (struct scsi_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.7.3.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Fix iSCSI crash on SG_IO with an iovector 2013-06-23 15:07 ` [Qemu-devel] [PATCH] Fix iSCSI crash on SG_IO with an iovector Ronnie Sahlberg @ 2013-06-23 15:23 ` Laszlo Ersek 2013-06-24 14:40 ` Paolo Bonzini 1 sibling, 0 replies; 10+ messages in thread From: Laszlo Ersek @ 2013-06-23 15:23 UTC (permalink / raw) To: Ronnie Sahlberg; +Cc: stefanha, 1191606, qemu-devel, pbonzini On 06/23/13 17:07, Ronnie Sahlberg wrote: > 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> > --- > block/iscsi.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++------- > 1 files changed, 49 insertions(+), 7 deletions(-) > > diff --git a/block/iscsi.c b/block/iscsi.c > index 0bbf0b1..dca38c4 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c > @@ -651,6 +651,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 +730,36 @@ 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 > + int i; > + char *buf; > + struct scsi_iovec *iov = (struct scsi_iovec *)acb->ioh->dxferp; > + > + acb->buf = g_malloc(acb->ioh->dxfer_len); > + buf = (char *)acb->buf; > + for (i = 0; i < acb->ioh->iovec_count; i++) { > + memcpy(buf, iov[i].iov_base, iov[i].iov_len); > + buf += iov[i].iov_len; > + } > + data.data = acb->buf; > + data.size = 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 +768,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 scsi_iovec *iov = (struct scsi_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); > Reviewed-by: Laszlo Ersek <lersek@redhat.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Fix iSCSI crash on SG_IO with an iovector 2013-06-23 15:07 ` [Qemu-devel] [PATCH] Fix iSCSI crash on SG_IO with an iovector Ronnie Sahlberg 2013-06-23 15:23 ` Laszlo Ersek @ 2013-06-24 14:40 ` Paolo Bonzini 2013-06-25 9:24 ` Stefan Hajnoczi 1 sibling, 1 reply; 10+ messages in thread From: Paolo Bonzini @ 2013-06-24 14:40 UTC (permalink / raw) To: Ronnie Sahlberg; +Cc: stefanha, 1191606, lersek, qemu-devel, 1.5.x Il 23/06/2013 17:07, Ronnie Sahlberg ha scritto: > 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> > --- > block/iscsi.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++------- > 1 files changed, 49 insertions(+), 7 deletions(-) Cc: qemu-stable@nongnu.org Will apply to scsi-next in the next few days. Paolo > diff --git a/block/iscsi.c b/block/iscsi.c > index 0bbf0b1..dca38c4 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c > @@ -651,6 +651,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 +730,36 @@ 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 > + int i; > + char *buf; > + struct scsi_iovec *iov = (struct scsi_iovec *)acb->ioh->dxferp; > + > + acb->buf = g_malloc(acb->ioh->dxfer_len); > + buf = (char *)acb->buf; > + for (i = 0; i < acb->ioh->iovec_count; i++) { > + memcpy(buf, iov[i].iov_base, iov[i].iov_len); > + buf += iov[i].iov_len; > + } > + data.data = acb->buf; > + data.size = 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 +768,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 scsi_iovec *iov = (struct scsi_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); > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Fix iSCSI crash on SG_IO with an iovector 2013-06-24 14:40 ` Paolo Bonzini @ 2013-06-25 9:24 ` Stefan Hajnoczi 2013-06-25 10:42 ` Paolo Bonzini 0 siblings, 1 reply; 10+ messages in thread From: Stefan Hajnoczi @ 2013-06-25 9:24 UTC (permalink / raw) To: Paolo Bonzini; +Cc: 1191606, lersek, qemu-devel, Ronnie Sahlberg, 1.5.x On Mon, Jun 24, 2013 at 04:40:11PM +0200, Paolo Bonzini wrote: > Il 23/06/2013 17:07, Ronnie Sahlberg ha scritto: > > 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> > > --- > > block/iscsi.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++------- > > 1 files changed, 49 insertions(+), 7 deletions(-) > > Cc: qemu-stable@nongnu.org > > Will apply to scsi-next in the next few days. Paolo, there are small whitespace issues that you might like to fix when merging: > > + } else { > > +#if defined(LIBISCSI_FEATURE_IOVECTOR) > > + scsi_task_set_iov_out(acb->task, > > + (struct scsi_iovec *) acb->ioh->dxferp, > > + acb->ioh->iovec_count); Should be 4-space indentation. > > + #else Space before #else? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Fix iSCSI crash on SG_IO with an iovector 2013-06-25 9:24 ` Stefan Hajnoczi @ 2013-06-25 10:42 ` Paolo Bonzini 0 siblings, 0 replies; 10+ messages in thread From: Paolo Bonzini @ 2013-06-25 10:42 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: 1191606, lersek, qemu-devel, Ronnie Sahlberg, 1.5.x Il 25/06/2013 11:24, Stefan Hajnoczi ha scritto: > On Mon, Jun 24, 2013 at 04:40:11PM +0200, Paolo Bonzini wrote: >> Il 23/06/2013 17:07, Ronnie Sahlberg ha scritto: >>> 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> >>> --- >>> block/iscsi.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++------- >>> 1 files changed, 49 insertions(+), 7 deletions(-) >> >> Cc: qemu-stable@nongnu.org >> >> Will apply to scsi-next in the next few days. > > Paolo, there are small whitespace issues that you might like to fix when > merging: Yup, will do. Paolo >>> + } else { >>> +#if defined(LIBISCSI_FEATURE_IOVECTOR) >>> + scsi_task_set_iov_out(acb->task, >>> + (struct scsi_iovec *) acb->ioh->dxferp, >>> + acb->ioh->iovec_count); > > Should be 4-space indentation. > >>> + #else > > Space before #else? > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH] iSCSI fix crash when using virtio and libiscsi V2 @ 2013-06-22 1:37 Ronnie Sahlberg 2013-06-22 1:37 ` [Qemu-devel] [PATCH] Fix iSCSI crash on SG_IO with an iovector Ronnie Sahlberg 0 siblings, 1 reply; 10+ messages in thread From: Ronnie Sahlberg @ 2013-06-22 1:37 UTC (permalink / raw) To: qemu-devel; +Cc: stefanha, 1191606, lersek, pbonzini List, Version 2 of patch to fix the crashbug for virtio and libiscsi Make block/iscsi.c aware that we might be called not with ioh containing a buffer for the data but that it might contain an array of io vectors. Updated to check LIBISCSI_FEATURE_IOVECTOR wether we can use iovectors or not. If not we fall back to either lots of datain buffers or a serialization dataout buffer. regards ronnie sahlberg ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH] Fix iSCSI crash on SG_IO with an iovector 2013-06-22 1:37 [Qemu-devel] [PATCH] iSCSI fix crash when using virtio and libiscsi V2 Ronnie Sahlberg @ 2013-06-22 1:37 ` Ronnie Sahlberg 2013-06-23 10:01 ` Laszlo Ersek 2013-06-25 9:21 ` Stefan Hajnoczi 0 siblings, 2 replies; 10+ messages in thread From: Ronnie Sahlberg @ 2013-06-22 1:37 UTC (permalink / raw) To: qemu-devel; +Cc: stefanha, 1191606, lersek, Ronnie Sahlberg, pbonzini 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> --- block/iscsi.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++------- 1 files changed, 49 insertions(+), 7 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index 0bbf0b1..cbe2e8f 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -651,6 +651,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 +730,36 @@ 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 + int i; + char *buf; + struct scsi_iovec *iov = (struct scsi_iovec *)acb->ioh->dxferp; + + acb->buf = g_malloc(acb->ioh->dxfer_len); + buf = acb->buf; + for (i = 0; i < acb->ioh->iovec_count; i++) { + memcpy(buf, iov[i].iov_base, iov[i].iov_len); + buf += iov[i].iov_len; + } + data.data = acb->buf; + data.size = 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 +768,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 scsi_iovec *iov = (struct scsi_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.7.3.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Fix iSCSI crash on SG_IO with an iovector 2013-06-22 1:37 ` [Qemu-devel] [PATCH] Fix iSCSI crash on SG_IO with an iovector Ronnie Sahlberg @ 2013-06-23 10:01 ` Laszlo Ersek 2013-06-25 9:21 ` Stefan Hajnoczi 1 sibling, 0 replies; 10+ messages in thread From: Laszlo Ersek @ 2013-06-23 10:01 UTC (permalink / raw) To: Ronnie Sahlberg; +Cc: stefanha, 1191606, qemu-devel, pbonzini On 06/22/13 03:37, Ronnie Sahlberg wrote: > 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> > --- > block/iscsi.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++------- > 1 files changed, 49 insertions(+), 7 deletions(-) Looks okay to me, but of course I'm not too familiar with this code. You (or the maintainer with jurisdiction) might want to change the commit message: - check the iovec_count and if it is > 1 then we need to pass an array + check the iovec_count and if it is >= 1 then we need to pass an array But I won't insist on a repost naturally! I assume you tested the code for both definednesses of LIBISCSI_FEATURE_IOVECTOR. Reviewed-by: Laszlo Ersek <lersek@redhat.com> Thanks Laszlo > > diff --git a/block/iscsi.c b/block/iscsi.c > index 0bbf0b1..cbe2e8f 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c > @@ -651,6 +651,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 +730,36 @@ 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 > + int i; > + char *buf; > + struct scsi_iovec *iov = (struct scsi_iovec *)acb->ioh->dxferp; > + > + acb->buf = g_malloc(acb->ioh->dxfer_len); > + buf = acb->buf; > + for (i = 0; i < acb->ioh->iovec_count; i++) { > + memcpy(buf, iov[i].iov_base, iov[i].iov_len); > + buf += iov[i].iov_len; > + } > + data.data = acb->buf; > + data.size = 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 +768,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 scsi_iovec *iov = (struct scsi_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); > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Fix iSCSI crash on SG_IO with an iovector 2013-06-22 1:37 ` [Qemu-devel] [PATCH] Fix iSCSI crash on SG_IO with an iovector Ronnie Sahlberg 2013-06-23 10:01 ` Laszlo Ersek @ 2013-06-25 9:21 ` Stefan Hajnoczi 1 sibling, 0 replies; 10+ messages in thread From: Stefan Hajnoczi @ 2013-06-25 9:21 UTC (permalink / raw) To: Ronnie Sahlberg; +Cc: pbonzini, 1191606, lersek, qemu-devel On Fri, Jun 21, 2013 at 06:37:18PM -0700, Ronnie Sahlberg wrote: Looks fine except these whitespace issues: > @@ -727,14 +730,36 @@ 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, Indentation should be 4 spaces. > + (struct scsi_iovec *) acb->ioh->dxferp, > + acb->ioh->iovec_count); > + #else Space before #else? ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH] iSCSI fix crash when using virtio and libiscsi @ 2013-06-21 2:32 Ronnie Sahlberg 2013-06-21 2:32 ` [Qemu-devel] [PATCH] Fix iSCSI crash on SG_IO with an iovector Ronnie Sahlberg 0 siblings, 1 reply; 10+ messages in thread From: Ronnie Sahlberg @ 2013-06-21 2:32 UTC (permalink / raw) To: qemu-devel; +Cc: stefanha, 1191606, lersek Stefan, List Please find a patch that fixes the crashes for using virtio with libiscsi. The problem was that block/iscsi.c always assumed we got a plain buffer to read data into, and when we got an iovector array instead we would overwrite pointers with garbage and crash. Since we can get iovectors for the write case as well I have added a fix for when the guest is writing data to the target to handle the iovector case as well. The new calls added are not protected with (LIBISCSI_FEATURE_IOVECTOR) checks since anyone building a new/current version of qemu should probably also build against a current libiscsi. I will send patches later to remove the current (LIBISCSI_FEATURE_IOVECTOR) checks in the rest of the file. regards ronnie sahlberg ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH] Fix iSCSI crash on SG_IO with an iovector 2013-06-21 2:32 [Qemu-devel] [PATCH] iSCSI fix crash when using virtio and libiscsi Ronnie Sahlberg @ 2013-06-21 2:32 ` Ronnie Sahlberg 0 siblings, 0 replies; 10+ messages in thread From: Ronnie Sahlberg @ 2013-06-21 2:32 UTC (permalink / raw) To: qemu-devel; +Cc: stefanha, 1191606, lersek, Ronnie Sahlberg 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> --- block/iscsi.c | 31 ++++++++++++++++++++++++------- 1 files changed, 24 insertions(+), 7 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index 0bbf0b1..2d1cb4e 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -727,25 +727,42 @@ 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; + } } 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); return NULL; } + /* We got an iovector for writing to the target */ + if (acb->task->xfer_dir == SCSI_XFER_WRITE) { + if (acb->ioh->iovec_count > 0) { + scsi_task_set_iov_out(acb->task, + (struct scsi_iovec *) acb->ioh->dxferp, + acb->ioh->iovec_count); + } + } + /* 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 { + scsi_task_set_iov_in(acb->task, + (struct scsi_iovec *) acb->ioh->dxferp, + acb->ioh->iovec_count); + } } iscsi_set_events(iscsilun); -- 1.7.3.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-06-25 10:42 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-06-23 15:07 [Qemu-devel] [PATCH] iSCSI fix crash when using virtio and libiscsi V3 Ronnie Sahlberg 2013-06-23 15:07 ` [Qemu-devel] [PATCH] Fix iSCSI crash on SG_IO with an iovector Ronnie Sahlberg 2013-06-23 15:23 ` Laszlo Ersek 2013-06-24 14:40 ` Paolo Bonzini 2013-06-25 9:24 ` Stefan Hajnoczi 2013-06-25 10:42 ` Paolo Bonzini -- strict thread matches above, loose matches on Subject: below -- 2013-06-22 1:37 [Qemu-devel] [PATCH] iSCSI fix crash when using virtio and libiscsi V2 Ronnie Sahlberg 2013-06-22 1:37 ` [Qemu-devel] [PATCH] Fix iSCSI crash on SG_IO with an iovector Ronnie Sahlberg 2013-06-23 10:01 ` Laszlo Ersek 2013-06-25 9:21 ` Stefan Hajnoczi 2013-06-21 2:32 [Qemu-devel] [PATCH] iSCSI fix crash when using virtio and libiscsi Ronnie Sahlberg 2013-06-21 2:32 ` [Qemu-devel] [PATCH] Fix iSCSI crash on SG_IO with an iovector Ronnie Sahlberg
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).