Re: ZFS boot environment patch

[Colin Watson]

On Wed, Sep 13, 2017 at 11:02:48AM -0400, Konrad Rzeszutek Wilk wrote:
> On Fri, Sep 01, 2017 at 01:31:28PM +0200, Paul Lagerweij wrote:
> > +zfs_be="0"
> 
> Why not just
> 
> zfs_be=
> 
> And then you can just check for zfs_be having an value instead of for 1 or 0?
> 
> Or alternatively,
> 
> zfs_be=0
> 
> and then you can do if [ $zfs_be -ne 0 ]; .. or such?

If you're going to do the latter, it doesn't matter whether you assign
it as "0" or 0.  Shell variables are untyped.

I think this is excessively nitpicky though.  There's no single way to
do boolean shell variables that everyone agrees with, and [ "$foo" = 1 ]
is a perfectly reasonable idiomatic choice.  There's very little to
choose between that and the other options you suggest, making it a
purely stylistic question.  (I've been known to do 'foo=false' or
'foo=:' and then 'if "$foo"; then ...', but a lot of people have an
allergic reaction to that one.)

> > @@ -62,9 +63,15 @@ case x"$GRUB_FS" in
> >     fi;;
> >      xzfs)
> >     rpool=`${grub_probe} --device ${GRUB_DEVICE} --target=fs_label 
> > 2>/dev/null || true`
> > -   bootfs="`make_system_path_relative_to_its_root / | sed -e "s,@$,,"`"
> > -   LINUX_ROOT_DEVICE="ZFS=${rpool}${bootfs}"
> > -   ;;
> > +   zfs_active_bootfs="`zpool list -H -o bootfs ${rpool} || true`"
> 
> Is zpool usually in /sbin or such? Perhaps a full path?

Full paths are brittle when they refer to something installed by a
different package.  If you need to do that kind of thing then it's
usually better to temporarily extend $PATH instead.

(I'm slightly surprised that the whole edifice of grub-mkconfig doesn't
currently appear to touch anything in /sbin or /usr/sbin, or if it does
it doesn't seem to have any particular handling for it.)

-- 
Colin Watson                                       address@hidden