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).
>>
>> 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
