7782 nfs: READDIR for referrals should conform to RFC 7530

Review Request #436 — Created April 21, 2017 and submitted

vgusev
illumos-gate
master
7782
4e81d4e...
general

7782 nfs: READDIR for referrals should conform to RFC 7530

This patch add returning error for whole READDIR operation if rdattr_error wasn't requested and:
- met referral
- any other error occured during getting attributes (it could be permission denied, for instance).

The patch intensionally misses first part of 8.3.2: returning success if fs_locations is set.

For more information look at Section 16.24.4:

"In some cases, the server may encounter an error while obtaining the
attributes for a directory entry..."

With Linux 4.8.5 kernel and vSphere-6.3. Both works fine.

For testing:

  1. Create nfs referral via nfsref: nfsref add somename 192.168.1.1:/data
  2. Do ls in directory that contains referral.

Linux client sends READDIR with rdattr_error attribute, but vSphere doesn't.

Before patch for vSphere client:
READDIR reply contains directory entry (that is referral) with empty attributes, i.e. w/o previously requested attributes.

After pathch for vSphere client:
READDIR reply with error NFS4ERR_MOVED, i.e. whole READDIR operation fails.

ADDITIONAL TESTING BY PYNFS WITH patched nfssrv (Revision/Changeset #10) and original nfssrv. As result: no regression errors. Additionally pynfs fslocations test reported better results (more PASS-ed tests).


PATCHED
./testserver.py 10.42.0.15:/codepool/data/test --rundep readdir


INIT st_setclientid.testValid : PASS
LOOKBLK st_lookup.testBlock : PASS
LOOKCHAR st_lookup.testChar : PASS
LOOKFIFO st_lookup.testFifo : PASS
LOOKFILE st_lookup.testFile : PASS
LOOKLINK st_lookup.testLink : PASS
LOOKSOCK st_lookup.testSocket : PASS
MKDIR st_create.testDir : PASS
MKFILE st_open.testOpen : PASS
MODE st_setattr.testMode : PASS
RDDR1 st_readdir.testEmptyDir : PASS
RDDR2 st_readdir.testFirst : PASS
RDDR3 st_readdir.testAttr : PASS
RDDR4 st_readdir.testSubsequent : PASS
RDDR5a st_readdir.testFhLink : PASS
RDDR5b st_readdir.testFhBlock : PASS
RDDR5c st_readdir.testFhChar : PASS
RDDR5f st_readdir.testFhFifo : PASS
RDDR5r st_readdir.testFhFile : PASS
RDDR5s st_readdir.testFhSocket : PASS
RDDR6 st_readdir.testNoFh : PASS
RDDR7 st_readdir.testMaxcountZero : PASS
RDDR8 st_readdir.testMaxcountSmall : FAILURE
READDIR of empty dir with maxcount=16 should return
NFS4_OK, instead got NFS4ERR_TOOSMALL
RDDR9 st_readdir.testWriteOnlyAttributes : PASS
RDDR10 st_readdir.testReservedCookies : FAILURE
READDIR with reserved cookie=1 should return
NFS4ERR_BAD_COOKIE, instead got NFS4_OK
RDDR11 st_readdir.testUnaccessibleDir : PASS
RDDR12 st_readdir.testUnaccessibleDirAttrs : PASS


Command line asked for 27 of 669 tests
Of those: 0 Skipped, 2 Failed, 0 Warned, 25 Passed


PATCHED
./testserver.py 10.42.0.15:/codepool/data/test --usespecial=/codepool/data/test/dir_00 --rundep fslocations


FSLOC1 st_fslocations.testReference : PASS
FSLOC2 st_fslocations.testReference2 : PASS
FSLOC3 st_fslocations.testReference3 : PASS
FSLOC4a st_fslocations.testAttr1a : FAILURE
GETATTR w/o FSLOC or RDATTR_ERROR should return
NFS4ERR_MOVED, instead got NFS4_OK
FSLOC4b st_fslocations.testAttr1b : PASS
FSLOC5a st_fslocations.testAttr2a : FAILURE
GETATTR w/o FSLOC but with RDATTR_ERROR should return
NFS4ERR_MOVED, instead got NFS4_OK
FSLOC5b st_fslocations.testAttr2b : PASS
FSLOC6a st_fslocations.testAttr3a : PASS
FSLOC6b st_fslocations.testAttr3b : PASS
FSLOC7a st_fslocations.testAttr4a : FAILURE
Expected 3 attrs returned, got 5
FSLOC7b st_fslocations.testAttr4b : PASS
FSLOC8a st_fslocations.testAttr5a : FAILURE
Expected 3 attrs returned, got 4
FSLOC8b st_fslocations.testAttr5b : PASS
GATT8 st_getattr.testFSLocations : PASS
LOOKFILE st_lookup.testFile : PASS


Command line asked for 15 of 669 tests
Of those: 0 Skipped, 4 Failed, 0 Warned, 11 Passed

----------------------------------------------------------------------------

ORIGINAL
./testserver.py 10.42.0.15:/codepool/data/test --usespecial=/codepool/data/test/dir_00 --rundep fslocations


FSLOC1 st_fslocations.testReference : PASS
FSLOC2 st_fslocations.testReference2 : PASS
FSLOC3 st_fslocations.testReference3 : PASS
FSLOC4a st_fslocations.testAttr1a : FAILURE
GETATTR w/o FSLOC or RDATTR_ERROR should return
NFS4ERR_MOVED, instead got NFS4_OK
FSLOC4b st_fslocations.testAttr1b : FAILURE
READDIR w/o FSLOC or RDATTR_ERROR should return
NFS4ERR_MOVED, instead got NFS4_OK
FSLOC5a st_fslocations.testAttr2a : FAILURE
GETATTR w/o FSLOC but with RDATTR_ERROR should return
NFS4ERR_MOVED, instead got NFS4_OK
FSLOC5b st_fslocations.testAttr2b : FAILURE
Expected 2 attrs returned for file dir_00, got 1
FSLOC6a st_fslocations.testAttr3a : PASS
FSLOC6b st_fslocations.testAttr3b : FAILURE
Expected 3 attrs returned for file dir_00, got 2
FSLOC7a st_fslocations.testAttr4a : FAILURE
Expected 3 attrs returned, got 5
FSLOC7b st_fslocations.testAttr4b : FAILURE
Expected 3 attrs returned for file dir_00, got 1
FSLOC8a st_fslocations.testAttr5a : FAILURE
Expected 3 attrs returned, got 4
FSLOC8b st_fslocations.testAttr5b : FAILURE
Expected 2 attrs returned for file dir_00, got 0
GATT8 st_getattr.testFSLocations : PASS
LOOKFILE st_lookup.testFile : PASS


Command line asked for 15 of 669 tests
Of those: 0 Skipped, 9 Failed, 0 Warned, 6 Passed


ORIGINAL

./testserver.py 10.42.0.15:/codepool/data/dir_test --rundep readdir


INIT st_setclientid.testValid : PASS
LOOKBLK st_lookup.testBlock : PASS
LOOKCHAR st_lookup.testChar : PASS
LOOKFIFO st_lookup.testFifo : PASS
LOOKFILE st_lookup.testFile : PASS
LOOKLINK st_lookup.testLink : PASS
LOOKSOCK st_lookup.testSocket : PASS
MKDIR st_create.testDir : PASS
MKFILE st_open.testOpen : PASS
MODE st_setattr.testMode : PASS
RDDR1 st_readdir.testEmptyDir : PASS
RDDR2 st_readdir.testFirst : PASS
RDDR3 st_readdir.testAttr : PASS
RDDR4 st_readdir.testSubsequent : PASS
RDDR5a st_readdir.testFhLink : PASS
RDDR5b st_readdir.testFhBlock : PASS
RDDR5c st_readdir.testFhChar : PASS
RDDR5f st_readdir.testFhFifo : PASS
RDDR5r st_readdir.testFhFile : PASS
RDDR5s st_readdir.testFhSocket : PASS
RDDR6 st_readdir.testNoFh : PASS
RDDR7 st_readdir.testMaxcountZero : PASS
RDDR8 st_readdir.testMaxcountSmall : FAILURE
READDIR of empty dir with maxcount=16 should return
NFS4_OK, instead got NFS4ERR_TOOSMALL
RDDR9 st_readdir.testWriteOnlyAttributes : PASS
RDDR10 st_readdir.testReservedCookies : FAILURE
READDIR with reserved cookie=1 should return
NFS4ERR_BAD_COOKIE, instead got NFS4_OK
RDDR11 st_readdir.testUnaccessibleDir : PASS
RDDR12 st_readdir.testUnaccessibleDirAttrs : PASS


Command line asked for 27 of 669 tests
Of those: 0 Skipped, 2 Failed, 0 Warned, 25 Passed


  • 0
  • 0
  • 14
  • 5
  • 19
Description From Last Updated
marcel
  1. 
      
  2. Plase change rfc7530 to RFC 7530.
  3. Please use ae instead of ar here.  The ae value is what will be encoded, not ar.  I know that in this particular case the result will be same, but ae used here should be easier understandable for future readers.
    
    This will also avoid a possible future problem when someone will change the source code above and the presence of the FATTR4_RDATTR_ERROR_MASK flag will start to differ in both ar and ae.
  4. What will happen in a case the fs_locations is requested, rdattr_error is not requested and the file system we are requesting attributes for is moved?  My understanding of section 8.3.2 is that the READDIR operation should succeed, but I suspect this code will make the whole READDIR operation to fail with NFS4ERR_MOVED in such a case.
    1. Yes, I mentioned that in Description of the review. It will make the code more complex and possibly without real usage of this. The common approach is:

      1. READDIR request with rdattr_error
      2. Reply contains NFS4ERR_MOVED for referrals
      3. A client analyses and send GETATTR with fh_location for those previously returned entries with NFS4ERR_MOVED.
      

      The intension of this patch - strict READDIR reply according to RFC because vmware vSphere crashes due to this.

      If someone requested fh_location handling w/o set rdattr_error, then it can be added of course.

    2. Ah, now I realized that you stated this in the review Description:

      The patch intensionally misses first part of 8.3.2: returning success if fs_locations is set.
      

      Unfortunately, this creates a regression, which is not acceptable. Sorry.

    3. Ha, we replied concurrently without seeing each other, and you were faster :-).

      Anyway, sorry, but you cannot fix one RFC 7530 violation by creating another RFC 7530 violation, even you believe such a case is rare.

      And no, you do not achieve strict READDIR reply according to RFC because you just introduces the RFC 7530 violation in a case the fs_location is requested.

  5. 
      
vgusev
vgusev
marcel
  1. 
      
  2. Please add word "Section" between 7530 and 8.3.1.

  3. Section 8.3.1 of the RFC does not list the rdattr_error attribute. Also, the section 8.3.1 is for GETATTR and VERIFY/NVERIFY only, not for READDIR.

    Maybe this is just a typo and the comment above should say 8.3.2 instead of 8.3.1?

  4. Please run git pbchk. Thanks.

  5. This comment should be updated.

  6. The extra empty line is not needed here.

  7. This should be X == 0, not !X.

  8. Here we assume that *bmval cannot be zero (the caller of this function is not prepared for that). I suggest to add ASSERT(*bmval != 0) after this line.

    1. Sorry, bad wording here. I wanted to say: (and the caller of this function is not prepared to see *bmval as zero after the return from this function).

  9. Why is this code no longer needed here?

  10. 
      
vgusev
  1. 
      
  2. No, it wasn't typo. Section 8.3.1 describes GETATTR valid attributes for absent filesystem. This is also true for READDIR attributes with a little addon in Section 8.3.2.

    Unfortunately there is no direct pointing from READDIR getting attributes to GETATTR attributes in this RFC, and we are rely on robust logic and linux nfs server sources.

    rdattr_error is valid attribute and described below in Section 8.3.2. and in READDIR operation.

    1. Ah, I think I understand what caused the confusion.

      My interpretation of the RFC is that for the GETATTR operation for an object in the absent file system the NFS server should return fs_locations, fsid, mounted_on_fileid only and nothing else (from RFC: Other attributes SHOULD NOT be made available for absent file systems, even when it is possible to provide them.). This is from Section 8.3.1 and it is valid for the GETATTR operation, not for READDIR.

      For the READDIR operation Section 8.3.2 applies. Here, the set of attributes is not limited. It means that the NFS server should provide as much as possible attributes as it can. It could easily happen that the set of the attributes that the NFS server can support will be limited to rdattr_error, fs_locations, fsid, and mounted_on_fileid only (or even without mounted_on_fileid, because it is RECOMMENDED only; and even without fsid because it could be not possible for the NFS server to determine it in some extreme case), but this limitation is not because of the Section 8.3.1.

      So, you can limit the set of the attributes for the absent file system for the READDIR operation to rdattr_error, fs_locations, fsid, and mounted_on_fileid only (if we really cannot support more), but you cannot say it is because of Section 8.3.1.

  3. It can be zero. No ASSERT is required.
    1. I probably miss something, but I see no way how *bmval could be zero after this line. Please show me the scenario how it can happen. Thanks.

  4. It is impossible to mount in softlink.

    Referral is link. vn_is_nfs_reparse() quickly returns FALSE for non-link vnode.

    Therefore if @vp is mountpoint then this is not referral.

    1. IOW, does it mean that this code was not needed even without your change?

  5. 
      
vgusev
  1. 
      
  2. I think you and me are on the same page. However, "restricted to those actually available" information from 8.3.2 is as result of 8.3.1. What it why I mentioned "8.3.1" in comment.
    1. Could you please use the "Reply" button so the discussion is properly threaded and we do not lost the context? Thanks.

      Sorry, but I disagree. 8.3.2 is not directly related to 8.3.1. Section 8.3.1 applies to the GETATTR operation only. For READDIR you should return as much attributes as possible, even the file system entry is a root of the absent filesystem.

    2. RFC7530 doesn't have description for valid attributes for Absent Fs and only one place is 8.3.1.

      Originally in Linux sources it has comment: "As per referral draft".

      This document (draft-ietf-nfsv4-referrals-00) has "3.3. Attributes Needed for Root of an Absent Fs" and "3.4. Attributes Valid for Root of an Absent Fs"

    3. Sorry, but both Linux sources and RFC drafts are not authoritative sources for the NFSv4 specification. Only RFC 7530 and other related RFCs are. If you think the RFC is not clear enough in this area (I don't think so) you can clarify this with the IETF NFSv4 working group and/or eventually submit an RFC errata.

    4. If I change comment with "RFC 7530 Section 8.3", will it be OK for you ?

    5. No. The Section 8.3 of the RFC does not limit the attributes to those four you are listing here for the READDIR operation with the absent fs entries.

    6. So you want I remove this comment?

    7. I would like to see implemented as much as possible attributes for READDIR for absent fs to follow RFC 7530. In a case we need to limit the list of attributes for some reason then the reason should be well understood.

    8. Other attributes are not valid for an absent filesystem. If you argee with that I put this information in comment for the ABSENT_FS_ATTRS.

    9. I'm not sure it is true. For example I think we could provide the lease_time attribute. Probably the filehandle attribute should be supported for READDIR too.

    10. You are wrong about filehandle. filehandle is never supported for absent filesystem and can't be returned.

      lease_time is per-server attribute and returning it is also wrong asumption because client do not need it for this entry.

      Additionally repeat that getting attribute via GETATTR and READDIR almost the same except READDIR is stronger and have some restriction. So that READDIR must never return more attribute than GETATTR and therefor Section 8.3.1 and 8.3.2 are actual.

    11. filehandle is never supported for absent filesystem and can't be returned.

      Such filehandle can't be returned by GETFH. It is a consequence of the fact that if the current filehandle on a start of an operation is for an absent filesystem the operation will fail with NFS4ERR_MOVED. But it is easily possible that the client can have the filehandle of the absent object and supply it to the server via the PUTFH operation. Maybe one of the ways how to get the absent filehandle is the READDIR operation.

      lease_time is per-server attribute and returning it is also wrong asumption because client do not need it for this entry.

      We do not know what a client needs or does not need. The important question here is: What should happen if the client will request the lease_time attribute for the READDIR operation and there are some absent entries? Should it be returned or not? If not, then why not?

      So that READDIR must never return more attribute than GETATTR ...

      The GETATTR operation should return fs_locations, fsid, and mounted_on_fileid only (Section 8.3.1 of RFC 7530). If "READDIR must never return more attribute than GETATTR" (your words) how is it possible that READDIR should return rdattr_error too (Section 8.3.2 of RFC 7530)?

    12. |Such filehandle can't be returned by GETFH. It is a consequence of the fact that if the current filehandle on a start of an
      |operation is for an absent filesystem the operation will fail with NFS4ERR_MOVED. But it is easily possible that the client can
      |have the filehandle of the absent object and supply it to the server via the PUTFH operation. Maybe one of the ways how to get
      |the absent filehandle is the READDIR operation.

      Your scheme via READDIR is impossible. Archivement can be done only via "LOOKUP".

      | We do not know what a client needs or does not need. The important question here is: What should happen if the client will
      | request the lease_time attribute for the READDIR operation and there are some absent entries? Should it be returned or not? If | not, then why not?

      I don't think that current nfs4 server implementation must be more NFSv4 than Linux NFSv4 server implementation. There are lot of reasons for that. Generally, lease_time is obtained at initial stage when client gets all server and channel information.

      Restriction ABSENT_FS_ATTRS is more than enough for clients and is bigger than current illumos nfs server has.

      |The GETATTR operation should return fs_locations, fsid, and mounted_on_fileid only (Section 8.3.1 of RFC 7530). If "READDIR must
      |never return more attribute than GETATTR" (your words) how is it possible that READDIR should return rdattr_error too (Section
      |8.3.2 of RFC 7530)?

      It is because rdattr_error generally is not attribute and is alias for GETATTR::status. READDIR reply doesn't have statuses for entries and rdattr_error is used to indicate status of getting attributes for an entry (if requested). GETTATR has status field that indicates success or fail for getting attributes.

      Generally attributes must be the same, exception are in 8.3.2.

    13. It is because rdattr_error generally is not attribute and is alias for GETATTR::status.

      Please read Section 5.6 of RFC 7530 to see that rdattr_error is actually a REQUIRED attribute.

      Apparently this discussion goes nowhere so I'm giving up.

    14. It is because rdattr_error generally is not attribute and is alias for GETATTR::status.
      Please read Section 5.6 of RFC 7530 to see that rdattr_error is actually a REQUIRED attribute.

      I just explained origin of rdattr_error requirements in READDIR operation.

      Apparently this discussion goes nowhere so I'm giving up.

      I don't get what is reason to spend time for disscussion whether ABSENT_FS_ATTRS need something additional or not. I pointed to Section 8.3 in RFC7530 where it is specified.

      Will update the patch according to your remarks.

      Thanks.

  3. Caller is aware if returned zero *bmval.
    1. ???

      Please show me how *bmval could be zero. Thanks.

    2. Sorry, maybe I miss something, but you asked whether is problem if bmval becomes zero after "&=". I answered no. If you ask about is it possible that bmval can be zero after "&=", then asnwer is "no" at least FS_LOCATIONS is left.

      Your pointing to the caller brought to me idea to fix usage "ar" after nfs4_readdir_getvp() call. Do you approve that idea?

  4. Yes, I think so. Generally, in Linux community it must be prepared as separate patch.
    1. We do not care what other communities does :-).

  5. 
      
vgusev
tsoome
  1. 
      
  2. sizeof (fs) here and below?

    1. If rule is not specified implicitly, I would use direct type specification due to it shows and helps to understand origin of the used variable. Additionally this practice is useful if "fs" is changed to pointer.

  3. 
      
tsoome
  1. Ship It!
  2. 
      
marcel
  1. 
      
  2. Please add more descriptive comment about this. Simple reference to section 8.3 in RFC without any other explanation is not enough, I think. Especially, because particularly section 8.3.1 does not apply here.

  3. Actually, these parts of 8.3.2 applies here:

       o  If the attribute set requested includes fs_locations, then the
          fetching of attributes proceeds normally, and no NFS4ERR_MOVED
          indication is returned even when the rdattr_error attribute is
          requested.
    
    ...
    
       o  The unavailability of an attribute because of a file system's
          absence, even one that is ordinarily REQUIRED, does not result in
          any error indication.  The set of attributes returned for the root
          directory of the absent file system in that case is simply
          restricted to those actually available.
    

    IOW, for READDIR that contains an absent directory entry we should provide as much attributes as possible for that missing entry, but this line effectively limits the set of provided attributes to those four only without any explanation why. Unfortunately, this is not aligned with section 8.3 of the RFC (although the comment says so).

    1. I do not agree. Look at:
      "The set of attributes returned for the root
      directory of the absent file system in that case is simply
      restricted to those actually available".

      That means we can limit number of attributes for an absent filesystem and ... do it as 8.3.2 says!

      To satisfy your complaints I would add the following to ABSENT_FS_ATTRS description:

      /* 
       * RFC 7530 Section 8.3 
       * Supported number of attributes for the root of an absent filesystem. 
       */
      
    2. Yes, exactly. The only problem is that 8.3.2 does not say what should be the set of available attributes.

      We have two cases here:

      1) GETATTR - in this case the set of supported attributes is: fs_locations, fsid, mounted_on_fileid (and nothing else) - see 8.3.1.
      2) READDIR - in this case the set of the supported attributes could (and very likely will) be limited, but the RFC does not specify the exact list (see 8.3.2). We should support as much attributes as possible.

      The question is: what is the right list of attributes we can support for the READDIR case? It is easily possible that the list contains those 4 attributes only (those you defined as ABSENT_FS_ATTRS). We can do that, but the limit to those 4 attributes is not because the RFC says so, but because we have no way how to support more attributes.

    3. I suggest to leave it as is, because the polishing doesn't move a train forward. It would be nice if RFC7530 had full description for all supported attributes for READDIR in case of an absent filesystem similar for GETATTR.

  4. 
      
vgusev
marcel
  1. 
      
  2. Please change 8.3 to 8.3.2

  3. I suggest to change this to something like:

    Supported attributes for READDIR for the root of an absent filesystem.

  4. that -> what

  5. 
      
vgusev
vgusev
tsoome
  1. 
      
  2. usr/src/uts/common/fs/nfs/nfs4_srv_readdir.c (Diff revisions 6 - 7)
     
     

    here we do pointer arithmetics, I think, we may have bug there as ptr is uint32_t *

    1. It is correct, because rndup is measured in BYTES_PER_XDR_UNIT == 4 bytes.

      Additionally want to note that it is not my change: It is not my change: ^7c478bd953 (stevel@tonic-gate 2005-06-14 784)

  3. usr/src/uts/common/fs/nfs/nfs4_srv_readdir.c (Diff revisions 6 - 7)
     
     

    we should not need void * casts for pointer (for second argument); but pointer arithmetics for third argument - wouldn't it better to use uintptr_t to do the arithmetics and then cast final result to void *?

    1. we should not need void * casts for pointer (for second argument);

      Why do you think so? Pointers for deduction should point to the same type, otherwise it is not compiled. Try "(void *)ptr_redzone - ptr" as you suggested.

      uintptr_t to do the arithmetics and then cast final result to void *

      It can be done in any variants, so I would leave it as is, but if you insist and in illumos kernel style is using uintptr_t, I will replace it with uintptr_t.

    2. I do not mind either way as long as it is working:D Sure we can play the game of casts, but excessive casting is quite unreadable too.

  4. 
      
vgusev
tsoome
  1. Ship It!
  2. 
      
domag02
  1. 
      
  2. Misused goto, it used just like a do...while loop:

        do {
            from: line 628
            .
            .
            .
            to: line 1487
        } while (!no_space && nents == 0 && !iseofdir);
    
    1. You are right about style of this block of code. But this patch fixes 'reference issue' #7782 and doesn't refactor all generic code (intention is stability, no regression and blame-annotate). Let's leave it as is.

      If you have checker tool's warning it should be fixed of course.

  3. 
      
domag02
  1. 
      
  2. Another misused goto, it used just to skip over a section of code:

            if (ar != 0) {
                from: line 715
                .
                .
                .
                to: line 763
            }
    
    1. The same as for previous: Let's leave generic code as is and really "goto reencode_attrs" is informative in this case.

      If you have checker tool's warning I will fix it.

    2. Tools can't check for misused gotos, but using goto like these doesn't conform to the C Style and Coding Standards.
      From cstyle(1ONBLD) manual page:

      ...
      It attempts to check for the cstyle documented in cstyle.ms.pdf.
      Note that there is much in that document that cannot be checked for;
      just because your code is cstyle(1ONBLD) clean does not mean that you've
      followed illumos C style. Caveat emptor.

      Please create separate ticket for fixing these instead of leaving as-is
      (of course, you can leave this style clean-up job to somebody else).

      A comment before the if (ar != 0) { line could be better to describe why this decision is made
      and what this section of code will do in summary.

  3. 
      
domag02
  1. 
      
  2. The same macro constants used multiple times in this expression:
    2x FATTR4_FILES_AVAIL_MASK,
    2x FATTR4_FILES_FREE_MASK and
    2x FATTR4_FILES_TOTAL_MASK.

    It seems like a typo-ed NFS4_FS_ATTR_MASK macro from nfs4_attr.h:171.

    1. Issue #9670 created.

      Thanks! Created separate ticket as fixing those are out of current ticket description.

  3. 
      
domag02
  1. 
      
  2. (2.)
    The same macro constants used multiple times in this expression:
    2x FATTR4_FILES_AVAIL_MASK,
    2x FATTR4_FILES_FREE_MASK and
    2x FATTR4_FILES_TOTAL_MASK.

    It seems like a typo-ed NFS4_FS_ATTR_MASK macro from nfs4_attr.h:171.

    1. Issue #9670 created.

  3. 
      
domag02
  1. 
      
  2. (3.)
    The same macro constants used multiple times in this expression:
    2x FATTR4_FILES_AVAIL_MASK,
    2x FATTR4_FILES_FREE_MASK and
    2x FATTR4_FILES_TOTAL_MASK.

    It seems like a typo-ed NFS4_FS_ATTR_MASK macro from nfs4_attr.h:171.

    1. Issue #9670 created.

  3. 
      
vgusev
domag02
  1. Ship It!
  2. 
      
danmcd
  1. 
      
  2. The comment is a bit unclear with "can be restricted". How about "can have values masked-out" instead. I.e. "@bmval can have values masked-out due to an absent file system."

  3. 
      
vgusev
danmcd
  1. Ship It!
  2. 
      
vgusev
danmcd
  1. Per our IRC chat, make sure the tests reconfirm the newest changes, and bonus points for explaining how changeset 10 came to be.

  2. 
      
tsoome
  1. Ship It!
  2. 
      
domag02
  1. Ship It!
  2. 
      
vgusev
vgusev
tsoome
  1. Ship It!
  2. 
      
vgusev
vgusev
Review request changed

Status: Closed (submitted)

Loading...