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

Trond Myklebust
2010-09-10T22:37:23Z

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