Re: [nfsv4] [pnfs] [PATCH 1/1] pnfsd: DLM file layout only support read iomode layouts

["William A. (Andy) Adamson" <[e-mail]>]

On Wed, Dec 16, 2009 at 7:18 AM, Benny Halevy <[e-mail]> wrote:
> Bruce, I agree that the spec isn't clear about what to return
> so I'm adding the ietf list to this thread.
>
> In summary, we have a server that supports read-only layouts but
> not read-write.  What should it return to a LAYOUTGET with LAYOUTIOMODE4_RW?
>
> We see two possibilities that make sense: NFS4ERR_BADIOMODE and
> NFS4ERR_LAYOUTUNAVAILABLE.
>
> 15.1.10.1.  NFS4ERR_BADIOMODE (Error Code 10049)
>
>   An invalid or inappropriate layout iomode was specified.

The spec is fine. Inappropriate means the server does not want to
return a layout for that file with the requested iomode at this time.
It doesn't necessarily mean that the server won't return a layout with
that same iomode for a different file, or return a layout for the file
with the requested iomode at a later date.  It's totally up to the
server what is appropriate.
For what ever reason, the client has no choice but to try a different
iomode and/or
shoulder on via the MDS,

>
> 15.1.10.4.  NFS4ERR_LAYOUTUNAVAILABLE (Error Code 10059)
>
>   Returned when layouts are not available for the current file system
>   or the particular specified file.
>
> NFS4ERR_BADIOMODE seems to make more sense since layouts are available,
> just not for the requested iomode. However its definition indicates that
> the iomode in the client request is "invalid or inappropriate".
> It is not clear what "inappropriate" means in this context.

What difference does 'what inappropriate means' make to the client?
None! It's up to the server.

> I guess that the original intent was for clients who e.g. send a LAYOUTGET
> with LAYOUTIOMODE4_ANY which is a valid iomode but inappropriate for LAYOUTGET.

The client specifies the iomode so that the server can decide whether
it is appropriate to return a layout or not. This is why
LAYOUTIOMODE4_ANY is not allowed on LAYOUTGET.

>
> To make the spec clearer we could say:
>
> 15.1.10.1.  NFS4ERR_BADIOMODE (Error Code 10049)
>
>   An invalid, inappropriate, or unsupported layout iomode was specified.

This is redundant. One meaning of inappropriate is unsupported.
Another reason is 'sorry, I'm re-striping, and this file has read-only
layouts now'. Another reason is ' it's Tuesday, and I don't hand out
layouts with iomode blah for file bleep on Tuesday. It's the server's
choice so the client just has to get over it and move on!

-->Andy


>
> 18.43.  Operation 50: LAYOUTGET - Get Layout Information
> 18.43.3.  DESCRIPTION
> iomode
> ...
> +     If the metadata server does not support read/write layouts and
> +     the value of the lo_iomode is LAYOUTIOMODE4_RW it MUST return
> +     NFS4ERR_BADIOMODE.
>
>
> Benny
>
> On Dec. 16, 2009, 0:41 +0200, "J. Bruce Fields" <[e-mail]> wrote:
>> On Tue, Dec 15, 2009 at 03:03:54PM -0500, [e-mail] wrote:
>>> From: Andy Adamson <[e-mail]>
>>>
>>> In a DLM cluster, writing to a node other than the node where the open call
>>> occurred will have performance implications due to meta data updates.
>>>
>>> Return only LAYOUTIOMODE4_READ layouts so that writes go through the MDS
>>> LAYOUTGET with LAYOUTIOMODE4_RW iomodes will receive the NFS4ERR_BADIOMODE
>>> error.
>>
>> Hm.  The 4.1 draft does refer (in 13.8) to using LAYOUTIOMODE4_READ
>> layouts for the read-only data-server case, but doesn't really give a
>> clear error recommendation for it.  BADIOMODE says "An invalid or
>> inappropriate layout iomode was specified".  The client definitely isn't
>> making an *invalid* request.  I don't know what inappropriate means.
>>
>> LAYOUTUNAVAILABLE might be better, though it doesn't mention this
>> particular case: "Returned when layouts are not available for the
>> current file system or the particular specified file."  So a client
>> might reasonably take that to mean that it shouldn't ever ask for that
>> file again.
>>
>> I guess BADIOMODE wins, but the spec could be clearer.
>>
>> --b.
>>
>>> Pass the NFS4ERR_BADIOMODE error to the client - Don't map
>>> pnfs_export_operation:layout_get() NFS4 errors.
>>>
>>> Signed-off-by: Andy Adamson <[e-mail]>
>>> ---
>>>  fs/nfsd/nfs4pnfsd.c   |    2 +-
>>>  fs/nfsd/nfs4pnfsdlm.c |    4 ++++
>>>  2 files changed, 5 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
>>> index aa7abad..658a343 100644
>>> --- a/fs/nfsd/nfs4pnfsd.c
>>> +++ b/fs/nfsd/nfs4pnfsd.c
>>> @@ -541,7 +541,7 @@ nfs4_pnfs_get_layout(struct nfsd4_pnfs_layoutget *lgp,
>>>              __func__, status, res.lg_seg.iomode,
>>>              res.lg_seg.offset, res.lg_seg.length);
>>>
>>> -    if (status) {
>>> +    if (status && status >= -1000) {
>>>              switch (status) {
>>>              case -ETOOSMALL:
>>>                      status = nfserr_toosmall;
>>> diff --git a/fs/nfsd/nfs4pnfsdlm.c b/fs/nfsd/nfs4pnfsdlm.c
>>> index ed2e940..94ddba2 100644
>>> --- a/fs/nfsd/nfs4pnfsdlm.c
>>> +++ b/fs/nfsd/nfs4pnfsdlm.c
>>> @@ -329,6 +329,10 @@ static int nfsd4_pnfs_dlm_layoutget(struct inode *inode,
>>>
>>>      dprintk("%s: LAYOUT_GET\n", __func__);
>>>
>>> +    /* Due to DLM internode locking, only support read layouts */
>>> +    if res->lg_seg.iomode != IOMODE_READ)
>>> +            return nfserr_badiomode;
>>> +
>>>      index = dlm_ino_hash(inode);
>>>      dprintk("%s first stripe index %d i_ino %lu\n", __func__, index,
>>>              inode->i_ino);
>>> --
>>> 1.6.2.5
>>>
> _______________________________________________
> pNFS mailing list
> [e-mail]
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
>