ZFS initrd script does not import zpool using /dev/disk/by-id device paths

[Sam Van den Eynde]

Ubuntu 16.04 includes initrd zfs support, but the provided script does not allow zpools to be imported using the /dev/disk/by-id paths. As a result, the pool will be imported using "/dev/sdX" device names, which is not the preferred way.

Tested and validated solution/proof of concept:

 - extract system generated initrd
 - edit scripts/zfs file: replace "zpool import -o readonly=on -N" with "zpool import -o readonly=on -d /dev/disk/by-id -N"
 - manually generate initrd using the altered script (using cpio)
 - replace system generated initrd with altered one

I'd suggest adding a variable to /etc/default/zfs that would be reflected in the initrd zfs script, but perhaps the ZoL folks are better suited to give advice on this.

[Richard Laager (rlaager)]

I'm away from my computer at the moment, or I'd test more myself. Did you initially have the pool imported using the by-id names? Is the problem that the initrd needs a zpool.cache file?

[Sam Van den Eynde (samvde)]

It is a new install on zfs root. I created the pool using the by-id names, and during my tests I have exported and re-imported it using the by-id names as well. As this is a root filesystem, this obviously happens from within a live environment so if the file got created, it sure isn't on the target fs. So I don't have a zpool.cache file, it was not created.

I'll try to force zpool.cache creation later and get back whether this solves it.

[Sam Van den Eynde (samvde)]

I created the zpool.cache file using zpool set cachefile=/etc/zfs/zpool.cache and recreated the initrd using update-initramfs -c -k all without success.

Tried creating the zpool on the system itself, and then again using the Live USB (since this shows by-id paths correctly). I double checked the zpool.cache was correctly included in the initrd.

[Sam Van den Eynde (samvde)]

Some additional tests.

If I add the zpool.cache (generated in Live environment to make sure it has full by-id paths) to /etc/zfs, and replace the zfs script with the one from 15.10 (/usr/share/initramfs-tools/scripts, requires initramfs regeneration + changing grub boot parameters to include rpool/bootfs/boot=) the system still won't use by-id paths.

If I just alter the 16.04 provided script at /usr/share/initramfs-tools/scripts (global replace "zpool import" with "zpool import -d /dev/disk/by-id") the pool comes up using by-id names as expected.

At this moment I do not have a zpool.cache file anymore.

This is a BIOS system in Virtualbox, I'm planning on doing an EFI one later today.

[Richard Laager (rlaager)]

Your testing confirms this is not a regression (or at least not one caused by my changes to the initramfs script), as I suspected. I am still doing a plain zpool import just as it did before.

[Hajo Möller (dasjoe)]

Possible upstream bug report:
https://github.com/zfsonlinux/zfs/issues/3043#issuecomment-173677425

Another upstream bug report which has been closed due to Ubuntu's inclusion of ZFS:
https://github.com/zfsonlinux/pkg-zfs/issues/148

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[Richard Laager (rlaager)]

/dev/disk/by-id isn't the only answer. Other people use other things, at least in some cases. And I don't think that using /dev/disk/by-id as a hard-coded default is acceptable. I'm pretty sure it's possible (though rare) to have drives that show up in /dev, but not /dev/disk/by-id.

Richard Yao (ryao), from upstream, pointed me to his code in Gentoo: https://gitweb.gentoo.org/proj/genkernel.git/commit/?h=zfs&id=69d703093a5a6560988d077e57ddf69303d08906

The technique there is to copy the zpool.cache file out of the pool at boot-time. An alternative would be to specify that /etc/zfs/zpool.cache should be copied into the initramfs at build-time, but then it wouldn't be regularly updated (e.g. if a disk is swapped).

I ported ryao's approach into the Ubuntu init script (patch attached for /usr/share/initramfs-tools/scripts/zfs). Coincidentally, we're already doing a read-only import anyway, but only if the rootdelay feature is being used. It's pretty straightforward to always try a read-only import.

I'm not sure that this patch should be accepted and shipped, though. Upstream seems to be generally deprecating the cache file. I feel like the correct answer is that `zpool import` should Just Work. The upstream bug 3043 seems to agree; it talks of a search path. I think the issue here is that the search path isn't work. If upstream fixes that, no changes are necessary in Ubuntu, and we get as close as possible to perfection, and it would be regression free (as it falls back to /dev if /dev/disk/by-id didn't work).

[Sam Van den Eynde (samvde)]

I agree that the solution by-id should not be hard coded, this was merely for testing purposes.

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "A patch (but note the design question here)." seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

[Richard Laager (rlaager)]

Attached is a backport of the fix from upstream. If I have a pool which was last imported using /dev/disk/by-id names, the patch causes zpool import to import it using /dev/disk/by-id names.

Note that this fix has not yet been merged upstream.

Also, note that the patch did not apply *perfectly* cleanly. In the original patch, #define DEV_BYID_PATH was moved. In my backport, it is simply added, because it did not exist. That is, the removal hunk did not apply cleanly. Likewise, a hunk which was the removal of a blank line did not apply cleanly.

The safest course of action here is to wait until it is accepted upstream and merged into the release branch (addressing the two hunks described above). Then it can be cleanly backported and SRUed.

[Richard Laager (rlaager)]

The upstream fix has been committed for 0.6.5.7:
https://github.com/zfsonlinux/zfs/commit/325414e483a7858c1d10fb30cefe5749207098f4

At this point, Ubuntu could accept my debdiff in comment #11, or wait for 0.6.5.7.

[Richard Laager (rlaager)]

@svde-tech: Can you please test from this PPA:
https://launchpad.net/~rlaager/+archive/ubuntu/zfs

Here's what I think should happen. Right now, you're seeing /dev/sdX names. If you reboot with this package, you'll still see /dev/sdX names. Reboot again and edit your GRUB command line (at boot, not permanently) and change the pool name to be wrong. When the initrd fails to import it, import it manually with: zpool import -d /dev/disk/by-id -f -R / -N POOLNAME Then export it with: zpool export POOLNAME Finally: reboot At that point, the system should come up normally.

I tested this and it works for me.

If you do test from my PPA, please remove it from your APT sources when you're done testing. I don't want some future experiment I upload to break your system.

[Sam Van den Eynde (samvde)]

No errors encountered using the PPA.

[NL (nawang81)]

My mistake it should be confirmed...