* [Qemu-devel] [PATCH 0/2] Add support for iscsi passthrough and better eventsystem support @ 2012-05-25 7:43 Ronnie Sahlberg 2012-05-25 7:43 ` [Qemu-devel] [PATCH 1/2] ISCSI: redo how we set up the events to only call qemu_aio_set_fd_handler() and qemu_notify_event() if something has changed Ronnie Sahlberg 2012-05-25 7:43 ` [Qemu-devel] [PATCH 2/2] SCSI: Add SCSI passthrough via scsi-generic to libiscsi Ronnie Sahlberg 0 siblings, 2 replies; 6+ messages in thread From: Ronnie Sahlberg @ 2012-05-25 7:43 UTC (permalink / raw) To: kwolf, pbonzini, qemu-devel Paolo, list Please find two patches. Both are big so maybe best to only add to the post 1.1 branches. The first patch tries to be more intelligent when setting up the fd events and only call these functions if something has changed. The second patch adds support for passthrough of SCSI comamnds. Just like /dev/sg allows the guest to talk to a real scsi device directly without qemu emulating the commands, this patch adds the same capability when using iscsi devices. With this patch the guest can talk directly to the target iscsi devices. regards ronnie sahlberg ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 1/2] ISCSI: redo how we set up the events to only call qemu_aio_set_fd_handler() and qemu_notify_event() if something has changed. 2012-05-25 7:43 [Qemu-devel] [PATCH 0/2] Add support for iscsi passthrough and better eventsystem support Ronnie Sahlberg @ 2012-05-25 7:43 ` Ronnie Sahlberg 2012-05-25 9:59 ` Paolo Bonzini 2012-05-25 7:43 ` [Qemu-devel] [PATCH 2/2] SCSI: Add SCSI passthrough via scsi-generic to libiscsi Ronnie Sahlberg 1 sibling, 1 reply; 6+ messages in thread From: Ronnie Sahlberg @ 2012-05-25 7:43 UTC (permalink / raw) To: kwolf, pbonzini, qemu-devel; +Cc: Ronnie Sahlberg Also first call out to the socket write functions director, and only set up the write event if the socket is full. This means that we will only need to invoke these two functions very rarely which will improve performance. Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com> --- block/iscsi.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++---------- 1 files changed, 46 insertions(+), 10 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index ed1ad7b..9642ee6 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -40,6 +40,8 @@ typedef struct IscsiLun { int lun; int block_size; unsigned long num_blocks; + IOHandler *read_handler; + IOHandler *write_handler; } IscsiLun; typedef struct IscsiAIOCB { @@ -105,18 +107,50 @@ static void iscsi_set_events(IscsiLun *iscsilun) { struct iscsi_context *iscsi = iscsilun->iscsi; + int need_set_fd = 0; + int need_notify_event = 0; - qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), iscsi_process_read, - (iscsi_which_events(iscsi) & POLLOUT) - ? iscsi_process_write : NULL, - iscsi_process_flush, iscsilun); - - /* If we just added the event for writeable we must call - and the socket is already writeable the callback might - not be invoked until after a short delay unless we call - qemu_notify_event(). + /* Try to write as much as we can to the socket + * without setting up an event */ - qemu_notify_event(); + if (iscsi_which_events(iscsi) & POLLOUT) { + iscsi_process_write(iscsilun); + } + + if (iscsilun->read_handler == NULL) { + iscsilun->read_handler = iscsi_process_read; + need_set_fd = 1; + need_notify_event = 1; + } + + if (iscsi_which_events(iscsi) & POLLOUT) { + if (iscsilun->write_handler == NULL) { + iscsilun->write_handler = iscsi_process_write; + need_set_fd = 1; + need_notify_event = 1; + } + } else { + if (iscsilun->write_handler != NULL) { + iscsilun->write_handler = NULL; + need_set_fd = 1; + } + } + + if (need_set_fd) { + qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), + iscsilun->read_handler, + iscsilun->write_handler, + iscsi_process_flush, + iscsilun); + + /* If we just added an event that may trigger almost immediately + the callback might not be invoked until after a short + delay unless we call qemu_notify_event(). + */ + if (need_notify_event) { + qemu_notify_event(); + } + } } static void @@ -790,6 +824,8 @@ static void iscsi_close(BlockDriverState *bs) IscsiLun *iscsilun = bs->opaque; struct iscsi_context *iscsi = iscsilun->iscsi; + iscsilun->read_handler = NULL; + iscsilun->write_handler = NULL; qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), NULL, NULL, NULL, NULL); iscsi_destroy_context(iscsi); memset(iscsilun, 0, sizeof(IscsiLun)); -- 1.7.3.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] ISCSI: redo how we set up the events to only call qemu_aio_set_fd_handler() and qemu_notify_event() if something has changed. 2012-05-25 7:43 ` [Qemu-devel] [PATCH 1/2] ISCSI: redo how we set up the events to only call qemu_aio_set_fd_handler() and qemu_notify_event() if something has changed Ronnie Sahlberg @ 2012-05-25 9:59 ` Paolo Bonzini 2012-05-25 10:06 ` ronnie sahlberg 0 siblings, 1 reply; 6+ messages in thread From: Paolo Bonzini @ 2012-05-25 9:59 UTC (permalink / raw) To: Ronnie Sahlberg; +Cc: kwolf, qemu-devel Il 25/05/2012 09:43, Ronnie Sahlberg ha scritto: > Also first call out to the socket write functions director, and only set up the write event if the socket is full. > > This means that we will only need to invoke these two functions very rarely which will improve performance. > > Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com> > --- > block/iscsi.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++---------- > 1 files changed, 46 insertions(+), 10 deletions(-) > > diff --git a/block/iscsi.c b/block/iscsi.c > index ed1ad7b..9642ee6 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c > @@ -40,6 +40,8 @@ typedef struct IscsiLun { > int lun; > int block_size; > unsigned long num_blocks; > + IOHandler *read_handler; > + IOHandler *write_handler; > } IscsiLun; > > typedef struct IscsiAIOCB { > @@ -105,18 +107,50 @@ static void > iscsi_set_events(IscsiLun *iscsilun) > { > struct iscsi_context *iscsi = iscsilun->iscsi; > + int need_set_fd = 0; > + int need_notify_event = 0; > > - qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), iscsi_process_read, > - (iscsi_which_events(iscsi) & POLLOUT) > - ? iscsi_process_write : NULL, > - iscsi_process_flush, iscsilun); > - > - /* If we just added the event for writeable we must call > - and the socket is already writeable the callback might > - not be invoked until after a short delay unless we call > - qemu_notify_event(). > + /* Try to write as much as we can to the socket > + * without setting up an event > */ > - qemu_notify_event(); > + if (iscsi_which_events(iscsi) & POLLOUT) { > + iscsi_process_write(iscsilun); > + } > + > + if (iscsilun->read_handler == NULL) { > + iscsilun->read_handler = iscsi_process_read; > + need_set_fd = 1; > + need_notify_event = 1; > + } This is not needed, just pass iscsi_process_read unconditionally to qemu_aio_set_fd_handler. > + if (iscsi_which_events(iscsi) & POLLOUT) { > + if (iscsilun->write_handler == NULL) { > + iscsilun->write_handler = iscsi_process_write; > + need_set_fd = 1; > + need_notify_event = 1; > + } > + } else { > + if (iscsilun->write_handler != NULL) { > + iscsilun->write_handler = NULL; > + need_set_fd = 1; > + } > + } > + > + if (need_set_fd) { > + qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), > + iscsilun->read_handler, > + iscsilun->write_handler, > + iscsi_process_flush, > + iscsilun); > + > + /* If we just added an event that may trigger almost immediately > + the callback might not be invoked until after a short > + delay unless we call qemu_notify_event(). > + */ > + if (need_notify_event) { > + qemu_notify_event(); > + } > + } What about this: /* We always register a read handler. */ ev = POLLIN; ev |= iscsi_which_events(iscsi); if (ev != iscsilun->events) { qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), iscsi_process_read, (ev & POLLOUT) ? iscsi_process_write : NULL, iscsi_process_flush, iscsilun); } /* If we just added an event, the callback might be delayed * unless we call qemu_notify_event(). */ if (ev & ~iscsilun->events) { qemu_notify_event(); } iscsilun->events = ev; ? Paolo ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] ISCSI: redo how we set up the events to only call qemu_aio_set_fd_handler() and qemu_notify_event() if something has changed. 2012-05-25 9:59 ` Paolo Bonzini @ 2012-05-25 10:06 ` ronnie sahlberg 0 siblings, 0 replies; 6+ messages in thread From: ronnie sahlberg @ 2012-05-25 10:06 UTC (permalink / raw) To: Paolo Bonzini; +Cc: kwolf, qemu-devel On Fri, May 25, 2012 at 7:59 PM, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 25/05/2012 09:43, Ronnie Sahlberg ha scritto: >> Also first call out to the socket write functions director, and only set up the write event if the socket is full. >> >> This means that we will only need to invoke these two functions very rarely which will improve performance. >> >> Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com> >> --- >> block/iscsi.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++---------- >> 1 files changed, 46 insertions(+), 10 deletions(-) >> >> diff --git a/block/iscsi.c b/block/iscsi.c >> index ed1ad7b..9642ee6 100644 >> --- a/block/iscsi.c >> +++ b/block/iscsi.c >> @@ -40,6 +40,8 @@ typedef struct IscsiLun { >> int lun; >> int block_size; >> unsigned long num_blocks; >> + IOHandler *read_handler; >> + IOHandler *write_handler; >> } IscsiLun; >> >> typedef struct IscsiAIOCB { >> @@ -105,18 +107,50 @@ static void >> iscsi_set_events(IscsiLun *iscsilun) >> { >> struct iscsi_context *iscsi = iscsilun->iscsi; >> + int need_set_fd = 0; >> + int need_notify_event = 0; >> >> - qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), iscsi_process_read, >> - (iscsi_which_events(iscsi) & POLLOUT) >> - ? iscsi_process_write : NULL, >> - iscsi_process_flush, iscsilun); >> - >> - /* If we just added the event for writeable we must call >> - and the socket is already writeable the callback might >> - not be invoked until after a short delay unless we call >> - qemu_notify_event(). >> + /* Try to write as much as we can to the socket >> + * without setting up an event >> */ >> - qemu_notify_event(); >> + if (iscsi_which_events(iscsi) & POLLOUT) { >> + iscsi_process_write(iscsilun); >> + } >> + >> + if (iscsilun->read_handler == NULL) { >> + iscsilun->read_handler = iscsi_process_read; >> + need_set_fd = 1; >> + need_notify_event = 1; >> + } > > This is not needed, just pass iscsi_process_read unconditionally to > qemu_aio_set_fd_handler. > >> + if (iscsi_which_events(iscsi) & POLLOUT) { >> + if (iscsilun->write_handler == NULL) { >> + iscsilun->write_handler = iscsi_process_write; >> + need_set_fd = 1; >> + need_notify_event = 1; >> + } >> + } else { >> + if (iscsilun->write_handler != NULL) { >> + iscsilun->write_handler = NULL; >> + need_set_fd = 1; >> + } >> + } >> + >> + if (need_set_fd) { >> + qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), >> + iscsilun->read_handler, >> + iscsilun->write_handler, >> + iscsi_process_flush, >> + iscsilun); >> + >> + /* If we just added an event that may trigger almost immediately >> + the callback might not be invoked until after a short >> + delay unless we call qemu_notify_event(). >> + */ >> + if (need_notify_event) { >> + qemu_notify_event(); >> + } >> + } > > What about this: > > /* We always register a read handler. */ > ev = POLLIN; > ev |= iscsi_which_events(iscsi); > if (ev != iscsilun->events) { > qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), > iscsi_process_read, > (ev & POLLOUT) ? iscsi_process_write : NULL, > iscsi_process_flush, > iscsilun); > > } > > /* If we just added an event, the callback might be delayed > * unless we call qemu_notify_event(). > */ > if (ev & ~iscsilun->events) { > qemu_notify_event(); > } > iscsilun->events = ev; > > ? > > Paolo Yes. Even better. Will you check that in or should I send a new patch with your suggestion? ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 2/2] SCSI: Add SCSI passthrough via scsi-generic to libiscsi 2012-05-25 7:43 [Qemu-devel] [PATCH 0/2] Add support for iscsi passthrough and better eventsystem support Ronnie Sahlberg 2012-05-25 7:43 ` [Qemu-devel] [PATCH 1/2] ISCSI: redo how we set up the events to only call qemu_aio_set_fd_handler() and qemu_notify_event() if something has changed Ronnie Sahlberg @ 2012-05-25 7:43 ` Ronnie Sahlberg 2012-05-25 10:20 ` Paolo Bonzini 1 sibling, 1 reply; 6+ messages in thread From: Ronnie Sahlberg @ 2012-05-25 7:43 UTC (permalink / raw) To: kwolf, pbonzini, qemu-devel; +Cc: Ronnie Sahlberg Update scsi-generic to allow passthrough of SG_IO scsi commands to iscsi devices too in addition to the real scsi-generic devices. Implement both bdrv_ioctl() and bdrv_aio_ioctl() in the iscsi backend, emulate the SG_IO ioctl and pass the SCSI commands across to the iscsi target. This allows end-to-end passthrough of SCSI all the way from the guest, to qemu, via scsi-generic, then libiscsi all the way to the iscsi target. To activate this you need to specify interface type as scsi when creating the device. Example: -drive file=iscsi://10.1.1.125/iqn.ronnie.test/1,if=scsi,bus=0,unit=5 Note, you can currently not boot a qemu guest from a 'if=scsi' device. Note, This only works when the host is linux, since the emulation relies on definitions of SG_IO from the scsi-generic implementation in the linux kernel. It should be fairly easy to re-implement some structures similar enough for non-linux hosts to do the same style of passthrough via a fake scsi generic layer and libiscsi if need be. Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com> --- block.c | 18 ++++-- block.h | 1 + block/iscsi.c | 191 ++++++++++++++++++++++++++++++++++++++++++++++++++++- block/raw.c | 1 + block_int.h | 1 + hw/scsi-bus.c | 7 ++- hw/scsi-disk.c | 4 + hw/scsi-generic.c | 4 +- 8 files changed, 217 insertions(+), 10 deletions(-) diff --git a/block.c b/block.c index af2ab4f..2467b8e 100644 --- a/block.c +++ b/block.c @@ -507,10 +507,11 @@ static int find_image_format(const char *filename, BlockDriver **pdrv) return ret; } - /* Return the raw BlockDriver * to scsi-generic devices or empty drives */ - if (bs->sg || !bdrv_is_inserted(bs)) { + /* Return the raw BlockDriver * to scsi-generic devices, + iscsi devices or empty drives */ + if (bs->sg || bs->iscsi || !bdrv_is_inserted(bs)) { bdrv_delete(bs); - drv = bdrv_find_format("raw"); + drv = bdrv_find_format(bs->iscsi ? "iscsi" : "raw"); if (!drv) { ret = -ENOENT; } @@ -550,9 +551,10 @@ static int refresh_total_sectors(BlockDriverState *bs, int64_t hint) { BlockDriver *drv = bs->drv; - /* Do not attempt drv->bdrv_getlength() on scsi-generic devices */ - if (bs->sg) + /* Do not attempt drv->bdrv_getlength() on scsi-generic/iscsi devices */ + if (bs->sg || bs->iscsi) { return 0; + } /* query actual device if possible, otherwise just trust the hint */ if (drv->bdrv_getlength) { @@ -879,6 +881,7 @@ void bdrv_close(BlockDriverState *bs) bs->encrypted = 0; bs->valid_key = 0; bs->sg = 0; + bs->iscsi = 0; bs->growable = 0; if (bs->file != NULL) { @@ -2355,6 +2358,11 @@ int bdrv_is_sg(BlockDriverState *bs) return bs->sg; } +int bdrv_is_iscsi(BlockDriverState *bs) +{ + return bs->iscsi; +} + int bdrv_enable_write_cache(BlockDriverState *bs) { return bs->enable_write_cache; diff --git a/block.h b/block.h index 7408acc..642194b 100644 --- a/block.h +++ b/block.h @@ -279,6 +279,7 @@ void bdrv_set_on_error(BlockDriverState *bs, BlockErrorAction on_read_error, BlockErrorAction bdrv_get_on_error(BlockDriverState *bs, int is_read); int bdrv_is_read_only(BlockDriverState *bs); int bdrv_is_sg(BlockDriverState *bs); +int bdrv_is_iscsi(BlockDriverState *bs); int bdrv_enable_write_cache(BlockDriverState *bs); int bdrv_is_inserted(BlockDriverState *bs); int bdrv_media_changed(BlockDriverState *bs); diff --git a/block/iscsi.c b/block/iscsi.c index 9642ee6..301ab69 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -34,10 +34,15 @@ #include <iscsi/iscsi.h> #include <iscsi/scsi-lowlevel.h> +#ifdef __linux__ +#include <scsi/sg.h> +#include <hw/scsi-defs.h> +#endif typedef struct IscsiLun { struct iscsi_context *iscsi; int lun; + enum scsi_inquiry_peripheral_device_type type; int block_size; unsigned long num_blocks; IOHandler *read_handler; @@ -55,6 +60,9 @@ typedef struct IscsiAIOCB { int canceled; size_t read_size; size_t read_offset; +#ifdef __linux__ + sg_io_hdr_t *ioh; +#endif } IscsiAIOCB; struct IscsiTask { @@ -526,6 +534,140 @@ iscsi_aio_discard(BlockDriverState *bs, return &acb->common; } +#ifdef __linux__ +static void +iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status, + void *command_data, void *opaque) +{ + IscsiAIOCB *acb = opaque; + + if (acb->canceled != 0) { + qemu_aio_release(acb); + scsi_free_scsi_task(acb->task); + acb->task = NULL; + return; + } + + acb->status = 0; + if (status < 0) { + error_report("Failed to ioctl(SG_IO) to iSCSI lun. %s", + iscsi_get_error(iscsi)); + acb->status = -EIO; + } + + acb->ioh->driver_status = 0; + acb->ioh->host_status = 0; + acb->ioh->resid = 0; + +#define SG_ERR_DRIVER_SENSE 0x08 + + if (status == SCSI_STATUS_CHECK_CONDITION && acb->task->datain.size >= 2) { + int ss; + + acb->ioh->driver_status |= SG_ERR_DRIVER_SENSE; + + acb->ioh->sb_len_wr = acb->task->datain.size - 2; + ss = (acb->ioh->mx_sb_len >= acb->ioh->sb_len_wr) ? + acb->ioh->mx_sb_len : acb->ioh->sb_len_wr; + memcpy(acb->ioh->sbp, &acb->task->datain.data[2], ss); + } + + iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb); + scsi_free_scsi_task(acb->task); + acb->task = NULL; +} + +static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs, + unsigned long int req, void *buf, + BlockDriverCompletionFunc *cb, void *opaque) +{ + IscsiLun *iscsilun = bs->opaque; + struct iscsi_context *iscsi = iscsilun->iscsi; + struct iscsi_data data; + IscsiAIOCB *acb; + + if (req != SG_IO) { + error_report("iSCSI: iscsi_aio_ioctl called for non-SG_IO " + "ioctl"); + return NULL; + } + + acb = qemu_aio_get(&iscsi_aio_pool, bs, cb, opaque); + + acb->iscsilun = iscsilun; + acb->canceled = 0; + acb->buf = NULL; + acb->ioh = buf; + + acb->task = malloc(sizeof(struct scsi_task)); + if (acb->task == NULL) { + error_report("iSCSI: Failed to allocate task for scsi command. %s", + iscsi_get_error(iscsi)); + qemu_aio_release(acb); + return NULL; + } + memset(acb->task, 0, sizeof(struct scsi_task)); + + switch (acb->ioh->dxfer_direction) { + case SG_DXFER_TO_DEV: + acb->task->xfer_dir = SCSI_XFER_WRITE; + break; + case SG_DXFER_FROM_DEV: + acb->task->xfer_dir = SCSI_XFER_READ; + break; + default: + acb->task->xfer_dir = SCSI_XFER_NONE; + break; + } + + acb->task->cdb_size = acb->ioh->cmd_len; + memcpy(&acb->task->cdb[0], acb->ioh->cmdp, acb->ioh->cmd_len); + acb->task->expxferlen = acb->ioh->dxfer_len; + + if (acb->task->xfer_dir == SCSI_XFER_WRITE) { + 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, + acb) != 0) { + scsi_free_scsi_task(acb->task); + qemu_aio_release(acb); + return NULL; + } + + /* 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); + } + + iscsi_set_events(iscsilun); + + return &acb->common; +} + +static int iscsi_ioctl(BlockDriverState *bs, unsigned long int req, void *buf) +{ + IscsiLun *iscsilun = bs->opaque; + + switch (req) { + case SG_GET_VERSION_NUM: + *(int *)buf = 30000; + break; + case SG_GET_SCSI_ID: + ((struct sg_scsi_id *)buf)->scsi_type = iscsilun->type; + break; + default: + return -1; + } + return 0; +} +#endif + static int64_t iscsi_getlength(BlockDriverState *bs) { @@ -575,18 +717,33 @@ iscsi_readcapacity16_cb(struct iscsi_context *iscsi, int status, } static void -iscsi_connect_cb(struct iscsi_context *iscsi, int status, void *command_data, +iscsi_inquiry_cb(struct iscsi_context *iscsi, int status, void *command_data, void *opaque) { struct IscsiTask *itask = opaque; - struct scsi_task *task; + struct scsi_task *task = command_data; + struct scsi_inquiry_standard *inq; if (status != 0) { itask->status = 1; itask->complete = 1; + scsi_free_scsi_task(task); + return; + } + + inq = scsi_datain_unmarshall(task); + if (inq == NULL) { + error_report("iSCSI: Failed to unmarshall inquiry data."); + itask->status = 1; + itask->complete = 1; + scsi_free_scsi_task(task); return; } + itask->iscsilun->type = inq->periperal_device_type; + + scsi_free_scsi_task(task); + task = iscsi_readcapacity16_task(iscsi, itask->iscsilun->lun, iscsi_readcapacity16_cb, opaque); if (task == NULL) { @@ -597,6 +754,30 @@ iscsi_connect_cb(struct iscsi_context *iscsi, int status, void *command_data, } } +static void +iscsi_connect_cb(struct iscsi_context *iscsi, int status, void *command_data, + void *opaque) +{ + struct IscsiTask *itask = opaque; + struct scsi_task *task; + + if (status != 0) { + itask->status = 1; + itask->complete = 1; + return; + } + + task = iscsi_inquiry_task(iscsi, itask->iscsilun->lun, + 0, 0, 36, + iscsi_inquiry_cb, opaque); + if (task == NULL) { + error_report("iSCSI: failed to send inquiry command."); + itask->status = 1; + itask->complete = 1; + return; + } +} + static int parse_chap(struct iscsi_context *iscsi, const char *target) { QemuOptsList *list; @@ -803,6 +984,7 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags) if (iscsi_url != NULL) { iscsi_destroy_url(iscsi_url); } + bs->iscsi = 1; return 0; failed: @@ -846,6 +1028,11 @@ static BlockDriver bdrv_iscsi = { .bdrv_aio_flush = iscsi_aio_flush, .bdrv_aio_discard = iscsi_aio_discard, + +#ifdef __linux__ + .bdrv_ioctl = iscsi_ioctl, + .bdrv_aio_ioctl = iscsi_aio_ioctl, +#endif }; static void iscsi_block_init(void) diff --git a/block/raw.c b/block/raw.c index 7086e31..62fc8c6 100644 --- a/block/raw.c +++ b/block/raw.c @@ -6,6 +6,7 @@ static int raw_open(BlockDriverState *bs, int flags) { bs->sg = bs->file->sg; + bs->iscsi = bs->file->iscsi; return 0; } diff --git a/block_int.h b/block_int.h index b80e66d..a6a9a1a 100644 --- a/block_int.h +++ b/block_int.h @@ -269,6 +269,7 @@ struct BlockDriverState { int encrypted; /* if true, the media is encrypted */ int valid_key; /* if true, a valid encryption key has been set */ int sg; /* if true, the device is a /dev/sg* */ + int iscsi; /* if true, the device is a iscsi://... */ int copy_on_read; /* if true, copy read backing sectors into image note this is a reference count */ diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c index 8ab9bcd..1466b1d 100644 --- a/hw/scsi-bus.c +++ b/hw/scsi-bus.c @@ -199,7 +199,12 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockDriverState *bdrv, const char *driver; DeviceState *dev; - driver = bdrv_is_sg(bdrv) ? "scsi-generic" : "scsi-disk"; + if (bdrv_is_sg(bdrv) || bdrv_is_iscsi(bdrv)) { + driver = "scsi-generic"; + } else { + driver = "scsi-disk"; + } + dev = qdev_create(&bus->qbus, driver); qdev_prop_set_uint32(dev, "scsi-id", unit); if (bootindex >= 0) { diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index 045c764..925dbcf 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -1711,6 +1711,10 @@ static int scsi_initfn(SCSIDevice *dev) error_report("unwanted /dev/sg*"); return -1; } + if (bdrv_is_iscsi(s->qdev.conf.bs)) { + error_report("unwanted iscsi://*"); + return -1; + } if (s->features & (1 << SCSI_DISK_F_REMOVABLE)) { bdrv_set_dev_ops(s->qdev.conf.bs, &scsi_cd_block_ops, s); diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c index d856d23..dfc5c54 100644 --- a/hw/scsi-generic.c +++ b/hw/scsi-generic.c @@ -401,8 +401,8 @@ static int scsi_generic_initfn(SCSIDevice *s) } /* check we are really using a /dev/sg* file */ - if (!bdrv_is_sg(s->conf.bs)) { - error_report("not /dev/sg*"); + if (!bdrv_is_sg(s->conf.bs) && !bdrv_is_iscsi(s->conf.bs)) { + error_report("not /dev/sg* or iscsi://"); return -1; } -- 1.7.3.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] SCSI: Add SCSI passthrough via scsi-generic to libiscsi 2012-05-25 7:43 ` [Qemu-devel] [PATCH 2/2] SCSI: Add SCSI passthrough via scsi-generic to libiscsi Ronnie Sahlberg @ 2012-05-25 10:20 ` Paolo Bonzini 0 siblings, 0 replies; 6+ messages in thread From: Paolo Bonzini @ 2012-05-25 10:20 UTC (permalink / raw) To: Ronnie Sahlberg; +Cc: kwolf, qemu-devel Il 25/05/2012 09:43, Ronnie Sahlberg ha scritto: > Update scsi-generic to allow passthrough of SG_IO scsi commands > to iscsi devices too in addition to the real scsi-generic devices. > > Implement both bdrv_ioctl() and bdrv_aio_ioctl() in the iscsi backend, > emulate the SG_IO ioctl and pass the SCSI commands across to the > iscsi target. > > This allows end-to-end passthrough of SCSI all the way from the guest, > to qemu, via scsi-generic, then libiscsi all the way to the iscsi target. > > To activate this you need to specify interface type as scsi when > creating the device. > > Example: > -drive file=iscsi://10.1.1.125/iqn.ronnie.test/1,if=scsi,bus=0,unit=5 > > Note, you can currently not boot a qemu guest from a 'if=scsi' device. > > Note, > This only works when the host is linux, since the emulation relies on > definitions of SG_IO from the scsi-generic implementation in the > linux kernel. > It should be fairly easy to re-implement some structures similar enough > for non-linux hosts to do the same style of passthrough via a fake > scsi generic layer and libiscsi if need be. > > Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com> > --- > block.c | 18 ++++-- > block.h | 1 + > block/iscsi.c | 191 ++++++++++++++++++++++++++++++++++++++++++++++++++++- > block/raw.c | 1 + > block_int.h | 1 + > hw/scsi-bus.c | 7 ++- > hw/scsi-disk.c | 4 + > hw/scsi-generic.c | 4 +- > 8 files changed, 217 insertions(+), 10 deletions(-) > > diff --git a/block.c b/block.c > index af2ab4f..2467b8e 100644 > --- a/block.c > +++ b/block.c > @@ -507,10 +507,11 @@ static int find_image_format(const char *filename, BlockDriver **pdrv) > return ret; > } > > - /* Return the raw BlockDriver * to scsi-generic devices or empty drives */ > - if (bs->sg || !bdrv_is_inserted(bs)) { > + /* Return the raw BlockDriver * to scsi-generic devices, > + iscsi devices or empty drives */ > + if (bs->sg || bs->iscsi || !bdrv_is_inserted(bs)) { > bdrv_delete(bs); > - drv = bdrv_find_format("raw"); > + drv = bdrv_find_format(bs->iscsi ? "iscsi" : "raw"); > if (!drv) { > ret = -ENOENT; > } Is this necessary? > @@ -550,9 +551,10 @@ static int refresh_total_sectors(BlockDriverState *bs, int64_t hint) > { > BlockDriver *drv = bs->drv; > > - /* Do not attempt drv->bdrv_getlength() on scsi-generic devices */ > - if (bs->sg) > + /* Do not attempt drv->bdrv_getlength() on scsi-generic/iscsi devices */ > + if (bs->sg || bs->iscsi) { > return 0; > + } > > /* query actual device if possible, otherwise just trust the hint */ > if (drv->bdrv_getlength) { Same here. > @@ -879,6 +881,7 @@ void bdrv_close(BlockDriverState *bs) > bs->encrypted = 0; > bs->valid_key = 0; > bs->sg = 0; > + bs->iscsi = 0; > bs->growable = 0; > > if (bs->file != NULL) { > @@ -2355,6 +2358,11 @@ int bdrv_is_sg(BlockDriverState *bs) > return bs->sg; > } > > +int bdrv_is_iscsi(BlockDriverState *bs) > +{ > + return bs->iscsi; > +} > + > int bdrv_enable_write_cache(BlockDriverState *bs) > { > return bs->enable_write_cache; > diff --git a/block.h b/block.h > index 7408acc..642194b 100644 > --- a/block.h > +++ b/block.h > @@ -279,6 +279,7 @@ void bdrv_set_on_error(BlockDriverState *bs, BlockErrorAction on_read_error, > BlockErrorAction bdrv_get_on_error(BlockDriverState *bs, int is_read); > int bdrv_is_read_only(BlockDriverState *bs); > int bdrv_is_sg(BlockDriverState *bs); > +int bdrv_is_iscsi(BlockDriverState *bs); > int bdrv_enable_write_cache(BlockDriverState *bs); > int bdrv_is_inserted(BlockDriverState *bs); > int bdrv_media_changed(BlockDriverState *bs); > diff --git a/block/iscsi.c b/block/iscsi.c > index 9642ee6..301ab69 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c > @@ -34,10 +34,15 @@ > #include <iscsi/iscsi.h> > #include <iscsi/scsi-lowlevel.h> > > +#ifdef __linux__ > +#include <scsi/sg.h> > +#include <hw/scsi-defs.h> > +#endif > > typedef struct IscsiLun { > struct iscsi_context *iscsi; > int lun; > + enum scsi_inquiry_peripheral_device_type type; > int block_size; > unsigned long num_blocks; > IOHandler *read_handler; > @@ -55,6 +60,9 @@ typedef struct IscsiAIOCB { > int canceled; > size_t read_size; > size_t read_offset; > +#ifdef __linux__ > + sg_io_hdr_t *ioh; > +#endif > } IscsiAIOCB; > > struct IscsiTask { > @@ -526,6 +534,140 @@ iscsi_aio_discard(BlockDriverState *bs, > return &acb->common; > } > > +#ifdef __linux__ > +static void > +iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status, > + void *command_data, void *opaque) > +{ > + IscsiAIOCB *acb = opaque; > + > + if (acb->canceled != 0) { > + qemu_aio_release(acb); > + scsi_free_scsi_task(acb->task); > + acb->task = NULL; > + return; > + } > + > + acb->status = 0; > + if (status < 0) { > + error_report("Failed to ioctl(SG_IO) to iSCSI lun. %s", > + iscsi_get_error(iscsi)); > + acb->status = -EIO; > + } > + > + acb->ioh->driver_status = 0; > + acb->ioh->host_status = 0; > + acb->ioh->resid = 0; > + > +#define SG_ERR_DRIVER_SENSE 0x08 > + > + if (status == SCSI_STATUS_CHECK_CONDITION && acb->task->datain.size >= 2) { > + int ss; > + > + acb->ioh->driver_status |= SG_ERR_DRIVER_SENSE; > + > + acb->ioh->sb_len_wr = acb->task->datain.size - 2; > + ss = (acb->ioh->mx_sb_len >= acb->ioh->sb_len_wr) ? > + acb->ioh->mx_sb_len : acb->ioh->sb_len_wr; > + memcpy(acb->ioh->sbp, &acb->task->datain.data[2], ss); > + } > + > + iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb); > + scsi_free_scsi_task(acb->task); > + acb->task = NULL; > +} > + > +static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs, > + unsigned long int req, void *buf, > + BlockDriverCompletionFunc *cb, void *opaque) > +{ > + IscsiLun *iscsilun = bs->opaque; > + struct iscsi_context *iscsi = iscsilun->iscsi; > + struct iscsi_data data; > + IscsiAIOCB *acb; > + > + if (req != SG_IO) { > + error_report("iSCSI: iscsi_aio_ioctl called for non-SG_IO " > + "ioctl"); > + return NULL; > + } I think you can assert here. > + > + acb = qemu_aio_get(&iscsi_aio_pool, bs, cb, opaque); > + > + acb->iscsilun = iscsilun; > + acb->canceled = 0; > + acb->buf = NULL; > + acb->ioh = buf; > + > + acb->task = malloc(sizeof(struct scsi_task)); > + if (acb->task == NULL) { > + error_report("iSCSI: Failed to allocate task for scsi command. %s", > + iscsi_get_error(iscsi)); > + qemu_aio_release(acb); > + return NULL; > + } > + memset(acb->task, 0, sizeof(struct scsi_task)); > + > + switch (acb->ioh->dxfer_direction) { > + case SG_DXFER_TO_DEV: > + acb->task->xfer_dir = SCSI_XFER_WRITE; > + break; > + case SG_DXFER_FROM_DEV: > + acb->task->xfer_dir = SCSI_XFER_READ; > + break; > + default: > + acb->task->xfer_dir = SCSI_XFER_NONE; > + break; > + } > + > + acb->task->cdb_size = acb->ioh->cmd_len; > + memcpy(&acb->task->cdb[0], acb->ioh->cmdp, acb->ioh->cmd_len); > + acb->task->expxferlen = acb->ioh->dxfer_len; > + > + if (acb->task->xfer_dir == SCSI_XFER_WRITE) { > + 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, > + acb) != 0) { > + scsi_free_scsi_task(acb->task); > + qemu_aio_release(acb); > + return NULL; > + } > + > + /* 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); > + } > + > + iscsi_set_events(iscsilun); > + > + return &acb->common; > +} > + > +static int iscsi_ioctl(BlockDriverState *bs, unsigned long int req, void *buf) > +{ > + IscsiLun *iscsilun = bs->opaque; > + > + switch (req) { > + case SG_GET_VERSION_NUM: > + *(int *)buf = 30000; > + break; > + case SG_GET_SCSI_ID: > + ((struct sg_scsi_id *)buf)->scsi_type = iscsilun->type; > + break; > + default: > + return -1; > + } > + return 0; > +} > +#endif > + > static int64_t > iscsi_getlength(BlockDriverState *bs) > { > @@ -575,18 +717,33 @@ iscsi_readcapacity16_cb(struct iscsi_context *iscsi, int status, > } > > static void > -iscsi_connect_cb(struct iscsi_context *iscsi, int status, void *command_data, > +iscsi_inquiry_cb(struct iscsi_context *iscsi, int status, void *command_data, > void *opaque) > { > struct IscsiTask *itask = opaque; > - struct scsi_task *task; > + struct scsi_task *task = command_data; > + struct scsi_inquiry_standard *inq; > > if (status != 0) { > itask->status = 1; > itask->complete = 1; > + scsi_free_scsi_task(task); > + return; > + } > + > + inq = scsi_datain_unmarshall(task); > + if (inq == NULL) { > + error_report("iSCSI: Failed to unmarshall inquiry data."); > + itask->status = 1; > + itask->complete = 1; > + scsi_free_scsi_task(task); > return; > } > > + itask->iscsilun->type = inq->periperal_device_type; > + > + scsi_free_scsi_task(task); > + > task = iscsi_readcapacity16_task(iscsi, itask->iscsilun->lun, > iscsi_readcapacity16_cb, opaque); > if (task == NULL) { > @@ -597,6 +754,30 @@ iscsi_connect_cb(struct iscsi_context *iscsi, int status, void *command_data, > } > } > > +static void > +iscsi_connect_cb(struct iscsi_context *iscsi, int status, void *command_data, > + void *opaque) > +{ > + struct IscsiTask *itask = opaque; > + struct scsi_task *task; > + > + if (status != 0) { > + itask->status = 1; > + itask->complete = 1; > + return; > + } > + > + task = iscsi_inquiry_task(iscsi, itask->iscsilun->lun, > + 0, 0, 36, > + iscsi_inquiry_cb, opaque); > + if (task == NULL) { > + error_report("iSCSI: failed to send inquiry command."); > + itask->status = 1; > + itask->complete = 1; > + return; > + } > +} > + > static int parse_chap(struct iscsi_context *iscsi, const char *target) > { > QemuOptsList *list; > @@ -803,6 +984,7 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags) > if (iscsi_url != NULL) { > iscsi_destroy_url(iscsi_url); > } > + bs->iscsi = 1; > return 0; > > failed: > @@ -846,6 +1028,11 @@ static BlockDriver bdrv_iscsi = { > .bdrv_aio_flush = iscsi_aio_flush, > > .bdrv_aio_discard = iscsi_aio_discard, > + > +#ifdef __linux__ > + .bdrv_ioctl = iscsi_ioctl, > + .bdrv_aio_ioctl = iscsi_aio_ioctl, > +#endif > }; > > static void iscsi_block_init(void) > diff --git a/block/raw.c b/block/raw.c > index 7086e31..62fc8c6 100644 > --- a/block/raw.c > +++ b/block/raw.c > @@ -6,6 +6,7 @@ > static int raw_open(BlockDriverState *bs, int flags) > { > bs->sg = bs->file->sg; > + bs->iscsi = bs->file->iscsi; > return 0; > } > > diff --git a/block_int.h b/block_int.h > index b80e66d..a6a9a1a 100644 > --- a/block_int.h > +++ b/block_int.h > @@ -269,6 +269,7 @@ struct BlockDriverState { > int encrypted; /* if true, the media is encrypted */ > int valid_key; /* if true, a valid encryption key has been set */ > int sg; /* if true, the device is a /dev/sg* */ > + int iscsi; /* if true, the device is a iscsi://... */ This may turn out to be completely unnecessary. > int copy_on_read; /* if true, copy read backing sectors into image > note this is a reference count */ > > diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c > index 8ab9bcd..1466b1d 100644 > --- a/hw/scsi-bus.c > +++ b/hw/scsi-bus.c > @@ -199,7 +199,12 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockDriverState *bdrv, > const char *driver; > DeviceState *dev; > > - driver = bdrv_is_sg(bdrv) ? "scsi-generic" : "scsi-disk"; > + if (bdrv_is_sg(bdrv) || bdrv_is_iscsi(bdrv)) { Hmm, for the legacy option I'm quite undecided as to whether you want an emulated target, or rather passthrough. On the other hand, making behavior different on Linux hosts vs. other hosts is a bit bad... I think we should restrict this to "-device scsi-generic". > + driver = "scsi-generic"; > + } else { > + driver = "scsi-disk"; > + } > + > dev = qdev_create(&bus->qbus, driver); > qdev_prop_set_uint32(dev, "scsi-id", unit); > if (bootindex >= 0) { > diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c > index 045c764..925dbcf 100644 > --- a/hw/scsi-disk.c > +++ b/hw/scsi-disk.c > @@ -1711,6 +1711,10 @@ static int scsi_initfn(SCSIDevice *dev) > error_report("unwanted /dev/sg*"); > return -1; > } > + if (bdrv_is_iscsi(s->qdev.conf.bs)) { > + error_report("unwanted iscsi://*"); > + return -1; > + } Please do not do this. scsi-disk is perfectly capable to deal with an iscsi disk. > > if (s->features & (1 << SCSI_DISK_F_REMOVABLE)) { > bdrv_set_dev_ops(s->qdev.conf.bs, &scsi_cd_block_ops, s); > diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c > index d856d23..dfc5c54 100644 > --- a/hw/scsi-generic.c > +++ b/hw/scsi-generic.c > @@ -401,8 +401,8 @@ static int scsi_generic_initfn(SCSIDevice *s) > } > > /* check we are really using a /dev/sg* file */ > - if (!bdrv_is_sg(s->conf.bs)) { > - error_report("not /dev/sg*"); > + if (!bdrv_is_sg(s->conf.bs) && !bdrv_is_iscsi(s->conf.bs)) { > + error_report("not /dev/sg* or iscsi://"); > return -1; > } > Here I think you can remove this altogether. Passing /dev/sda to scsi-generic should just work. The test that matters is SG_GET_VERSION_NUM, perhaps you can do the test with two different error messages: if (bdrv_ioctl(s->conf.bs, SG_GET_VERSION_NUM, &sg_version) < 0) { error_report("scsi generic interface not supported"); return -1; } if (sg_version < 30000) { error_report("scsi generic interface too old"); return -1; } Paolo ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-05-25 10:20 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-05-25 7:43 [Qemu-devel] [PATCH 0/2] Add support for iscsi passthrough and better eventsystem support Ronnie Sahlberg 2012-05-25 7:43 ` [Qemu-devel] [PATCH 1/2] ISCSI: redo how we set up the events to only call qemu_aio_set_fd_handler() and qemu_notify_event() if something has changed Ronnie Sahlberg 2012-05-25 9:59 ` Paolo Bonzini 2012-05-25 10:06 ` ronnie sahlberg 2012-05-25 7:43 ` [Qemu-devel] [PATCH 2/2] SCSI: Add SCSI passthrough via scsi-generic to libiscsi Ronnie Sahlberg 2012-05-25 10:20 ` 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).