Record physical path information in ZFS Vdevs

[asomers]

Summary

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c:
If available, record the physical path of a vdev in ZFS meta-data.
Do this both when opening the vdev, and when receiving an attribute
change notification from GEOM.

Make vdev_geom_close() synchronous instead of deferring its work to
a GEOM event handler. There is no benefit to deferring the work and
this prevents a future open call from referencing a consumer that is
scheduled for destruction. The close followed by an immediate open
will occur during a vdev reprobe triggered by any type of I/O error.

Consolidate vdev_geom_close() and vdev_geom_detach() into
vdev_geom_close() and vdev_geom_close_locked(). This also moves the
cross linking operations between vdev and GEOM consumer into a
single place (linking in vdev_geom_attach() and unlinking in
vdev_geom_close_locked()).

Test Plan

On a system with SES expanders, use "zdb -l /dev/daX" to check that the
phys_path is set to something like
'id1,enc@n50015b20980e273d/type@0/slot@1/elmdesc@ArrayDevice01'.

[smh]

The rest of the changes look OK, but I'm no expert in geom so someone else who is should review this as well.

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c

96 ↗

(On Diff #11155)

Move above the g_topology_assert?

106 ↗

(On Diff #11155)

Any reason to not just use physpath[MAXPATHLEN] declaration instead?

The only reason I can think of is if you add in the retry based on failure with the new value of physpath_len, but as that should never happen in this case we should be good.

110 ↗

(On Diff #11155)

Might be nice to use the following to avoid the extra level on indentation?

if (error != 0)
    return;

Will also need g_free(physpath); if you don't switch to static buffer.

[asomers]

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c

96 ↗

(On Diff #11155)

Actually, we can remove g_topology_assert altogether, because g_access calls g_topology_assert.

106 ↗

(On Diff #11155)

MAXPATHLEN is 1kB. Do you really want to add that much to the stack?

110 ↗

(On Diff #11155)

If you don't mind, I'm planning to fix the indentation with a future commit that will pull all of this code out into a separate function.

g_free(physpath) is there at the bottom of the function.

[asomers]

Remove extraneous g_topology_assert

[smh]

just the one more style(9) nit

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c

807 ↗

(On Diff #11332)

missing blank line.

[asomers]

style fix suggested by smh

[asomers]

Fix a panic when closing a shared spare vdev. It's possible for ZFS to open
the same geom consumer more than once. If that happens, the cp->private field
will get screwed up, causing attrchange and orphan events to get ignored. On
the second time that vdev is closed, the system would panic due to an
inappropriate KASSERT included in the first version of this CR.

In due course I'll fix the attrchange and orphan event problems. They were not introduced by this CR, only the panic was.

[asomers]

smh, are you ready to accept this revision?

[asomers]

Ping. Are you ready to accept this revision, or do you need me to find more reviewers?