Just nits:
On Wed, Feb 01, 2012 at 10:44:08AM -0500, Jeff Layton wrote:
> +static struct nfsd4_client_tracking_ops nfsd4_legacy_tracking_ops = {
> + .init = nfsd4_load_reboot_recovery_data,
> + .exit = nfsd4_shutdown_recdir,
> + .create = nfsd4_create_clid_dir,
> + .remove = nfsd4_remove_clid_dir,
> + .check = nfsd4_check_legacy_client,
> + .grace_done = nfsd4_recdir_purge_old,
> +};
> +
> +int
> +nfsd4_client_tracking_init(void)
> +{
> + int status;
> +
> + client_tracking_ops = &nfsd4_legacy_tracking_ops;
> +
> + if (!client_tracking_ops->init)
> + return 0;
Is that check necessary?
> +
> + status = client_tracking_ops->init();
> + if (status) {
> + printk(KERN_WARNING "NFSD: Unable to initialize client "
> + "recovery tracking! (%d)\n", status);
> + client_tracking_ops = NULL;
> + }
> + return status;
> +}
> +
> +void
> +nfsd4_client_tracking_exit(void)
> +{
> + if (client_tracking_ops && client_tracking_ops->exit)
In general I don't see the point of handling the case where one of the
ops is NULL. If there was some implementation that really wanted that
we could get the same effect by defining a default no-op implementation
they could use, but I don't think there actually is?
> +int
> +nfsd4_client_record_check(struct nfs4_client *clp)
> +{
> + if (client_tracking_ops && client_tracking_ops->check)
> + return client_tracking_ops->check(clp);
> +
> + return -EOPNOTSUPP;
> +}
> +
> +void
> +nfsd4_grace_done(time_t boot_time)
That name's a bit generic--but I don't have a better suggestion.
(nfsd4_record_grace_done()?)
Looks basically fine, though.
--b.
--
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
