#2104 PR `merged`: Fix disk device name in efibootmgr call for eMMC devices¶

Labels: enhancement, fixed / solved / done

fabz5 opened issue at 2019-03-29 19:01:¶

Pull Request Details:¶

Type: Bug Fix / New Feature / Enhancement / Other?
Enhancement
Impact: Low / Normal / High / Critical / Urgent
Low
Reference to related issue (URL):
https://github.com/rear/rear/issues/2103
How was this pull request tested?
Manually on corresponding hardware (Z83-F Mini PC)
Brief description of the changes in this pull request:
For eMMC devices strip the trailing "p" in the disk device name,
so that during recovery the efibootmgr call will not fail, but create an UEFI boot entry.

jsmeix commented at 2019-04-01 07:13:¶

@fabz5
from plain looking at the code your changes look o.k. to me.

But currently it looks as if your additional conditional code
somehow belongs to the have 'mapper' in devname case.
Technically the elif makes it separated but
it doesn't make it look separated,
cf. "Code must be easy to read" in
https://github.com/rear/rear/wiki/Coding-Style

Therefore I suggest this changed code

# /dev/sda or /dev/mapper/vol34_part or /dev/mapper/mpath99p or /dev/mmcblk0p
Disk=$( echo ${Dev%$ParNr} )

# Strip trailing partition remainders like '_part' or '-part' or 'p'
# if we have 'mapper' in disk device name:
if [[ ${Dev/mapper//} != $Dev ]] ; then
    # we only expect mpath_partX  or mpathpX or mpath-partX
    case $Disk in
        (*p)     Disk=${Disk%p} ;;
        (*-part) Disk=${Disk%-part} ;;
        (*_part) Disk=${Disk%_part} ;;
        (*)      Log "Unsupported kpartx partition delimiter for $Dev"
    esac
fi

# For eMMC devices the trailing 'p' in the Disk value like /dev/mmcblk0p
# needs to be stripped, otherwise the efibootmgr call will fail
# because of a wrong disk device name. See also
# https://github.com/rear/rear/issues/2103
if [[ $Disk = *'/mmcblk'+([0-9])p ]] ; then
    Disk=${Disk%p}
fi

to keep the eMMC devices case more clearly separtated
from the 'mapper' cases and have the /dev/mmcblk0p also
in the comment at Disk=$( echo ${Dev%$ParNr} )
plus a better readable comment for the 'mapper' case.

fabz5 commented at 2019-04-02 09:40:¶

You are right of course, the coding was a bit sloppy. I guess it was too late at night when I did this. I will test your suggested changes and then recommit.