[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: ZFS boot environment patch
From: |
Colin Watson |
Subject: |
Re: ZFS boot environment patch |
Date: |
Wed, 13 Sep 2017 21:17:42 +0100 |
User-agent: |
NeoMutt/20170113 (1.7.2) |
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