Re: pNFS client structure and function rename suggestions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jul 28, 2010 at 7:09 AM, Boaz Harrosh <bharrosh@xxxxxxxxxxx> wrote:
> On 07/27/2010 06:37 PM, Andy Adamson wrote:
>> *********
>> include/linux/nfs_fs.h
>>
>> struct pnfs_layout_type => pnfs_layout
>
> I must disagree. Sorry.
>
> The layout4 type in the standard is defined as:
> struct layout4 {
>           offset4                 lo_offset;
>           length4                 lo_length;
>           layoutiomode4           lo_iomode;
>           layout_content4         lo_content;
> };
>
> Which is what we use as layout_segment. All the "LAYOUT" operations
> operate on an embedded concept of the above layout (seg) and or
> the LAYOUTGET actually returns a layout4.
>
> Calling something else pnfs_layout, will totally confuse. (And it seems
> it has already confused the best of us ;-) )
>
> The above, former struct pnfs_layout_type, is a collection of "layout"s
> and governing state, plus general LD based IO management stuff.
>
> For me it is just the "nfs inode extension for pnfs", embedded inside the
> "LD inode extension"
>
> Call it what you like. But the concept of "layout" was invented by the
> rfc5661. Please don't overload it to mean something else.
>
>> fields:
>>     refcount => lo_refcount
>>     segs => lo_segs
>>     roc_iomode => lo_roc_iomode
>>     seqlock => lo_seqlock
>>     stateid => lo_stateid
>>     pnfs_layout_state => lo_state
>>     pnfs_write_begin_pos => lo_write_begin
>>     pnfs_write_end_pos => lo_write_end
>>
>
> Maybe just call it:
> struct nfs_inode {
>        ...
>        struct pnfs_info *pnfs;
>
> If the "layout" name is dropped then by naming convention the "lo_" is not
> appropriate as well.
>
>> *********
>> include/linux/pnfs_xdr.h
>>
>> NOTE: consider removing and adding to include/linux/nfs_xdr.h where the other
>> nfsv4.1 xdr structs are declared.
>>
>> struct nfs4_pnfs_layout => remove, inline in nfs4_layoutget_args.
>>
>> struct nfs4_pnfs_layout_segment => pnfs_layout_range
>
> Isn't this a struct layout4 above?

No, this is probably the most confusingly named structure of them all,
and one I would strongly urge be changed along the line of Andy's
suggestion.

Fred

> (And what an ugly reorder from the STD's fields, with these double
> miss alignment inside the nfs4_pnfs_layoutget_arg)
>
> I don't see why we have to invent new names (and structures) that translate
> to yet other names and concepts in the standard. (And a dictionary that
> translates one to the other). Why can't we just use the STDs names and structures
> directly? Is there not to many already?
>
>>
>> struct nfs4_pnfs_layoutget_arg => nfs4_layoutget_args
>> fields:
>>     lseg => range
>>     layout: remove. inline with:
>>       add: layout_len
>>         add: layout_buf
>>
>> struct nfs4_pnfs_layoutget_res => nfs4_layoutget_res
>> fields:
>>     lseg => range
>>
>
> All these lsegs above and below are layouts
>
>> struct nfs4_pnfs_layoutget => nfs4_layoutget
>>
>
> OMG, yes!
>
> Boaz
>
>> struct pnfs_layoutcommit_arg => nfs4_layoutcommit_args
>> fields:
>>     lseg => range
>>
>> struct pnfs_layoutcommit_res => nfs4_layoutcommit_res
>>
>> struct pnfs_layoutcommit_data => nfs4_layoutcommit_data
>>
>> struct nfs4_pnfs_layoutreturn_arg => nfs4_layoutreturn_args
>> fields:
>>     lseg => range
>>
>> struct nfs4_pnfs_layoutreturn_res => nfs4_layoutreturn_res
>>
>> struct nfs4_pnfs_layoutreturn => nfs4_layoutreturn
>>
>> struct nfs4_pnfs_getdeviceinfo_arg => nfs4_getdeviceinfo_args
>>
>> struct nfs4_pnfs_getdeviceinfo_res => nfs4_getdeviceinfo_res
>>
>> *********
>> include/linux/nfs4.h
>>
>> NFSPROC4_CLNT_PNFS_LAYOUTGET => NFSPROC4_CLNT_LAYOUTGET
>> NFSPROC4_CLNT_PNFS_LAYOUTCOMMIT => NFSPROC4_CLNT_LAYOUTCOMMIT
>> NFSPROC4_CLNT_PNFS_LAYOUTRETURN => NFSPROC4_CLNT_LAYOUTRETURN
>> NFSPROC4_CLNT_PNFS_GETDEVICEINFO => NFSPROC4_CLNT_GETDEVICEINFO
>>
>> *********
>>
>> fs/nfs/nfs4proc.c
>>
>> nfs4_pnfs_layoutget_prepare => nfs4_layoutget_prepare
>> nfs4_pnfs_layoutget_done => nfs4_layoutget_done
>> nfs4_pnfs_layoutget_release => nfs4_layoutget_release
>>
>> _pnfs4_proc_layoutget => _nfs4_proc_layoutget
>> pnfs4_proc_layoutget => nfs4_proc_layoutget
>>
>>
>> pnfs_layoutcommit_prepare => nfs4_layoutcommit_prepare
>> pnfs_layoutcommit_done => nfs4_layoutcommit_done
>> pnfs_layoutcommit_release => nfs4_layoutcommit_release
>>
>> _pnfs4_proc_layoutcommit => _nfs4_proc_layoutcommit
>> pnfs4_proc_layoutcommit => nfs4_proc_layoutcommit
>>
>> nfs4_pnfs_layoutreturn_prepare => nfs4_layoutreturn_prepare
>> nfs4_pnfs_layoutreturn_done => nfs4_layoutreturn_done
>> nfs4_pnfs_layoutreturn_release => nfs4_layoutreturn_release
>>
>> _pnfs4_proc_layoutreturn => _nfs4_proc_layoutreturn
>> pnfs4_proc_layoutreturn => nfs4_proc_layoutreturn
>>
>> nfs4_pnfs_getdeviceinfo => nfs4_proc_getdeviceinfo
>>
> --
> 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


[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux