Fix spa_nonallocating_dspace for special vdevs

[amotin]

Since spa_dspace accounts only normal allocation class space, spa_nonallocating_dspace should do the same. Otherwise we may get negative overflow or respective assertion in spa_update_dspace() if removed special/dedup vdev is bigger than all normal class space.

How Has This Been Tested?

Originally I spotted it while reviewing ztest CI failures. After that I reproduced it manually by trying to remove from a pool special vdev that is bigger than normal vdev. Applying the patch fixed the manual test.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

[jumbi77]

Well spotted. Is a dedicated test case useful here?

[amotin]

@jumbi77 I haven't looked what device removal tests do we actually have in ZTS now, but if you wish to review them and add a case to something existing -- be my guest.

[pcd1193182]

This looks similar to the patch I was testing out for #17182 , though I also added handling for the embedded slog. This is simpler, though it does mean that sometimes our value for the space we still have available will be wrong. This might be the source of some ENOSPC bugs that I've seen in ztest, now that I think about it; we use dspace to see how much space we have left in the pool, and if it's overestimating, we could have allocations fail that should not be able to.

[amotin]

@pcd1193182 If you think your patch looks better, I don't care which one goes in. I just wanted CI to be happy. PS: I think without embedded slog accounting we should underestimate the free space, not overestimate it.

[pcd1193182]

Ah right, since this is the value we're subtracting from the dspace. In that case, the problem we would run into is if the total space in the normal class is less than the size of the vdev we're removing, which means all the other vdev's normal class space would have to add up to less than the embedded slog metaslab on the removing vdev. In that case, we could still panic. Not impossible, but needs a pretty weird pool configuration.

Let me post a PR with my version and we can see which we prefer.

[amotin]

Closing in favor of #17183 .