From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41227) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uqh4i-0003p0-VZ for qemu-devel@nongnu.org; Sun, 23 Jun 2013 05:59:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Uqh4g-0000Ol-BE for qemu-devel@nongnu.org; Sun, 23 Jun 2013 05:59:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:2607) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uqh4g-0000Of-3F for qemu-devel@nongnu.org; Sun, 23 Jun 2013 05:59:10 -0400 Message-ID: <51C6C778.5080805@redhat.com> Date: Sun, 23 Jun 2013 12:01:28 +0200 From: Laszlo Ersek MIME-Version: 1.0 References: <1371865038-20821-1-git-send-email-ronniesahlberg@gmail.com> <1371865038-20821-2-git-send-email-ronniesahlberg@gmail.com> In-Reply-To: <1371865038-20821-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/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 > --- > 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 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); >