* [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; 6+ 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] 6+ 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; 6+ 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] 6+ 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; 6+ 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] 6+ 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; 6+ 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] 6+ 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; 6+ 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] 6+ 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; 6+ 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] 6+ messages in thread
end of thread, other threads:[~2013-06-25 10:42 UTC | newest] Thread overview: 6+ 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
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).