#2851 PR `merged`: insert missing prefix to fix #2850¶

Labels: fixed / solved / done, minor bug

justinotherguy opened issue at 2022-08-12 13:05:¶

Pull Request Details:¶

Type: Bug Fix
Impact: Normal
Reference to related issue (URL): #2850
How was this pull request tested? make rpm; install on 2 LPARs; rear mkrescue
Brief description of the changes in this pull request:
adds the missing prefix to the symlink so it is found during tftp boot

jsmeix commented at 2022-09-06 10:04:¶

I am not a PXE user so I cannot test it myself.
But from plain looking at the code in
output/PXE/default/810_create_pxelinux_cfg.sh
I do not understand how pxe_link_prefix is used therein.

Code excerpts with the change of this pull request
and with better readable indentation here:

# When using Grub network boot via tftp/bootp,
# the client is looking at a file named "grub.cfg-01-<MAC>"
# or grub.cfg-<IP in hex>. It is like PXE, but prefixed with "grub.cfg-"
if is_true $PXE_CONFIG_GRUB_STYLE ; then
    pxe_link_prefix="grub.cfg-"
else
    pxe_link_prefix=""
fi

case "$PXE_CREATE_LINKS" in
    IP)
...
        if has_binary gethostip &>/dev/null ; then
            ln -sf $v "$PXE_CONFIG_FILE" $(gethostip -x $IP) >&2
            # to capture the whole subnet as well
            ln -sf $v "$PXE_CONFIG_FILE" $(gethostip -x $IP | cut -c 1-6) >&2
        else
            # if gethostip is not available on your platform (like ppc64),
            # use awk to generate IP in hex mode.
            ln -sf $v "$PXE_CONFIG_FILE" $pxe_link_prefix$(printf '%02X' ${IP//./ }) >&2
            # to capture the whole subnet as well
            ln -sf $v "$PXE_CONFIG_FILE" $pxe_link_prefix$(printf '%02X' ${IP//./ } | cut -c 1-6) >&2
        fi
...
    MAC)
        # look at all devices that have link/ether
        ip l | grep link/ether | \
            while read link mac junk ; do
                ln -sf $v "$PXE_CONFIG_FILE" ${pxe_link_prefix}01-${mac//:/-} >&2
            done

I do not understand why $pxe_link_prefix is not prepended
for the case

        if has_binary gethostip &>/dev/null ; then
            ln -sf $v "$PXE_CONFIG_FILE" $(gethostip -x $IP) >&2
            # to capture the whole subnet as well
            ln -sf $v "$PXE_CONFIG_FILE" $(gethostip -x $IP | cut -c 1-6) >&2

Shouldn't that case also be like

        if has_binary gethostip &>/dev/null ; then
            ln -sf $v "$PXE_CONFIG_FILE" $pxe_link_prefix$(gethostip -x $IP) >&2
            # to capture the whole subnet as well
            ln -sf $v "$PXE_CONFIG_FILE" $pxe_link_prefix$(gethostip -x $IP | cut -c 1-6) >&2

jsmeix commented at 2022-09-06 10:08:¶

FYI:
PXE_CONFIG_GRUB_STYLE support was added via
https://github.com/rear/rear/pull/1339
by
https://github.com/rear/rear/commit/ac6cc5acc830c93bf10a13a3a061b19bcad1d151

jsmeix commented at 2022-09-06 10:13:¶

@gdha @pcahyna
if you think $pxe_link_prefix should be also prepended
for the if has_binary gethostip ... case
I would first merge this pull request as is
and then add the missing $pxe_link_prefix prepend
for the if has_binary gethostip ... case
in a separated commit.

jsmeix commented at 2022-09-06 14:51:¶

@rear/contributors
if there are no objections
I would like to merge it as is tomorrow afternoon
and then add the missing $pxe_link_prefix prepend
for the if has_binary gethostip ... case
via a separated pull request.

jsmeix commented at 2022-09-07 12:13:¶

@justinotherguy
thank you for contribution to ReaR that fixes a bug!

Contributions for unusual/special use cases in ReaR
are much appreciated because we at ReaR upstream
do not have the time to try out all possible use cases in ReaR
so we basically totally depend on contributions from users
who use ReaR with unusual/special use cases.

jsmeix commented at 2022-09-07 12:26:¶

The follow-up pull request according to
https://github.com/rear/rear/pull/2851#issuecomment-1237939248
is
https://github.com/rear/rear/pull/2857

[Export of Github issue for rear/rear.]

#2851 PR merged: insert missing prefix to fix #2850¶