7693 pynfs: OPEN4 st_open.testCreatExclusiveFile FAILURE

Review Request #308 — Created Dec. 27, 2016 and submitted

Information
vgusev
illumos-gate
master
7693
92b9401...
Reviewers
general
marcel 
7693 pynfs: OPEN4 st_open.testCreatExclusiveFile FAILURE
./testserver.py 192.168.5.152:/codepool/data --rundeps --nocleanup OPEN4




INIT     st_setclientid.testValid                                 : PASS

OPEN4    st_open.testCreatExclusiveFile                           : PASS




Command line asked for 2 of 662 tests

Of those: 0 Skipped, 0 Failed, 0 Warned, 2 Passed
andy_js
andy_js 
  1. Ship It!
  2. 
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   




rm




   

   







  

 



 

  


   

   

    
 
rm


    
   


  

  

  



    




  1. 
 
    
  
    
   
    I think the changes make sense here. Thanks for putting it together.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   




marcel




   

   





  

 



 

  


   

   

    
 
marcel


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      usr/src/uts/common/fs/nfs/nfs4_srv.c
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    




    
    This copy operation is valid only in a case when vap->va_mask contains AT_MTIME.  This is definitely true for mode == EXCLUSIVE4, but for other modes the vap->va_mtime might just be uninitialized.

This is not a big problem, because mtime is used (below) only for the EXCLUSIVE4 case, but still, the initialization of mtime here is confusing a bit.

I suggest to remove the mtime local variable and use vap->va_mtime.tv_sec and vap->va_mtime.tv_nsec below respectively.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      Marcel, thanks for your suggestion. But one thing why I did so is that vop_create() could change vap->va_mtime because @vap passed here not as const pointer. That is why I did caching via @mtime.
      
 
      

      2. 



    2. 
 
 
      
 
      
  
 
      
 
      
  
      I do not think there is any danger that VOP_CREATE() will change the vap->va_mtime value.  Especially in a case the VOP_CREATE() call fails.  But if you are really afraid of that, please add a comment describing why do you need to "cache" the vap->va_mtime value.  Thanks.
      
 
      

      3. 


 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 



 

  

   




vgusev




   

   

  

 



 















 





 

  

   




marcel




   

   







  

 



 

  


   

   

    
 
marcel


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   




kmays




   

   







  

 



 

  


   

   

    
 
kmays


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




vgusev




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  






 

  Status:
  

  Closed (submitted)

  
 




 
Change Summary:

 
commit 77e6f23f311b4352dcc27ce2fcbd2c06546ce837

Author: Vitaliy Gusev gusev.vitaliy@gmail.com

Date:   Mon Jan 16 11:34:17 2017 -0500


7693 pynfs: OPEN4 st_open.testCreatExclusiveFile FAILURE
Reviewed by: Andrew Stormont <andyjstormont@gmail.com>
Reviewed by: Robert Mustacchi <rm@joyent.com>
Reviewed by: Marcel Telka <marcel@telka.sk>
Approved by: Dan McDonald <danmcd@omniti.com>






 


 










  

 









     

    

   

  

  

   
   Loading...