[PATCH] fabrics: ensure zero kato for non-persistent controllers

[George, Martin]

On Tue, 2021-02-23 at 06:06 +0000, Chaitanya Kulkarni wrote:
> On 2/22/21 21:47, George, Martin wrote:
> > Ping?
> > 
> > On Thu, 2021-02-18 at 15:24 +0530, Martin George wrote:
> > > Along with ensuring a non-zero kato is passed to a
> > > persistent discovery controller, also ensure a zero
> > > kato is passed to a non-persistent discovery controller.
> > > 
> > > Signed-off-by: Martin George <marting at netapp.com>
> > > ---
> > >  fabrics.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/fabrics.c b/fabrics.c
> > > index 1880a5d..ed15e08 100644
> > > --- a/fabrics.c
> > > +++ b/fabrics.c
> > > @@ -1399,6 +1399,8 @@ static int discover_from_conf_file(const
> > > char
> > > *desc, char *argstr,
> > > 
> > >                 if (cfg.persistent && !cfg.keep_alive_tmo)
> > >                         cfg.keep_alive_tmo = NVMF_DEF_DISC_TMO;
> > > +               else if (!cfg.persistent && (cfg.keep_alive_tmo >
> > > 0))
> > > +                       cfg.keep_alive_tmo = 0;
> > > 
> > >                 if (traddr_is_hostname(&cfg)) {
> > >                         ret = hostname2traddr(&cfg);
> > > @@ -1474,6 +1476,8 @@ int fabrics_discover(const char *desc, int
> > > argc, char **argv, bool connect)
> > >         } else {
> > >                 if (cfg.persistent && !cfg.keep_alive_tmo)
> > >                         cfg.keep_alive_tmo = NVMF_DEF_DISC_TMO;
> > > +               else if (!cfg.persistent && (cfg.keep_alive_tmo >
> > > 0))
> > > +                       cfg.keep_alive_tmo = 0;
> > > 
> > >                 if (traddr_is_hostname(&cfg)) {
> > >                         ret = hostname2traddr(&cfg);
> Is that if else if ladder code is repeated ? if so can you please add
> a
> helper ?
> unless there is a reason for keeping the duplicate code.

Ok. Will write a helper function for this and resend the patch.

> Also can we remove () for (cfg.keep_alive_tmo > 0) condition ?

I'd prefer keeping the parentheses here to be on the safer side, unless
there is a strong argument against it.

-Martin