On Mon, Dec 01, 2008 at 01:57:58PM -0500, Chuck Lever wrote:
> Clean up: A few minor clean-ups for nsm_monitor():
All looks fine, thanks, however:
>
> o Make sure to return an error if the SM_MON call result is not zero.
I'd prefer to have a change in behavior split out into a separate patch
from pure cleanup.
--b.
>
> o Remove the BUG_ON() -- the code will die anyway if nsm is NULL.
>
> o Use nsm->sm_name instead of host->h_name to be consistent with
> other functions in fs/lockd/mon.c.
>
> o Collect the public declaration of nsm_monitor() in lockd.h with
> other NSM public function declarations (eg. nsm_release).
>
> o Add documenting comments.
>
> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> ---
>
> fs/lockd/mon.c | 29 ++++++++++++++++++-----------
> include/linux/lockd/lockd.h | 4 ++++
> include/linux/lockd/sm_inter.h | 1 -
> 3 files changed, 22 insertions(+), 12 deletions(-)
>
> diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c
> index a606fbb..78d5f59 100644
> --- a/fs/lockd/mon.c
> +++ b/fs/lockd/mon.c
> @@ -69,18 +69,24 @@ nsm_mon_unmon(struct nsm_handle *nsm, u32 proc, struct nsm_res *res)
> return status;
> }
>
> -/*
> - * Set up monitoring of a remote host
> +/**
> + * nsm_monitor - Notify a peer in case we reboot
> + * @host: pointer to nlm_host of peer to notify
> + *
> + * If this peer is not already monitored, this function sends an
> + * upcall to the local rpc.statd to record the name/address of
> + * the peer to notify in case we reboot.
> + *
> + * Returns zero if the peer is monitored by the local rpc.statd;
> + * otherwise a negative errno value is returned.
> */
> -int
> -nsm_monitor(struct nlm_host *host)
> +int nsm_monitor(const struct nlm_host *host)
> {
> struct nsm_handle *nsm = host->h_nsmhandle;
> - struct nsm_res res;
> - int status;
> + struct nsm_res res;
> + int status;
>
> - dprintk("lockd: nsm_monitor(%s)\n", host->h_name);
> - BUG_ON(nsm == NULL);
> + dprintk("lockd: nsm_monitor(%s)\n", nsm->sm_name);
>
> if (nsm->sm_monitored)
> return 0;
> @@ -92,9 +98,10 @@ nsm_monitor(struct nlm_host *host)
> nsm->sm_mon_name = nsm_use_hostnames ? nsm->sm_name : nsm->sm_addrbuf;
>
> status = nsm_mon_unmon(nsm, SM_MON, &res);
> -
> - if (status < 0 || res.status != 0)
> - printk(KERN_NOTICE "lockd: cannot monitor %s\n", host->h_name);
> + if (res.status != 0)
> + status = -EIO;
> + if (status < 0)
> + printk(KERN_NOTICE "lockd: cannot monitor %s\n", nsm->sm_name);
> else
> nsm->sm_monitored = 1;
> return status;
> diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
> index de9ea7b..4ca6f39 100644
> --- a/include/linux/lockd/lockd.h
> +++ b/include/linux/lockd/lockd.h
> @@ -233,6 +233,10 @@ extern void nlm_host_rebooted(const struct sockaddr_in *, const char *,
> unsigned int, u32);
> void nsm_release(struct nsm_handle *);
>
> +/*
> + * Host monitoring
> + */
> +int nsm_monitor(const struct nlm_host *host);
>
> /*
> * This is used in garbage collection and resource reclaim
> diff --git a/include/linux/lockd/sm_inter.h b/include/linux/lockd/sm_inter.h
> index 5a5448b..546b610 100644
> --- a/include/linux/lockd/sm_inter.h
> +++ b/include/linux/lockd/sm_inter.h
> @@ -41,7 +41,6 @@ struct nsm_res {
> u32 state;
> };
>
> -int nsm_monitor(struct nlm_host *);
> int nsm_unmonitor(struct nlm_host *);
> extern int nsm_local_state;
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
