On 2010-09-11 01:37, Trond Myklebust wrote:
> 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@xxxxxxxxxx> 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.
>
Later in the game we introduce the layout driver policy ops.
That said, they could be added to the same vector as the io ops.
>> 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.
Agreed.
I think we should just have
struct pnfs_layoutdriver_type {
struct list_head pnfs_tblid;
const u32 id;
const char *name;
const struct module *owner;
const struct layoutdriver_operations *ld_ops;
};
Benny
>
> Cheers
> Trond
> --
> 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
--
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
