On Fri, May 18, 2018 at 12:14:36PM +0200, Theo Buehler wrote:
> On Thu, May 17, 2018 at 06:42:15PM -0600, Aaron Bieber wrote:
> > On Thu, May 17, 2018 at 06:37:56PM -0600, Aaron Bieber wrote:
> > > On Fri, Mar 02, 2018 at 07:32:04AM -0700, Aaron Bieber wrote:
> > > > Hi,
> > > >
> > > > Currently disks can only be entered in the [sw]d[0-9][0-9] format at the
> > > > "Which disk is the root disk?" prompt. This is great for humans, but
> > > > things get tricky when doing an autoinstall upgrade on systems where
> > > > connected disks change frequently.
> > > >
> > > > This diff lets you put the DUID in the response file.
> > > >
> > > > If anyone has a better way to determine the disk from the duid, I am all
> > > > ears :D
> > > >
> > > > Cheers,
> > > > Aaron
> > > >
> > >
> > > Thanks to tb@, kn@, Philipp Buehler and phy1729 on #metabug! I tested a
> > > bsd.rd with an auto-upgrade response file. Once with a duid and once
> > > with a disk name. Both worked.
> > >
> > > I have an OK from tb@ unless anyone objects.
> > >
> > > The diff has been distilled down to this:
> > >
> >
> > Paste fail, here is the latest diff:
>
> Sleeping over it once more, I must say I'm still not terribly fond of
> this hack and would like to retract my ok for these two reasons:
>
> * we clobber 'resp' which may then leave a confusing error message.
> It's only in the $AUTO case, but still.
Theo is right here. The orginal input should be preserved and used
in the error messages and should be written to the autoinstall logfile.
> * In the is_duid case we walk the hw.disknames output twice even though
> we know that we already have a good device. This is a bit stupid.
The get_rootinfo() function is not very "efficient" anyway because
it is meant to kind of dynamically detect just connected disks.
That's why get_dkdev() is already called multiple times and this
even on every iteration of the while-loop. Which also means walking
through the hw.disknames output multiple times.
It's more about reusing an existing function (get_dkdev_name) that
uses hw.disknames to extract the corresponding disk device name.
> I do think this is going in the right direction, but I'd really like to
> see these two points resolved before it goes in. It's probably not too
> difficult, but I've got too many things on my hand, so I won't be able
> to do it myself anytime soon.
My suggestion is to not special case autoinstall, but to always
use get_dkdev_name() to translate a possible supplied DUID to a
disk device name. If the input $resp was already a disk device
name, get_dkdev_name() just returns this disk device name. So in
both cases, $_dkdev holds the disk device name.
$_dkdev can then be used where a disk device name is required.
$resp can still be used for the error messages and the ai.log.
This would be the resulting diff, which looks more complex but
the change is actually simpler then the previous diff.
I've tested it sucessfully with DUIDs and disk dev names, both
interactivly and with autoinstall.
Index: install.sub
===================================================================
RCS file: /cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.1068
diff -u -p -p -u -r1.1068 install.sub
--- install.sub 29 May 2018 20:37:22 -0000 1.1068
+++ install.sub 2 Jun 2018 19:27:03 -0000
@@ -2222,7 +2222,7 @@ is_rootdisk() {
# Get global root information. ie. ROOTDISK, ROOTDEV and SWAPDEV.
get_rootinfo() {
- local _default=$(get_dkdevs)
+ local _default=$(get_dkdevs) _dkdev
local _q="Which disk is the root disk? ('?' for details)"
while :; do
@@ -2231,11 +2231,14 @@ get_rootinfo() {
case $resp in
"?") diskinfo $(get_dkdevs);;
'') ;;
- *) if isin "$resp" $(get_dkdevs); then
+ *) # Translate $resp to disk dev name in case it is a DUID.
+ # get_dkdev_name bounces back the disk dev name if not.
+ _dkdev=$(get_dkdev_name "$resp")
+ if isin "$_dkdev" $(get_dkdevs); then
[[ $MODE == install ]] && break
- is_rootdisk "$resp" && break
+ is_rootdisk "$_dkdev" && break
echo "$resp is not a valid root disk."
- _default="$(rmel "$resp" $_default) $resp"
+ _default="$(rmel "$_dkdev" $_default) $_dkdev"
else
echo "no such disk"
fi
@@ -2245,9 +2248,9 @@ get_rootinfo() {
done
log_answers "$_q" "$resp"
- make_dev $resp || exit
+ make_dev $_dkdev || exit
- ROOTDISK=$resp
+ ROOTDISK=$_dkdev
ROOTDEV=${ROOTDISK}a
SWAPDEV=${ROOTDISK}b
}
===================================================================
Stats: --- 6 lines 191 chars
Stats: +++ 9 lines 365 chars
Stats: 3 lines
Stats: 174 chars
