Re: [Qemu-devel] [PATCH] iscsi: fix deadlock during login

[Peter Lieven]

Am 15.11.2012 um 15:57 schrieb ronnie sahlberg <address@hidden>:

> I dont know if we should switch to use synchronous code here.
> It is much nicer if all code is async.

Of course, but its just the initial login after which qemu should exit if it 
fails.

> 
> Is it possible to add a timeout instead that would break out if the
> connect/login has not completed within a certain amount of time?

I think it is, but this has to be done by qemu or not?

I found switching to synchronous code fixed 2 issues at once, the issue
that it hangs if the connection is interrupted during login and another
issue that i submitted a patch for earlier:

https://github.com/sahlberg/libiscsi/commit/a9257d52a7577b607e237e209b9868c5ce78a927

it might be that this fix introduced the new issue.

Peter


> 
> 
> regards
> ronnie sahlberg
> 
> On Thu, Nov 15, 2012 at 6:50 AM, Peter Lieven <address@hidden> wrote:
>> If the connection is interrupted before the first login is successfully
>> completed qemu-kvm is waiting forever in qemu_aio_wait().
>> 
>> This is fixed by performing an sync login to the target. If the
>> connection breaks after the first successful login errors are
>> handled internally by libiscsi.
>> 
>> Signed-off-by: Peter Lieven <address@hidden>
>> ---
>> block/iscsi.c |   56
>> +++++++++++++++++++++-----------------------------------
>> 1 file changed, 21 insertions(+), 35 deletions(-)
>> 
>> diff --git a/block/iscsi.c b/block/iscsi.c
>> index b5c3161..f44bb57 100644
>> --- a/block/iscsi.c
>> +++ b/block/iscsi.c
>> @@ -798,30 +798,6 @@ iscsi_inquiry_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;
>> @@ -934,7 +910,8 @@ static int iscsi_open(BlockDriverState *bs, const char
>> *filename, int flags)
>>     IscsiLun *iscsilun = bs->opaque;
>>     struct iscsi_context *iscsi = NULL;
>>     struct iscsi_url *iscsi_url = NULL;
>> -    struct IscsiTask task;
>> +    struct IscsiTask itask;
>> +    struct scsi_task *task;
>>     char *initiator_name = NULL;
>>     int ret;
>> 
>> @@ -997,27 +974,36 @@ static int iscsi_open(BlockDriverState *bs, const char
>> *filename, int flags)
>>     /* check if we got HEADER_DIGEST via the options */
>>     parse_header_digest(iscsi, iscsi_url->target);
>> 
>> -    task.iscsilun = iscsilun;
>> -    task.status = 0;
>> -    task.complete = 0;
>> -    task.bs = bs;
>> +    if (iscsi_full_connect_sync(iscsi, iscsi_url->portal, iscsi_url->lun)
>> != 0) {
>> +        error_report("iSCSI: Failed to connect to LUN : %s",
>> +            iscsi_get_error(iscsi));
>> +        ret = -EINVAL;
>> +        goto out;
>> +    }
>> +
>> +    itask.iscsilun = iscsilun;
>> +    itask.status = 0;
>> +    itask.complete = 0;
>> +    itask.bs = bs;
>> 
>>     iscsilun->iscsi = iscsi;
>>     iscsilun->lun   = iscsi_url->lun;
>> 
>> -    if (iscsi_full_connect_async(iscsi, iscsi_url->portal, iscsi_url->lun,
>> -                                 iscsi_connect_cb, &task)
>> -        != 0) {
>> -        error_report("iSCSI: Failed to start async connect.");
>> +    task = iscsi_inquiry_task(iscsi, iscsilun->lun,
>> +                              0, 0, 36,
>> +                              iscsi_inquiry_cb, &itask);
>> +    if (task == NULL) {
>> +        error_report("iSCSI: failed to send inquiry command.");
>>         ret = -EINVAL;
>>         goto out;
>>     }
>> 
>> -    while (!task.complete) {
>> +    while (!itask.complete) {
>>         iscsi_set_events(iscsilun);
>>         qemu_aio_wait();
>>     }
>> -    if (task.status != 0) {
>> +
>> +    if (itask.status != 0) {
>>         error_report("iSCSI: Failed to connect to LUN : %s",
>>                      iscsi_get_error(iscsi));
>>         ret = -EINVAL;
>> --
>> 1.7.9.5