Should we really mix co-routines and AIO in the same backend?
Would it not be better to instead add a new bdrb_aio_is_allocaed and
use non-blocking async calls to libiscsi ?
On Fri, Jun 21, 2013 at 2:18 AM, Kevin Wolf <kw...@redhat.com> wrote:
> Am 20.06.2013 um 20:20 hat Peter Lieven geschrieben:
>> Signed-off-by: Peter Lieven <p...@kamp.de>
>> ---
>> block/iscsi.c | 57
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 57 insertions(+)
>>
>> diff --git a/block/iscsi.c b/block/iscsi.c
>> index 0bbf0b1..e6b966d 100644
>> --- a/block/iscsi.c
>> +++ b/block/iscsi.c
>> @@ -49,6 +49,7 @@ typedef struct IscsiLun {
>> uint64_t num_blocks;
>> int events;
>> QEMUTimer *nop_timer;
>> + uint8_t lbpme;
>> } IscsiLun;
>>
>> typedef struct IscsiAIOCB {
>> @@ -800,6 +801,60 @@ 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_task *task = NULL;
>> + struct scsi_get_lba_status *lbas = NULL;
>> + struct scsi_lba_status_descriptor *lbasd = NULL;
>> + int ret;
>> +
>> + *pnum = nb_sectors;
>> +
>> + if (iscsilun->lbpme == 0) {
>> + return 1;
>> + }
>> +
>> + /* in-flight requests could invalidate the lba status result */
>> + while (iscsi_process_flush(iscsilun)) {
>> + qemu_aio_wait();
>> + }
>
> Note that you're blocking here. The preferred way would be something
> involving a yield from the coroutine and a reenter as soon as all
> requests are done. Maybe a CoRwLock does what you need?
>
>> +
>> + task = iscsi_get_lba_status_sync(iscsilun->iscsi, iscsilun->lun,
>> + sector_qemu2lun(sector_num, iscsilun),
>> + 8+16);
>
> Spacing around operators (8 + 16)
>
>> +
>> + if (task == NULL || task->status != SCSI_STATUS_GOOD) {
>> + scsi_free_scsi_task(task);
>> + return 1;
>
> Error cases should set *pnum = 0 and return 0.
>
> Kevin
