Plumb ZPOOL_CONFIG_PHYS_PATH and ZPOOL_CONFIG_DEVID into vdev label

[don-brady]

In the ZFS vdev label, ZFS uses the ZPOOL_CONFIG_DEVID identifier to refer to a persistent name for the disk and the ZPOOL_CONFIG_PHYS_PATH location to refer to the actual location of the disk on the system.

illumos example:
path: '/dev/dsk/c0t1d0s0’
devid: 'id1,sd@SATA_____Hitachi_HDS72101______JP2940HZ3H74MC/a'
phys_path: '/pci@0,0/pci103c,1609@11/disk@1,0:a'

I’m in the process of implementing the auto-{expand, online, replace} functionality in zfsonlinux when a device add event occurs (hot plug). This logic requires being able to reliably match a vdev by identifier or by location for the disk add event. On Linux, it seems the tail element of "/dev/disk/by-id/<bus-disk-identifier>" maps perfectly for a ZPOOL_CONFIG_DEVID. And the tail element of "/dev/disk/by-path/<path>" maps to a ZPOOL_CONFIG_PHYS_PATH. Linux currently only uses the ZPOOL_CONFIG_PATH in the vdev label. This path is not normalized and can sometimes refer to the non-persistent /dev/sd[a-z] device names.

Linux example:
For a typical SAS HBA with a sata backplane, we see something like:
/dev/sdi1
/dev/disk/by-id/ata-ST3500418AS_9VMGAJ49-part1
/dev/disk/by-path/pci-0000:04:00.0-sas-0x4433221107000000-lun-0-part1

Which could map as follows:
path: '/dev/disk/by-id/ata-ST3500418AS_9VMGAJ49-part1'
devid: 'ata-ST3500418AS_9VMGAJ49-part1'
phys_path: 'pci-0000:04:00.0-sas-0x4433221107000000-lun-0-part1'

The libudev event reporting mechanism provides the by-id and by-path properties (when the device is on a bus like ata or scsi). I’d like to propose backing ZPOOL_CONFIG_DEVID and ZPOOL_CONFIG_PHYS_PATH with actual values (when available) so that auto-{expand, online, replace} can work as it does on illumos. These additional device values can also help when logging data (e.g. zpool history and zpool events).

Is there any objection to using the above mappings in zfsonlinux (when they exist)? The existing use of ZPOOL_CONFIG_PATH can remain intact and still be used for vdev_open and what gets displayed when listing a pool config.

[don-brady]

Related to
zpool autoexpand doesn't work #120
Autoreplace not working #2449

[ilovezfs]

Would there be any value in also storing the transient names in a separate field on the label?

[don-brady]

@ilovezfs Are you thinking of an additional field besides the stock 4 (“path”, “devid”, “phys_path” and “fru”)? If so, who would consume it if it was stored in the label? Just wondering what the use case would be.

The /dev/disk/by-id is just a symlink to the runtime devnode so if you have the by-id path you can easily derive the current transient name. This proposal isn’t changing the fact that the ‘path’, value (ZPOOL_CONFIG_PATH) can hold a transient path (like /dev/sda) which can be useful for testing purposes.

[ilovezfs]

I was thinking about it as a string to grep for in the logs, and as a match to the entries in zpool status for users that do use /dev. Would one of the stock four still match the zpool status entries when the pool has been imported -d /dev?

(Of course, one downside to maintaining the transient names in any way is that it would mean dirtying the labels on import/boot just due to the transient names' changing in order to keep the information on the label current.)

[don-brady]

Initial prototype for plumbing in ZPOOL_CONFIG_DEVID and ZPOOL_CONFIG_PHYS_PATH
looks promising. A single function in libzfs provides these strings from libudev during a
pool create|add|attach|import.

extern void update_vdev_config_dev_strs(nvlist_t *nv);

tested with sata, scsi and usb devices
will begin testing with disk-add events for auto-{online,expand,replace}

[ilovezfs]

@don-brady Very cool. Does it happen to get usable paths for virtual devices that get treated as whole_disk=0 like LUKS, LVM, loopback, md, etc.? https://github.com/zfsonlinux/zfs/blob/master/lib/libefi/rdwr_efi.c#L182-L240

[behlendorf]

@don-brady this sounds like a entirely reasonable approach to me. I can't think of any reason not to store the mapping you're proposing.