Re: [pnfs] [PATCH 2/5] pnfsd: fix pnfs_export_operations layoutget valid errors

Andy Adamson
2010-02-08T20:41:46Z

On Feb 7, 2010, at 4:05 AM, Benny Halevy wrote:

> On Feb. 05, 2010, 19:10 +0200, andros@netapp.com wrote:
>> From: Andy Adamson <andros@netapp.com>
>>
>> Signed-off-by: Andy Adamson <andros@netapp.com>
>> ---
>> fs/nfsd/nfs4pnfsd.c | 6 ++----
>> 1 files changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
>> index 816e2f0..3951e02 100644
>> --- a/fs/nfsd/nfs4pnfsd.c
>> +++ b/fs/nfsd/nfs4pnfsd.c
>> @@ -870,6 +870,7 @@ nfs4_pnfs_get_layout(struct
>> nfsd4_pnfs_layoutget *lgp,
>> if (status) {
>> switch (status) {
>> case -ETOOSMALL:
>> + case -E2BIG:
>
> Should we allow the filesystem to return nfs errors?
> Or even require it to do so?
>
> This can be done by adding cases for the
> valid error values for LAYOUTGET in this switch statement.

OK. From re-reading all the past mail and the comments on this latest
patch set:

1) We want a limited number of well documented error returns -
documented as part of the filesystem API not just the spec.

2) Start with just the errors that we know we need for gfs2 and exofs,
and expand the list later as necessary.

3) Allow only nfserr_xxxx errors.

Does this address the comments? Do you want more documentation, if so,
where?

if (status) {
+ /*
+ * The allowable error codes for the layout_get pNFS
export
+ * operations vector function can be expanded as needed
+ * to include other errors defined for the LAYOUTGET
pNFS
+ * operation.
+ */
+ case nfserr_badiomode:
+ case nfserr_badlayout:
+ case nfserr_layouttrylater:
+ case nfserr_layoutunavailable:
+ case nfserr_toosmall:
break;
default:
+ BUG();
}
goto out_freelayout;

-->Andy

>
> Benny
>
>> status = nfserr_toosmall;
>> break;
>> case -ENOMEM:
>> @@ -878,10 +879,7 @@ nfs4_pnfs_get_layout(struct
>> nfsd4_pnfs_layoutget *lgp,
>
> case nfserr_layouttrylater:
>
>> status = nfserr_layouttrylater;
>> break;
>> case -ENOENT:
>> - status = nfserr_badlayout;
>> - break;
>> - case -E2BIG:
>> - status = nfserr_toosmall;
>> + status = nfserr_stale;
>> break;
>> default:
>> status = nfserr_layoutunavailable;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html