| Message ID | 1372338695-411-4-git-send-email-pl@kamp.de |
|---|---|
| State | New |
| Headers | show |
On Thu, Jun 27, 2013 at 03:11:27PM +0200, Peter Lieven wrote: > @@ -113,6 +122,45 @@ iscsi_schedule_bh(IscsiAIOCB *acb) > qemu_bh_schedule(acb->bh); > } > > +static void > +iscsi_co_generic_cb(struct iscsi_context *iscsi, int status, > + void *command_data, void *opaque) > +{ > + struct IscsiTask *iTask = opaque; > + struct scsi_task *task = command_data; > + > + iTask->complete = 1; > + iTask->status = status; > + iTask->do_retry = 0; > + iTask->task = task; > + > + if (iTask->retries-- > 0 && status == SCSI_STATUS_CHECK_CONDITION > + && task->sense.key == SCSI_SENSE_UNIT_ATTENTION) { > + iTask->do_retry = 1; > + goto out; > + } > + > + if (status != SCSI_STATUS_GOOD) { > + error_report("iSCSI: Failure. %s", iscsi_get_error(iscsi)); > + goto out; > + } > + > +out: > + if (iTask->status != SCSI_STATUS_GOOD) { > + scsi_free_scsi_task(iTask->task); > + iTask->task = NULL; > + } Not sure about freeing the task here since the caller is still responsible for freeing the task in the success case and higher-level error cases. I suggest keeping iTask->task alive here so the caller is easier to maintain (it can use a single out/error path).
Am 01.07.2013 um 15:46 schrieb Stefan Hajnoczi <stefanha@redhat.com>: > On Thu, Jun 27, 2013 at 03:11:27PM +0200, Peter Lieven wrote: >> @@ -113,6 +122,45 @@ iscsi_schedule_bh(IscsiAIOCB *acb) >> qemu_bh_schedule(acb->bh); >> } >> >> +static void >> +iscsi_co_generic_cb(struct iscsi_context *iscsi, int status, >> + void *command_data, void *opaque) >> +{ >> + struct IscsiTask *iTask = opaque; >> + struct scsi_task *task = command_data; >> + >> + iTask->complete = 1; >> + iTask->status = status; >> + iTask->do_retry = 0; >> + iTask->task = task; >> + >> + if (iTask->retries-- > 0 && status == SCSI_STATUS_CHECK_CONDITION >> + && task->sense.key == SCSI_SENSE_UNIT_ATTENTION) { >> + iTask->do_retry = 1; >> + goto out; >> + } >> + >> + if (status != SCSI_STATUS_GOOD) { >> + error_report("iSCSI: Failure. %s", iscsi_get_error(iscsi)); >> + goto out; >> + } >> + >> +out: >> + if (iTask->status != SCSI_STATUS_GOOD) { >> + scsi_free_scsi_task(iTask->task); >> + iTask->task = NULL; >> + } > > Not sure about freeing the task here since the caller is still > responsible for freeing the task in the success case and higher-level > error cases. > > I suggest keeping iTask->task alive here so the caller is easier to > maintain (it can use a single out/error path). ok, will change this in v3
Am 27.06.2013 um 15:11 hat Peter Lieven geschrieben: > this patch adds a coroutine for .bdrv_co_is_allocated as well as > a generic framework that can be used to build coroutines in block/iscsi. > > Signed-off-by: Peter Lieven <pl@kamp.de> > --- > block/iscsi.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 123 insertions(+) > > diff --git a/block/iscsi.c b/block/iscsi.c > index 2e2455d..d31ee95 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c > @@ -58,6 +58,15 @@ typedef struct IscsiLun { > uint32_t max_unmap_bdc; > } IscsiLun; > > +typedef struct IscsiTask { > + int status; > + int complete; > + int retries; > + int do_retry; > + struct scsi_task *task; > + Coroutine *co; > +} IscsiTask; > + > typedef struct IscsiAIOCB { > BlockDriverAIOCB common; > QEMUIOVector *qiov; > @@ -113,6 +122,45 @@ iscsi_schedule_bh(IscsiAIOCB *acb) > qemu_bh_schedule(acb->bh); > } > > +static void > +iscsi_co_generic_cb(struct iscsi_context *iscsi, int status, > + void *command_data, void *opaque) > +{ > + struct IscsiTask *iTask = opaque; > + struct scsi_task *task = command_data; > + > + iTask->complete = 1; > + iTask->status = status; > + iTask->do_retry = 0; > + iTask->task = task; > + > + if (iTask->retries-- > 0 && status == SCSI_STATUS_CHECK_CONDITION > + && task->sense.key == SCSI_SENSE_UNIT_ATTENTION) { > + iTask->do_retry = 1; > + goto out; > + } > + > + if (status != SCSI_STATUS_GOOD) { > + error_report("iSCSI: Failure. %s", iscsi_get_error(iscsi)); > + goto out; > + } > + > +out: > + if (iTask->status != SCSI_STATUS_GOOD) { > + scsi_free_scsi_task(iTask->task); > + iTask->task = NULL; > + } > + if (iTask->co) { > + qemu_coroutine_enter(iTask->co, NULL); > + } > +} In which context does this callback run? If this is from a separate thread, you may not do all of this (without even holding the QBL). You should probably use a BH to switch to the I/O thread. > + > +static void iscsi_co_init_iscsitask(IscsiLun *iscsilun, struct IscsiTask *iTask) > +{ > + memset(iTask, 0x00, sizeof(struct IscsiTask)); > + iTask->co = qemu_coroutine_self(); > + iTask->retries = ISCSI_CMD_RETRIES; > +} Matter of taste, but in my own code I prefer not to have a memset: *iTask = (struct IscsiTask) { .co = qemu_coroutine_self(), .retries = ISCSI_CMD_RETRIES, }; > static void > iscsi_abort_task_cb(struct iscsi_context *iscsi, int status, void *command_data, > @@ -807,6 +855,79 @@ iscsi_getlength(BlockDriverState *bs) > return len; > } > > +static int coroutine_fn iscsi_co_is_allocated(BlockDriverState *bs, > + int64_t sector_num, > + int nb_sectors, int *pnum) > +{ > + IscsiLun *iscsilun = bs->opaque; > + struct scsi_get_lba_status *lbas = NULL; > + struct scsi_lba_status_descriptor *lbasd = NULL; > + struct IscsiTask iTask; > + int ret; > + > + /* LUN does not support logical block provisioning */ > + if (iscsilun->lbpme == 0) { > + *pnum = nb_sectors; > + return 1; > + } > + > + iscsi_co_init_iscsitask(iscsilun, &iTask); > + > +retry: > + if (iscsi_get_lba_status_task(iscsilun->iscsi, iscsilun->lun, > + sector_qemu2lun(sector_num, iscsilun), > + 8 + 16, iscsi_co_generic_cb, > + &iTask) == NULL) { > + goto fail; > + } > + > + while (!iTask.complete) { > + iscsi_set_events(iscsilun); > + qemu_coroutine_yield(); > + } > + > + if (iTask.do_retry) { > + goto retry; > + } > + > + if (iTask.status != SCSI_STATUS_GOOD) { > + /* in case the get_lba_status_callout fails (i.e. > + * because the device is busy or the cmd is not > + * supported) we pretend all blocks are allocated > + * for backwards compatiblity */ > + *pnum = nb_sectors; > + return 1; > + } > + > + lbas = scsi_datain_unmarshall(iTask.task); > + if (lbas == NULL) { > + goto fail; > + } > + > + lbasd = &lbas->descriptors[0]; > + > + if (sector_qemu2lun(sector_num, iscsilun) != lbasd->lba) { > + goto fail; > + } > + > + *pnum = lbasd->num_blocks * (iscsilun->block_size / BDRV_SECTOR_SIZE); > + if (*pnum > nb_sectors) { > + *pnum = nb_sectors; > + } > + > + ret = lbasd->provisioning == SCSI_PROVISIONING_TYPE_MAPPED ? 1 : 0; Wouldn't it be safer to default to allocated, in case new values are added in a later protocol version? I agree with returning 0 for both unmapped and anchored, though. > + scsi_free_scsi_task(iTask.task); > + return ret; > + > +fail: > + if (iTask.task != NULL) { > + scsi_free_scsi_task(iTask.task); > + } > + *pnum = 0; > + return 0; > +} Kevin
Am 10.07.2013 11:41, schrieb Kevin Wolf: > Am 27.06.2013 um 15:11 hat Peter Lieven geschrieben: >> this patch adds a coroutine for .bdrv_co_is_allocated as well as >> a generic framework that can be used to build coroutines in block/iscsi. >> >> Signed-off-by: Peter Lieven <pl@kamp.de> >> --- >> block/iscsi.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 123 insertions(+) >> >> diff --git a/block/iscsi.c b/block/iscsi.c >> index 2e2455d..d31ee95 100644 >> --- a/block/iscsi.c >> +++ b/block/iscsi.c >> @@ -58,6 +58,15 @@ typedef struct IscsiLun { >> uint32_t max_unmap_bdc; >> } IscsiLun; >> >> +typedef struct IscsiTask { >> + int status; >> + int complete; >> + int retries; >> + int do_retry; >> + struct scsi_task *task; >> + Coroutine *co; >> +} IscsiTask; >> + >> typedef struct IscsiAIOCB { >> BlockDriverAIOCB common; >> QEMUIOVector *qiov; >> @@ -113,6 +122,45 @@ iscsi_schedule_bh(IscsiAIOCB *acb) >> qemu_bh_schedule(acb->bh); >> } >> >> +static void >> +iscsi_co_generic_cb(struct iscsi_context *iscsi, int status, >> + void *command_data, void *opaque) >> +{ >> + struct IscsiTask *iTask = opaque; >> + struct scsi_task *task = command_data; >> + >> + iTask->complete = 1; >> + iTask->status = status; >> + iTask->do_retry = 0; >> + iTask->task = task; >> + >> + if (iTask->retries-- > 0 && status == SCSI_STATUS_CHECK_CONDITION >> + && task->sense.key == SCSI_SENSE_UNIT_ATTENTION) { >> + iTask->do_retry = 1; >> + goto out; >> + } >> + >> + if (status != SCSI_STATUS_GOOD) { >> + error_report("iSCSI: Failure. %s", iscsi_get_error(iscsi)); >> + goto out; >> + } >> + >> +out: >> + if (iTask->status != SCSI_STATUS_GOOD) { >> + scsi_free_scsi_task(iTask->task); >> + iTask->task = NULL; >> + } >> + if (iTask->co) { >> + qemu_coroutine_enter(iTask->co, NULL); >> + } >> +} > In which context does this callback run? If this is from a separate > thread, you may not do all of this (without even holding the QBL). You > should probably use a BH to switch to the I/O thread. good question. the callbacks can only be fired when iscsi_service() is invoked. this is currently done by registereing iscsi_process_read/write/flush via qemu_aio_set_fd_handler(). so the callbacks are invoked in the aio context it seems. can this be right? >> + >> +static void iscsi_co_init_iscsitask(IscsiLun *iscsilun, struct IscsiTask *iTask) >> +{ >> + memset(iTask, 0x00, sizeof(struct IscsiTask)); >> + iTask->co = qemu_coroutine_self(); >> + iTask->retries = ISCSI_CMD_RETRIES; >> +} > Matter of taste, but in my own code I prefer not to have a memset: > > *iTask = (struct IscsiTask) { > .co = qemu_coroutine_self(), > .retries = ISCSI_CMD_RETRIES, > }; ok > >> static void >> iscsi_abort_task_cb(struct iscsi_context *iscsi, int status, void *command_data, >> @@ -807,6 +855,79 @@ iscsi_getlength(BlockDriverState *bs) >> return len; >> } >> >> +static int coroutine_fn iscsi_co_is_allocated(BlockDriverState *bs, >> + int64_t sector_num, >> + int nb_sectors, int *pnum) >> +{ >> + IscsiLun *iscsilun = bs->opaque; >> + struct scsi_get_lba_status *lbas = NULL; >> + struct scsi_lba_status_descriptor *lbasd = NULL; >> + struct IscsiTask iTask; >> + int ret; >> + >> + /* LUN does not support logical block provisioning */ >> + if (iscsilun->lbpme == 0) { >> + *pnum = nb_sectors; >> + return 1; >> + } >> + >> + iscsi_co_init_iscsitask(iscsilun, &iTask); >> + >> +retry: >> + if (iscsi_get_lba_status_task(iscsilun->iscsi, iscsilun->lun, >> + sector_qemu2lun(sector_num, iscsilun), >> + 8 + 16, iscsi_co_generic_cb, >> + &iTask) == NULL) { >> + goto fail; >> + } >> + >> + while (!iTask.complete) { >> + iscsi_set_events(iscsilun); >> + qemu_coroutine_yield(); >> + } >> + >> + if (iTask.do_retry) { >> + goto retry; >> + } >> + >> + if (iTask.status != SCSI_STATUS_GOOD) { >> + /* in case the get_lba_status_callout fails (i.e. >> + * because the device is busy or the cmd is not >> + * supported) we pretend all blocks are allocated >> + * for backwards compatiblity */ >> + *pnum = nb_sectors; >> + return 1; >> + } >> + >> + lbas = scsi_datain_unmarshall(iTask.task); >> + if (lbas == NULL) { >> + goto fail; >> + } >> + >> + lbasd = &lbas->descriptors[0]; >> + >> + if (sector_qemu2lun(sector_num, iscsilun) != lbasd->lba) { >> + goto fail; >> + } >> + >> + *pnum = lbasd->num_blocks * (iscsilun->block_size / BDRV_SECTOR_SIZE); >> + if (*pnum > nb_sectors) { >> + *pnum = nb_sectors; >> + } >> + >> + ret = lbasd->provisioning == SCSI_PROVISIONING_TYPE_MAPPED ? 1 : 0; > Wouldn't it be safer to default to allocated, in case new values are > added in a later protocol version? I agree with returning 0 for both > unmapped and anchored, though. agreed. > >> + scsi_free_scsi_task(iTask.task); >> + return ret; >> + >> +fail: >> + if (iTask.task != NULL) { >> + scsi_free_scsi_task(iTask.task); >> + } >> + *pnum = 0; >> + return 0; >> +} > Kevin Peter
Am 10.07.2013 um 15:49 hat Peter Lieven geschrieben: > Am 10.07.2013 11:41, schrieb Kevin Wolf: > > Am 27.06.2013 um 15:11 hat Peter Lieven geschrieben: > >> this patch adds a coroutine for .bdrv_co_is_allocated as well as > >> a generic framework that can be used to build coroutines in block/iscsi. > >> > >> Signed-off-by: Peter Lieven <pl@kamp.de> > >> --- > >> block/iscsi.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 123 insertions(+) > >> > >> diff --git a/block/iscsi.c b/block/iscsi.c > >> index 2e2455d..d31ee95 100644 > >> --- a/block/iscsi.c > >> +++ b/block/iscsi.c > >> @@ -58,6 +58,15 @@ typedef struct IscsiLun { > >> uint32_t max_unmap_bdc; > >> } IscsiLun; > >> > >> +typedef struct IscsiTask { > >> + int status; > >> + int complete; > >> + int retries; > >> + int do_retry; > >> + struct scsi_task *task; > >> + Coroutine *co; > >> +} IscsiTask; > >> + > >> typedef struct IscsiAIOCB { > >> BlockDriverAIOCB common; > >> QEMUIOVector *qiov; > >> @@ -113,6 +122,45 @@ iscsi_schedule_bh(IscsiAIOCB *acb) > >> qemu_bh_schedule(acb->bh); > >> } > >> > >> +static void > >> +iscsi_co_generic_cb(struct iscsi_context *iscsi, int status, > >> + void *command_data, void *opaque) > >> +{ > >> + struct IscsiTask *iTask = opaque; > >> + struct scsi_task *task = command_data; > >> + > >> + iTask->complete = 1; > >> + iTask->status = status; > >> + iTask->do_retry = 0; > >> + iTask->task = task; > >> + > >> + if (iTask->retries-- > 0 && status == SCSI_STATUS_CHECK_CONDITION > >> + && task->sense.key == SCSI_SENSE_UNIT_ATTENTION) { > >> + iTask->do_retry = 1; > >> + goto out; > >> + } > >> + > >> + if (status != SCSI_STATUS_GOOD) { > >> + error_report("iSCSI: Failure. %s", iscsi_get_error(iscsi)); > >> + goto out; > >> + } > >> + > >> +out: > >> + if (iTask->status != SCSI_STATUS_GOOD) { > >> + scsi_free_scsi_task(iTask->task); > >> + iTask->task = NULL; > >> + } > >> + if (iTask->co) { > >> + qemu_coroutine_enter(iTask->co, NULL); > >> + } > >> +} > > In which context does this callback run? If this is from a separate > > thread, you may not do all of this (without even holding the QBL). You > > should probably use a BH to switch to the I/O thread. > good question. the callbacks can only be fired when iscsi_service() is invoked. > this is currently done by registereing iscsi_process_read/write/flush via qemu_aio_set_fd_handler(). > > so the callbacks are invoked in the aio context it seems. can this be right? Yes, I took another look and I think it's fine. Kevin
Am 10.07.2013 16:43, schrieb Kevin Wolf: > Am 10.07.2013 um 15:49 hat Peter Lieven geschrieben: >> Am 10.07.2013 11:41, schrieb Kevin Wolf: >>> Am 27.06.2013 um 15:11 hat Peter Lieven geschrieben: >>>> this patch adds a coroutine for .bdrv_co_is_allocated as well as >>>> a generic framework that can be used to build coroutines in block/iscsi. >>>> >>>> Signed-off-by: Peter Lieven <pl@kamp.de> >>>> --- >>>> block/iscsi.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>> 1 file changed, 123 insertions(+) >>>> >>>> diff --git a/block/iscsi.c b/block/iscsi.c >>>> index 2e2455d..d31ee95 100644 >>>> --- a/block/iscsi.c >>>> +++ b/block/iscsi.c >>>> @@ -58,6 +58,15 @@ typedef struct IscsiLun { >>>> uint32_t max_unmap_bdc; >>>> } IscsiLun; >>>> >>>> +typedef struct IscsiTask { >>>> + int status; >>>> + int complete; >>>> + int retries; >>>> + int do_retry; >>>> + struct scsi_task *task; >>>> + Coroutine *co; >>>> +} IscsiTask; >>>> + >>>> typedef struct IscsiAIOCB { >>>> BlockDriverAIOCB common; >>>> QEMUIOVector *qiov; >>>> @@ -113,6 +122,45 @@ iscsi_schedule_bh(IscsiAIOCB *acb) >>>> qemu_bh_schedule(acb->bh); >>>> } >>>> >>>> +static void >>>> +iscsi_co_generic_cb(struct iscsi_context *iscsi, int status, >>>> + void *command_data, void *opaque) >>>> +{ >>>> + struct IscsiTask *iTask = opaque; >>>> + struct scsi_task *task = command_data; >>>> + >>>> + iTask->complete = 1; >>>> + iTask->status = status; >>>> + iTask->do_retry = 0; >>>> + iTask->task = task; >>>> + >>>> + if (iTask->retries-- > 0 && status == SCSI_STATUS_CHECK_CONDITION >>>> + && task->sense.key == SCSI_SENSE_UNIT_ATTENTION) { >>>> + iTask->do_retry = 1; >>>> + goto out; >>>> + } >>>> + >>>> + if (status != SCSI_STATUS_GOOD) { >>>> + error_report("iSCSI: Failure. %s", iscsi_get_error(iscsi)); >>>> + goto out; >>>> + } >>>> + >>>> +out: >>>> + if (iTask->status != SCSI_STATUS_GOOD) { >>>> + scsi_free_scsi_task(iTask->task); >>>> + iTask->task = NULL; >>>> + } >>>> + if (iTask->co) { >>>> + qemu_coroutine_enter(iTask->co, NULL); >>>> + } >>>> +} >>> In which context does this callback run? If this is from a separate >>> thread, you may not do all of this (without even holding the QBL). You >>> should probably use a BH to switch to the I/O thread. >> good question. the callbacks can only be fired when iscsi_service() is invoked. >> this is currently done by registereing iscsi_process_read/write/flush via qemu_aio_set_fd_handler(). >> >> so the callbacks are invoked in the aio context it seems. can this be right? > Yes, I took another look and I think it's fine. Would this still be true if all aio routines are replaced by coroutines? Peter
Am 10.07.2013 um 16:49 hat Peter Lieven geschrieben: > Am 10.07.2013 16:43, schrieb Kevin Wolf: > > Am 10.07.2013 um 15:49 hat Peter Lieven geschrieben: > >> Am 10.07.2013 11:41, schrieb Kevin Wolf: > >>> Am 27.06.2013 um 15:11 hat Peter Lieven geschrieben: > >>>> this patch adds a coroutine for .bdrv_co_is_allocated as well as > >>>> a generic framework that can be used to build coroutines in block/iscsi. > >>>> > >>>> Signed-off-by: Peter Lieven <pl@kamp.de> > >>>> --- > >>>> block/iscsi.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > >>>> 1 file changed, 123 insertions(+) > >>>> > >>>> diff --git a/block/iscsi.c b/block/iscsi.c > >>>> index 2e2455d..d31ee95 100644 > >>>> --- a/block/iscsi.c > >>>> +++ b/block/iscsi.c > >>>> @@ -58,6 +58,15 @@ typedef struct IscsiLun { > >>>> uint32_t max_unmap_bdc; > >>>> } IscsiLun; > >>>> > >>>> +typedef struct IscsiTask { > >>>> + int status; > >>>> + int complete; > >>>> + int retries; > >>>> + int do_retry; > >>>> + struct scsi_task *task; > >>>> + Coroutine *co; > >>>> +} IscsiTask; > >>>> + > >>>> typedef struct IscsiAIOCB { > >>>> BlockDriverAIOCB common; > >>>> QEMUIOVector *qiov; > >>>> @@ -113,6 +122,45 @@ iscsi_schedule_bh(IscsiAIOCB *acb) > >>>> qemu_bh_schedule(acb->bh); > >>>> } > >>>> > >>>> +static void > >>>> +iscsi_co_generic_cb(struct iscsi_context *iscsi, int status, > >>>> + void *command_data, void *opaque) > >>>> +{ > >>>> + struct IscsiTask *iTask = opaque; > >>>> + struct scsi_task *task = command_data; > >>>> + > >>>> + iTask->complete = 1; > >>>> + iTask->status = status; > >>>> + iTask->do_retry = 0; > >>>> + iTask->task = task; > >>>> + > >>>> + if (iTask->retries-- > 0 && status == SCSI_STATUS_CHECK_CONDITION > >>>> + && task->sense.key == SCSI_SENSE_UNIT_ATTENTION) { > >>>> + iTask->do_retry = 1; > >>>> + goto out; > >>>> + } > >>>> + > >>>> + if (status != SCSI_STATUS_GOOD) { > >>>> + error_report("iSCSI: Failure. %s", iscsi_get_error(iscsi)); > >>>> + goto out; > >>>> + } > >>>> + > >>>> +out: > >>>> + if (iTask->status != SCSI_STATUS_GOOD) { > >>>> + scsi_free_scsi_task(iTask->task); > >>>> + iTask->task = NULL; > >>>> + } > >>>> + if (iTask->co) { > >>>> + qemu_coroutine_enter(iTask->co, NULL); > >>>> + } > >>>> +} > >>> In which context does this callback run? If this is from a separate > >>> thread, you may not do all of this (without even holding the QBL). You > >>> should probably use a BH to switch to the I/O thread. > >> good question. the callbacks can only be fired when iscsi_service() is invoked. > >> this is currently done by registereing iscsi_process_read/write/flush via qemu_aio_set_fd_handler(). > >> > >> so the callbacks are invoked in the aio context it seems. can this be right? > > Yes, I took another look and I think it's fine. > Would this still be true if all aio routines are replaced by coroutines? Yes, AIO callbacks and coroutines are mostly equivalent from the point of view of anything external to the block driver itself. Kevin
diff --git a/block/iscsi.c b/block/iscsi.c index 2e2455d..d31ee95 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -58,6 +58,15 @@ typedef struct IscsiLun { uint32_t max_unmap_bdc; } IscsiLun; +typedef struct IscsiTask { + int status; + int complete; + int retries; + int do_retry; + struct scsi_task *task; + Coroutine *co; +} IscsiTask; + typedef struct IscsiAIOCB { BlockDriverAIOCB common; QEMUIOVector *qiov; @@ -113,6 +122,45 @@ iscsi_schedule_bh(IscsiAIOCB *acb) qemu_bh_schedule(acb->bh); } +static void +iscsi_co_generic_cb(struct iscsi_context *iscsi, int status, + void *command_data, void *opaque) +{ + struct IscsiTask *iTask = opaque; + struct scsi_task *task = command_data; + + iTask->complete = 1; + iTask->status = status; + iTask->do_retry = 0; + iTask->task = task; + + if (iTask->retries-- > 0 && status == SCSI_STATUS_CHECK_CONDITION + && task->sense.key == SCSI_SENSE_UNIT_ATTENTION) { + iTask->do_retry = 1; + goto out; + } + + if (status != SCSI_STATUS_GOOD) { + error_report("iSCSI: Failure. %s", iscsi_get_error(iscsi)); + goto out; + } + +out: + if (iTask->status != SCSI_STATUS_GOOD) { + scsi_free_scsi_task(iTask->task); + iTask->task = NULL; + } + if (iTask->co) { + qemu_coroutine_enter(iTask->co, NULL); + } +} + +static void iscsi_co_init_iscsitask(IscsiLun *iscsilun, struct IscsiTask *iTask) +{ + memset(iTask, 0x00, sizeof(struct IscsiTask)); + iTask->co = qemu_coroutine_self(); + iTask->retries = ISCSI_CMD_RETRIES; +} static void iscsi_abort_task_cb(struct iscsi_context *iscsi, int status, void *command_data, @@ -807,6 +855,79 @@ iscsi_getlength(BlockDriverState *bs) return len; } +static int coroutine_fn iscsi_co_is_allocated(BlockDriverState *bs, + int64_t sector_num, + int nb_sectors, int *pnum) +{ + IscsiLun *iscsilun = bs->opaque; + struct scsi_get_lba_status *lbas = NULL; + struct scsi_lba_status_descriptor *lbasd = NULL; + struct IscsiTask iTask; + int ret; + + /* LUN does not support logical block provisioning */ + if (iscsilun->lbpme == 0) { + *pnum = nb_sectors; + return 1; + } + + iscsi_co_init_iscsitask(iscsilun, &iTask); + +retry: + if (iscsi_get_lba_status_task(iscsilun->iscsi, iscsilun->lun, + sector_qemu2lun(sector_num, iscsilun), + 8 + 16, iscsi_co_generic_cb, + &iTask) == NULL) { + goto fail; + } + + while (!iTask.complete) { + iscsi_set_events(iscsilun); + qemu_coroutine_yield(); + } + + if (iTask.do_retry) { + goto retry; + } + + if (iTask.status != SCSI_STATUS_GOOD) { + /* in case the get_lba_status_callout fails (i.e. + * because the device is busy or the cmd is not + * supported) we pretend all blocks are allocated + * for backwards compatiblity */ + *pnum = nb_sectors; + return 1; + } + + lbas = scsi_datain_unmarshall(iTask.task); + if (lbas == NULL) { + goto fail; + } + + lbasd = &lbas->descriptors[0]; + + if (sector_qemu2lun(sector_num, iscsilun) != lbasd->lba) { + goto fail; + } + + *pnum = lbasd->num_blocks * (iscsilun->block_size / BDRV_SECTOR_SIZE); + if (*pnum > nb_sectors) { + *pnum = nb_sectors; + } + + ret = lbasd->provisioning == SCSI_PROVISIONING_TYPE_MAPPED ? 1 : 0; + + scsi_free_scsi_task(iTask.task); + return ret; + +fail: + if (iTask.task != NULL) { + scsi_free_scsi_task(iTask.task); + } + *pnum = 0; + return 0; +} + static int parse_chap(struct iscsi_context *iscsi, const char *target) { QemuOptsList *list; @@ -1347,6 +1468,8 @@ static BlockDriver bdrv_iscsi = { .bdrv_getlength = iscsi_getlength, .bdrv_truncate = iscsi_truncate, + .bdrv_co_is_allocated = iscsi_co_is_allocated, + .bdrv_aio_readv = iscsi_aio_readv, .bdrv_aio_writev = iscsi_aio_writev, .bdrv_aio_flush = iscsi_aio_flush,
this patch adds a coroutine for .bdrv_co_is_allocated as well as a generic framework that can be used to build coroutines in block/iscsi. Signed-off-by: Peter Lieven <pl@kamp.de> --- block/iscsi.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 123 insertions(+)