Thanks for the quick review.

I think you are suggesting that I should be scaling volsize after numdb has been added.  Is that right?

I'm all over that.

heh.  I used to always not use $ in arithmetic expressions, then I was convinced to switch to using $ and suffering the "unnecessary string conversion" messages from ksh -n.  The argument for $ is so that tracing (set -x) gives more meaningful output.  Regardless, it should be consistent.

I previously had refreserv_00{6,7} as a single test.  Since they are really testing different things, I split them apart.  Most of the of the logic near the top of both is quite similar.  006 now skips tracking of refreservation for each zvol and 007 skips the dd in the first loop.

The version that is in reservation.shlib is still accurate for single device or mirrored zpools (and some raidz), as evidenced by tests continuing to pass.  While this could be extended to add the special case for raidz, I question whether implementing the same algorithm in a different language is the right way to ensure the alogithm is correct.  New tests verify that the refreservation is appropriate by actually filling some zvols and ensuring that the result falls within an acceptable range.

I'm not sure if the existing tests pass due to the specific sizes of disks you're testing with or if there are no configurations in the existing tests which would fail with raidz and different sizes of disks. I went through all of the tests using the shell version of the code, but I am still not certain.

Thanks for the review.

This work was done specifically for the 4k scenario, which is why I go through specific effort to test it.
I had initially started down the path of using files or zvols on $TESTPOOL and kept getting zfs-on-zfs deadlock.  As noted in my test notes, some of the existing tests caused repeatable zfs-on-zfs deadlock while trying to get a baseline.  It is for this reason that I went down this other path.
A 3 disk configuration will provide basic coverage for 2 and 3 disk raidz configurations.

It looks as though default_cleanup_noexit could be used in place of the zpool destroy, but it is not a substitute for default_setup_noexit.  Other tests in this directory assume that $TESTPOOL has already been created by setup.

will fix

I think that would be the wrong thing to do.  0 is a string that can only be interprted as itself.  Since [[ ]] is used, it is not coercered into an integer.  The thing we are comparing it to is the name of a key from an associative array, which is also a string.  If I were to change it to -eq, then ksn -n is not happy.

$ cat -n f.ksh
     1  #! /bin/ksh
     2
     3  if [[ $a == 0 ]]; then
     4          echo blah
     5  fi
     6  if [[ $a -eq 0 ]]; then
     7          echo blah
     8  fi
$ ksh -n f.ksh
f.ksh: warning: line 6: -eq within [[...]] obsolete, use ((...))


Per The New Kornshell by Bolsky and Korn (1995) page 141, (emphasis mine):



A primitive can be any of the following exprssions that compare to arithmetic expressions:

 exp1 -eq exp2 ...

The man page in SmartOS is kinda broken in the area around -eq.  If you squint a little bit, you can see how someone messed up while transcribing the book, leaving out the sentence about arithmetic expressions, implying that -eq has something to do with files.
https://drive.google.com/open?id=1HpHMxIrX23aWt4Cz35-0Npv0m3eEwqiq

https://drive.google.com/open?id=1-809JrpyUx-xiHgVOir582Pv9R312Slg

Same as with the other file

will do

I then expect the next reviewer to say "you don't need those parens, because that's the normal order of operations".  I'll just delete the comment because none of the reviewers were confused before I added it.

ok

ok