Re: [gentoo-dev] [PATCH] mount-boot.eclass: Check if /boot is sane, but don't try to mount it.

[Ulrich Mueller <ulm@gentoo.org>]

[-- Attachment #1: Type: text/plain, Size: 1601 bytes --]

>>>>> On Fri, 06 Dec 2019, Michał Górny wrote:

>> reaching pkg_pretend() at all. Furthermore, with the "mount-sandbox"

> Did you mean: pkg_preinst?

Of course.

>> -	if [[ "${ROOT:-/}" != "/" ]] ; then
>> +	if [[ ${ROOT:-/} != "/" ]] ; then

> I suppose you can unquote RHS too since it doesn't contain any pattern
> characters, if you are already touching quoting.

Quotes dropped (globally).

>> +# Check if /boot is sane, i.e., mounted read/write if on a separate
>> +# partition.  Return 0 if conditions are fulfilled, otherwise die.

> I don't think there's a point in explicitly defining the return value
> if there is no alternative.

Fixed.

>> +	local fstabstate=$(awk '!/^[[:blank:]]*#|^\/dev\/BOOT/ && $2 == "/boot" \
>> +		{print $2}' /etc/fstab)

> The 'print' here is used as a boolean... why not use a boolean output
> instead?

Changed, it's printing a 1 now.

>> +	local procstate=$(awk '$2 == "/boot" \
>> +		{print gensub(/^(.*,)?(ro|rw)(,.*)?$/, "\\2", 1, $4)}' /proc/mounts)

> Shouldn't this use /proc/self/mounts?

I haven't actually changed this, it was /proc/mounts before (which is a
symlink to self/mounts). Maybe we should keep it as-is for backwards
compatibility? (But TBH, I have no idea with which kernel version
/proc/self/mounts appeared.)

>> +	mount-boot_check_status
>> +	if ! ( shopt -s failglob; : "${ROOT}"/boot/.keep* ) 2>/dev/null ; then

> EROOT?

Hm, we mount /boot, not ${EROOT}/boot. I've added an additional test for
[[ ${EROOT:-/} == / ]] before touching the file.

Patch v2 will follow.

Ulrich

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]