> On Jun 2, 2018, at 3:27 PM, Robert Peichaer <rob...@peichaer.org> wrote:
>
>> 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
I like this spproach better. ok krw@
.... Ken
