On Thu, 2 Feb 2012 17:45:41 -0500
"J. Bruce Fields" <bfields@xxxxxxxxxxxx> wrote:
> 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?
>
No, see below...
> > +
> > + 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?
>
In an earlier iteration of this patchset, some of these fields were
NULL pointers, but in the current version they're all populated. I
can remove those checks if you'd prefer.
> > +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()?)
>
Ok, that sounds reasonable. I'll rename it when I respin this patch in
the next week or so.
Thanks,
--
Jeff Layton <jlayton@xxxxxxxxxx>
--
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
