Subject: [nfsv4] draft-ietf-nfsv4-minorversion1-01.txt: pNFS ops: redundant pnfs_layouttype4 in GETDEVICELIST4resok

From: "William A.(Andy) Adamson" <[email-masked]>
Date: Fri, 13 Jan 2006 11:54:38 -0500

the client specifies the pnfs_layouttype4 it wants devices for in the 
GETDEVICELIST4args.

    struct GETDEVICELIST4args {
             /* CURRENT_FH: file */
             pnfs_layouttype4                layout_type;
             count4                          maxcount;
             nfs_cookie4                     cookie;
             verifier4                       cookieverf;
     };

in the result, there is an opaque pnfs_devlist_item4; the list of devices for 
the
layout type specified in the GETDEVICELIST4args.

   RESULT

     struct GETDEVICELIST4resok {
             nfs_cookie4                     cookie;
             verifier4                       cookieverf;
             pnfs_devlist_item4              device_addrs<>;
     };

    struct pnfs_devlist_item4 {
            pnfs_deviceid4          id;
            pnfs_deviceaddr4        addr;
     };

but, the pnfs_deviceaddr4 member of pnfs_devlist_item4 also has a 
pnfs_layouttype4.

    struct pnfs_deviceaddr4 {
              pnfs_layouttype4 type;
              opaque           device_addr<>;
     };

as far as i can tell, this is redundant - its value will never be different 
from the pnfs_layouttype4 requested by the client in GETDEVICELIST4args.

so, i propose we change the declaration of struct pnfs_devlist_item4 which is 
only used in GETDEVICELIST4resok as follows:

 struct pnfs_devlist_item4 {
            pnfs_deviceid4          id;
            opaque                  device_addr<>; /* of pnfs_layouttype4 
specified in GETDEVICELIST4args */
     };

removing the redundant pnfs_layouttype4.

i also propose we change the text of GETDEVICELIST Description from

  This operation returns an array of items (pnfs_devlist_item4) that
   establish the association between the short pnfs_deviceid4 and the
   addressing information for that device, for a particular layout type.

to 
  
   This operation returns an array of items (pnfs_devlist_item4) that
   establish the association between the short pnfs_deviceid4 and the
   addressing information for that device, for the layout type
   specified in GETDEVICELIST4args.

-->Andy

_______________________________________________
nfsv4 mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/nfsv4

---

From: Brent Welch <[email-masked]>
Date: Mon, 16 Jan 2006 11:13:42 -0800

I'm not sure I feel that strongly about this.  One advantage
of the redundancy is that there isn't yet another type for the
return, and the opaque device addressing information is always
closely associated with its type identifier.  It seems like the
server is going to want to keep all of
(short device ID, type information, opaque blob of info)
fairly close together, and this is exactly what the 
pnfs_devlist_item4 is.

In otherwords, the spec has a pattern that where every this
opaque blob appears, it is always next to a type identifier.
While that is redundant in this case, I still like it.

>>>"William A.(Andy) Adamson" said:
 > 
 > 
 > in the result, there is an opaque pnfs_devlist_item4; the list of 
devices fo
     r 
 > so, i propose we change the declaration of struct pnfs_devlist_item4 
which i
     s 

--
Brent Welch
Software Architect, Panasas Inc
Accelerating Time to Results(tm) with Clustered Storage

www.panasas.com
[email-masked]

---

From: Garth Goodson <[email-masked]>
Date: Mon, 16 Jan 2006 12:14:41 -0800

I tend to agree with Brent.  I like having opaque blobs be somewhat 
self-describing, just another sanity check.

-Garth

---

From: Dean Hildebrand <[email-masked]>
Date: Mon, 16 Jan 2006 15:29:02 -0500

I don't feel strongly either, and Brent is right that there is a pattern 
in the spec where every time an
opaque blob appears, it is always next to a type identifier. But the 
redundancy still seems a little silly, since the server is forced to 
either return the type requested by the client (in layoutget and 
getdevicelist) or return an error.  As an example in the NFSv4 spec, the 
READ call doesn't return the filehandle for which the opaque result 
applies, the opaque result must be from the file tied to the filehandle 
used to read the data.  I'm sure there are other examples....

Dean

Garth Goodson wrote:

>>  >  >  > the client specifies the pnfs_layouttype4 it wants devices 
>> for in the  > GETDEVICELIST4args.
>>  >  > in the result, there is an opaque pnfs_devlist_item4; the list 
>> of devices fo
>>      r  > the
>>  >  > but, the pnfs_deviceaddr4 member of pnfs_devlist_item4 also has 
>> a  > pnfs_layouttype4.
>>  >  > so, i propose we change the declaration of struct 
>> pnfs_devlist_item4 which i
>>      s  > only used in GETDEVICELIST4resok as follows:
>>  >  > to  >    >    This operation returns an array of items 
>> (pnfs_devlist_item4) that

---

From: "J. Bruce Fields" <[email-masked]>
Date: Mon, 16 Jan 2006 15:50:17 -0500

On Mon, Jan 16, 2006 at 12:14:41PM -0800, Garth Goodson wrote:
> I like having opaque blobs be somewhat self-describing, just another
> sanity check.

xdr isn't self-describing, and this doesn't function as a literal
"sanity check"--there aren't server failures that this would allow us to
detect that we couldn't otherwise.

Rather than imposing a constraint on some fields in a packet, and thus
making it possible to ignore the constraint, it seems safest to make it
impossible to break the constraint in the first place.  (OK, the chance
of someone thinking it's reasonable to throw in other types of
deviceid's may be small.  But you can see the temptation to assume those
fields were meant to be used for *something*.)

>opaque blob appears, it is always next to a type identifier.

I don't see this as much of an argument either way.

--b.

---

From: "William A.(Andy) Adamson" <[email-masked]>
Date: Mon, 16 Jan 2006 15:53:00 -0500

> I'm not sure I feel that strongly about this.  

ok, i don't really care either. it just seems silly.

> One advantage
> closely associated with its type identifier.

i don't see the advantage. it seems like most pNFS clients will hand off the 
whole list of pnfs_delist_item4's to the layout driver of type GETDEVICELIST4a 
layout_type and let the driver parse the list. if a blob in the list can't be 
decoded, (for whatever reason) there's an error; i don't see and advantage for 
including the per list element type.

>  It seems like the

it seems like the server will store a whole bunch of information per layout 
type, and doesn't need to waste memory on repeating the layout type with each 
instance of a piece of info. if i have 100 <short device ID, type information, 
opaque blob of info> tuples, i would simply put the 100 <short device ID, 
opaque blob of info> in a list under a struct with the common layout type.

> While that is redundant in this case, I still like it.

it seems silly to me. the list has a type which is closely associated with 
each opaque blob, why repeat it? but, i can put it on the wire even though i 
think it's useless.

>  >    addressing information for that device, for a particular layout type.

the wording 'particular layout type' gives no indication that this is 
redundant info. i think there should be some text to indicate that any 
layouttype other than the GETDEVICELIST4a layout_type is an error.

-->Andy

---

From: Brent Welch <[email-masked]>
Date: Mon, 16 Jan 2006 14:40:14 -0800

As I said, I don't feel strongly about this - it appears that others do.
Especially in the listing context where everything in the list will have 
the
same time, it is clearly redundant.  I would be fine with the change
that Andy proposes.

 > whole list of pnfs_delist_item4's to the layout driver of type 
GETDEVICELIST
     4a 
 > layout_type and let the driver parse the list. if a blob in the list 
can't b
     e 
 > decoded, (for whatever reason) there's an error; i don't see and 
advantage f
     or 
 > type, and doesn't need to waste memory on repeating the layout type with 
eac
     h 
 > instance of a piece of info. if i have 100 <short device ID, type 
informatio
     n, 
      
 > >  >    addressing information for that device, for a particular layout 
type
     .
 > >  > from the pnfs_layouttype4 requested by the client in 
GETDEVICELIST4args
     .
 > >  > so, i propose we change the declaration of struct 
pnfs_devlist_item4
 > >  >    addressing information for that device, for a particular layout 
type
     .

www.panasas.com
[email-masked]