Re: [Qemu-devel] [PATCH][RESEND] iscsi: add support for iSCSI NOPs

[Peter Lieven]

All, please ignore this patch. During testing we found a problem in live
usage. We will resubmit once this is fixed.

Peter

Am 04.12.2012 um 06:03 schrieb ronnie sahlberg:

> Acked-By: address@hidden (Ronnie Sahlberg)
> 
> 
> This verified that the service is actually operational and is much
> more reliable than TCP-KEEPALIVES.
> This is the proper way to monitor that the iscsi target is alive.
> 
> We should as a later patch add the ability to configure this via the
> qemu config file instead of using hardcoded values.
> 
> 
> regards
> ronnie sahlberg
> 
> 
> On Mon, Dec 3, 2012 at 11:34 AM, Peter Lieven <address@hidden> wrote:
>> This patch will send NOP-Out PDUs every 5 seconds to the iSCSI target.
>> If a consecutive number of NOP-In replies fail a reconnect is initiated.
>> iSCSI NOPs help to ensure that the connection to the target is still 
>> operational.
>> This should not, but in reality may be the case even if the TCP connection 
>> is still
>> alive if there are bugs in either the target or the initiator implementation.
>> 
>> Reported-by: Ronnie Sahlberg <address@hidden>
>> Signed-off-by: Peter Lieven <address@hidden>
>> ---
>> block/iscsi.c |   43 +++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 43 insertions(+)
>> 
>> diff --git a/block/iscsi.c b/block/iscsi.c
>> index d0b1a10..fab4c8b 100644
>> --- a/block/iscsi.c
>> +++ b/block/iscsi.c
>> @@ -47,6 +47,9 @@ typedef struct IscsiLun {
>>     int block_size;
>>     uint64_t num_blocks;
>>     int events;
>> +
>> +    QEMUTimer *nop_timer;
>> +    int nops_in_flight;
>> } IscsiLun;
>> 
>> typedef struct IscsiAIOCB {
>> @@ -72,6 +75,9 @@ struct IscsiTask {
>>     int complete;
>> };
>> 
>> +#define NOP_INTERVAL 5000
>> +#define MAX_NOP_FAILURES 3
>> +
>> static void
>> iscsi_bh_cb(void *p)
>> {
>> @@ -925,6 +931,35 @@ static char *parse_initiator_name(const char *target)
>>     }
>> }
>> 
>> +static void iscsi_nop_cb(struct iscsi_context *iscsi, int status, void 
>> *command_data, void *private_data)
>> +{
>> +    IscsiLun *iscsilun = private_data;
>> +
>> +    if (iscsilun) {
>> +        iscsilun->nops_in_flight = 0;
>> +    }
>> +}
>> +
>> +static void iscsi_nop_timed_event(void *opaque)
>> +{
>> +    IscsiLun *iscsilun = opaque;
>> +
>> +    if (iscsilun->nops_in_flight > MAX_NOP_FAILURES) {
>> +        error_report("iSCSI: NOP timeout. Reconnecting...");
>> +        iscsi_reconnect(iscsilun->iscsi);
>> +        iscsilun->nops_in_flight = 0;
>> +    }
>> +
>> +    if (iscsi_nop_out_async(iscsilun->iscsi, iscsi_nop_cb, NULL, 0, 
>> iscsilun) != 0) {
>> +        error_report("iSCSI: failed to sent NOP-Out. Disabling NOP 
>> messages.");
>> +        return;
>> +    }
>> +
>> +    qemu_mod_timer(iscsilun->nop_timer, qemu_get_clock_ms(rt_clock) + 
>> NOP_INTERVAL);
>> +    iscsi_set_events(iscsilun);
>> +    iscsilun->nops_in_flight++;
>> +}
>> +
>> /*
>>  * We support iscsi url's on the form
>>  * iscsi://[<username>%<password>@]<host>[:<port>]/<targetname>/<lun>
>> @@ -1036,6 +1071,10 @@ static int iscsi_open(BlockDriverState *bs, const 
>> char *filename, int flags)
>> 
>>     ret = 0;
>> 
>> +    /* Set up a timer for sending out iSCSI NOPs */
>> +    iscsilun->nop_timer = qemu_new_timer_ms(rt_clock, 
>> iscsi_nop_timed_event, iscsilun);
>> +    qemu_mod_timer(iscsilun->nop_timer, qemu_get_clock_ms(rt_clock) + 
>> NOP_INTERVAL);
>> +
>> out:
>>     if (initiator_name != NULL) {
>>         g_free(initiator_name);
>> @@ -1058,6 +1097,10 @@ static void iscsi_close(BlockDriverState *bs)
>>     IscsiLun *iscsilun = bs->opaque;
>>     struct iscsi_context *iscsi = iscsilun->iscsi;
>> 
>> +    if (iscsilun->nop_timer) {
>> +        qemu_del_timer(iscsilun->nop_timer);
>> +        qemu_free_timer(iscsilun->nop_timer);
>> +    }
>>     qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), NULL, NULL, NULL, NULL);
>>     iscsi_destroy_context(iscsi);
>>     memset(iscsilun, 0, sizeof(IscsiLun));
>> --
>> 1.7.9.5
>> 
>>