11076 ZFS test mmp_on_zdb should clear disk labels on exit

Review Request #1865 — Created May 29, 2019 and submitted

kkantor
illumos-gate
reviewboard
ae02d38...
general

11076 ZFS test mmp_on_zdb should clear disk labels on exit

We should use the common cleanup_devices pattern instead of manually clearing labels.



  • 0
  • 0
  • 1
  • 0
  • 1
Description From Last Updated
andy_js
  1. Ship It!
  2. 
      
tsoome
  1. Ship It!
  2. 
      
igork
  1. Ship It!
  2. 
      
xvoid
  1. I wonder how this works at all, running labelclear on imported pool will fail no matter if '-f' is specified or not.

    1. Doh, I missed the destroy in cleanup. But why labelclear is needed, i.e. how this test is different from others that it requires the labels cleanup?

    2. A number of tests do something similar to 'zpool labelclear' using the 'cleanup_devices' function defined in libtest.shlib. cleanup_devices overwrites disk labels with valid pool labels for a unique pool named 'foopool[pid]' rather than explicitly deleting the labels as the mmp_on_zdb test does. This is actually a better approach than using 'zpool labelclear' since it's a common function. Now that I see cleanup_devices I'm going to change this to use that instead.

      I would prefer that all tests that create a pool would clean the disk labels when they exit, but the majority of tests do not do this.

  2. 
      
xvoid
  1. 
      
  2. Can we make this labelclear -f $DEV_RDSKDIR$DISK and have DEV_RDSKDIR end with / in ZoL? This way they'll still get the correct path and we will end up with just disk name.

  3. 
      
jkennedy
  1. 
      
  2. Perhaps it would be nice to import the is_linux function from ZoL for this? It does the same thing, but by using it we could more easily share code.

    1. Yeah, that's a good idea. Since we already use the case $(uname) in pattern in various places right now I would prefer to leave this as it is. We should pull in the ZoL changes and refactor the tests to use is_linux/is_illumos in a separate commit though.

  3. 
      
kkantor
kkantor
jjelinek
  1. Ship It!
  2. 
      
tsoome
  1. Ship It!
  2. 
      
igork
  1. Ship It!
  2. 
      
andy_js
  1. Ship It!
  2. 
      
jkennedy
  1. Ship It!
  2. 
      
kkantor
Review request changed

Status: Closed (submitted)

Loading...