On 2010-09-13 15:01, Fred Isaman wrote:
> On Mon, Sep 13, 2010 at 3:32 AM, Benny Halevy <bhalevy@xxxxxxxxxxx> wrote:
>> 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.
>
>
> Yes, I think they should be merged when we get to that stage.
>
>
>>
>>>> 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
>>
>
> I'll point out that what I took from the above conversation, was that
> the fields id, name, and possibly owner should be placed inside struct
> layoutdriver_operations (which should probably be renamed slightly).
>
>
just keep it simple please :)
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
>>
--
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
