From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50248) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uqm6D-0003nZ-9r for qemu-devel@nongnu.org; Sun, 23 Jun 2013 11:21:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Uqm69-0000u8-Ri for qemu-devel@nongnu.org; Sun, 23 Jun 2013 11:21:05 -0400 Received: from mx1.redhat.com ([209.132.183.28]:22618) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uqm69-0000tw-Je for qemu-devel@nongnu.org; Sun, 23 Jun 2013 11:21:01 -0400 Message-ID: <51C712E8.2050102@redhat.com> Date: Sun, 23 Jun 2013 17:23:20 +0200 From: Laszlo Ersek MIME-Version: 1.0 References: <1372000028-27775-1-git-send-email-ronniesahlberg@gmail.com> <1372000028-27775-2-git-send-email-ronniesahlberg@gmail.com> In-Reply-To: <1372000028-27775-2-git-send-email-ronniesahlberg@gmail.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] Fix iSCSI crash on SG_IO with an iovector List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ronnie Sahlberg Cc: stefanha@gmail.com, 1191606@bugs.launchpad.net, qemu-devel@nongnu.org, pbonzini@redhat.com 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 > --- > 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