zfs-initramfs on debian: Failures and no luck with overlapping snapshot names

[cfellinger]

running debian jessie, zfs-intramfs-0.6.5.6-5, when booting from zfs with a snapshot name that matches other snapshot names, like ONE matching both snapshot ONE and snapshot ONETWO, the booting fails

but when I apply the simple patch of anchoring the grep (head and tail) as in....

--- /etc/zfs/zfs-functions      2016-04-17 22:05:33.000000000 +0200
+++ /target/etc/zfs/zfs-functions       2016-04-18 01:43:14.844678633 +0200
@@ -1096,7 +1096,7 @@
        # Get all snapshots, recursivly (might need to clone /usr, /var etc
        # as well).
        "${ZFS}" list -H -oname -tsnapshot -r "${rootfs}" | \
-               grep "${snapname}" | \
+               grep "@${snapname}$" | \
        while read s
        do
                if grep -qiE '(^|[^\\](\\\\)* )(rollback)=(on|yes|true|1)( |$)' /proc/cmdline

see patch file patch-snapname-match.txt

....all is swell as can be seen in the following screenshot

[FransUrbo]

This is fixed in the new v0.6.5.6-6 which is on its way up to the repository.

[cfellinger]

yep it works, thanks. But looking at the code I'm surprised you use the -E switch to grep. If any I would expect the -F switch preventing anything in the snapname (e.ge. a simple dot) to get interpreted as a pattern and hence as a potential source for multi matching again. helas the -F wouldn't work (anchers unknown), so -G (the default) it must be. leaving only ? and * as potential multi matching patters, but I suppose those don't get used as easily as a simple dot (.)

[FransUrbo]

Good point! I'll update that in the next release (I'm not going to do one specifically for this though).

[cfellinger]

Ive done some testing and even -G would play havoc with snap.shot as shown here:

I propose the following patch:

--- /etc/zfs/zfs-functions      2016-04-18 14:19:49.000000000 +0200
+++ /target/etc/zfs/zfs-functions       2016-04-19 15:01:57.331893769 +0200
@@ -1153,9 +1153,9 @@
        # Get all snapshots, recursivly (might need to clone /usr, /var etc
        # as well).
        "${ZFS}" list -H -oname -tsnapshot -r "${rootfs}" | \
-               grep -E "@${snapname}$" | \
        while read s
        do
+               [ "x${s##*@}" = "x${snapname}" ] || continue
                if grep -qiE '(^|[^\\](\\\\)* )(rollback)=(on|yes|true|1)( |$)' /proc/cmdline
                then
                        # Rollback snapshot

for your convenience: patch-snapname-match.txt

wich seems to work as can be seen here:

PS: the x in the string comparisson is an old shell trick, i'm not sure it's still needed.

[FransUrbo]

@@ -1153,9 +1153,9 @@

Get all snapshots, recursivly (might need to clone /usr, /var etc

    # as well).
    "${ZFS}" list -H -oname -tsnapshot -r "${rootfs}" | \

• grep -E "@${snapname}$" | \ while read s do
• [ "x${s##@}" = "x${snapname}" ] || continue if grep -qiE '(^|^\ )(rollback)=(on|yes|true|1)( |$)' /proc/cmdline then

  Rollback snapshot

So what's wrong with the normal grep instead of the 'grep -E'? And why the continue line instead of grep'ing for it before we start the while loop?

[cfellinger]

plain grep gives raise to the above failure example (or so I thought, if you want to I will retest to be sure), it surprised me, I expected only ? and $ to be special. In any case doing the test as done in my proposed fix will trully only ever match when the snapshot names are equal, no special chars and no regexp there. So it seems more correct and more robust.

[FransUrbo]

So it seems more correct and more robust.

Ok, fair enough. I'll have to take your word for it, because I don't have [edit: time] to dig to deep in this. I'll use your latest suggestion next time then. Thanx for all the help in this!