Fix ambiguous bay/disk labels on Joyent-S10G5

Review Request #2471 — Created Dec. 10, 2019 and submitted

Information
rejohnst
illumos-gate
12068
Reviewers
general 
The Joyent S10G5 platform contains two SAS expanders to drive the front and rear disks. The SES implementation on both of these expanders encodes a simplistic element descriptor string for the array device elemnts that uses the format "Slot##". The result is that front disks get labels "Slot00 - Slot23" and the rear disks get labels "Slot00-Slot11" and so you have essentially duplicate labels for the first 12 disks between the front and rear disk backplanes.


This ticket is to cover changes to the ses enumerator to add custom logic to dynamically assign less ambiguous labels to the bay and disk nodes when we detect we're on the Joyent S10G5 platform. The logic will look at the product ID of the SAS expanders (which are different between the front and rear) and create labels "Front Slot 0" through "Front Slot 23" "Rear Slot 0" through "Rear Slot 11" for the front and rear bay/disks nodes, respectively.
See ticket for testing details:


https://www.illumos.org/issues/12068
  • 0
  • 0
  • 3
  • 1
  • 4
Description From Last Updated
rm
rm 
  1. 
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      Thanks for looking at this and sorry for the long delay in responding - I've been out the last few weeks.  I've restructured this change quite a bit and retested it across the different variants of the SMCI 4u36 that we have deployed internally.
      
 
      

      2. 


 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    Can we structure this so it's not quite so hardcoded to be just Joyent here? I think you may want an array of things which are overriden. As this likely impacts not just the Joyent model, but also the underlying SMCI system that this is built upon (assuming https://github.com/joyent/manufacturing/blob/master/allocations.md#storage-products is accurate).
    

    Maybe having a top-level array of products and methods to call? I think that'd make this a little bit cleaner and more extensible as this shouldn't be specific to a single SKU.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      Yeah, I restructured this code quite a bit to both make it more extensible and also make sure we can apply it to all the variants of the SMCI 4U36.
      
 
      

      2. 


 
    


    

 
    

    3. 






  3. 
 
    
  
  
    
  
 

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    I believe you want to initialize nvl to NULL. If topo_mod_nvalloc fails, we will always call nvlist_free() on it, but there's no guarantee that the failing function will zero out the nvl pointer. Certainly the existing code doesn't today.
    
  
    
 
    

 
    
  



 
    

    4. 






  4. 
 
    
  
  
    
  
 

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    Maybe this should look like the case at +3668 (or vice versa) as I don't believe there's any change in resource mappings between them.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      I left this as-is.   Maybe I'm not understanding the concern here?
      
 
      

      2. 



    2. 
 
 
      
 
      
  
 
      
 
      
  
      I think this was trying to say using topo_mod_seterrno() and goto err, rather than handling it differently. Though it was some time ago and my original message was pretty vague.
      
 
      

      3. 


 
    


    

 
    

    5. 






  5. 
 
    
  
  
    
  
 

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    cstyle wants the != bits on the previous line.
    
  
    
 
    

 
    
  



 
    

    6. 




  6. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    7. 




  

 
















 



 

  

   




rejohnst




   

   

  

 



 















 





 

  

   




rm




   

   







  

 



 

  


   

   

    
 
rm


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




rejohnst




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  






 

  Status:
  

  Closed (submitted)

  
 







 


 










  

 









     

    

   

  

  

   
   Loading...