Re: [PATCH 08/13] RFC: pnfs: filelayout: introduce minimal file layout driver

[Trond Myklebust <Trond.Myklebust@netapp.com>]

On Fri, 2010-09-10 at 14:11 -0700, Fred Isaman wrote:
> On Fri, Sep 10, 2010 at 12:31 PM, Trond Myklebust
> <Trond.Myklebust@netapp.com> wrote:
> > On Thu, 2010-09-02 at 14:00 -0400, Fred Isaman wrote:
> OK
> 
> >> +     .initialize_mountpoint   = filelayout_initialize_mountpoint,
> >> +     .uninitialize_mountpoint = filelayout_uninitialize_mountpoint,
> >> +};
> >> +
> >> +
> >> +struct pnfs_layoutdriver_type filelayout_type = {
> >
> > Ditto.
> 
> This includes a list_head field which is set by the generic layer.
> 
> >
> >> +     .id = LAYOUT_NFSV4_1_FILES,
> >> +     .name = "LAYOUT_NFSV4_1_FILES",
> >> +     .ld_io_ops = &filelayout_io_operations,
> >
> > Why do we need a separate 'struct layoutdriver_io_operations'? Any
> > reason those can't just be embedded in struct pnfs_layoutdriver_type?
> 
> I believe this decision was primarily aesthetics.  However, keeping
> the static io_ops seperate from the variable list_head seems like a
> good idea.

I dunno. They are in a 1-1 correspondence, so I'm not sure I see the
need for a separation.

> Perhaps having a driver structure that includes the io_ops and static
> portions of pnfs_layoutdriver_type, with the generic layer allocating
> a wrapper structure that is basically:
> struct {
>     struct list_head list;
>     struct pnfs_layoutdriver_type *driver_info;

      Should be const...

      struct module *owner = THIS_MODULE;

> }

...although the struct module could probably indeed be part of
pnfs_layoutdriver_type too.

Cheers
  Trond