Message ID | 1371865038-20821-2-git-send-email-ronniesahlberg@gmail.com |
---|---|
State | New |
Headers | show |
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); >
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?
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);
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(-)