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

[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
>
>