[PATCH] distro_boot: Fix bootfile env after calling boot_extlinux

[Art Nikpal]

> could you check out my syslinux refactor
> series? It could use some review.
>
> http://patchwork.ozlabs.org/project/uboot/list/?series=264265

Yes ! But I already can't apply this series for actual uboot code
(need to change)
maybe u can make v3 patches for actual uboot state.

I checked it quickly. This series has many useful improvements!

> Here's the patch where (I believe) this problem was addressed:
> http://patchwork.ozlabs.org/project/uboot/patch/20210927092350.v2.11.I6a01e73ef114448e39cb6899c21f6c169e4da216@changeid/

look like yes and  if your series will be applied my patch already no need.

On Thu, Oct 14, 2021 at 12:58 AM Simon Glass <sjg at chromium.org> wrote:
>
> Hi Art,
>
> On Tue, 12 Oct 2021 at 21:45, Art Nikpal <email2tema at gmail.com> wrote:
> >
> > Yes changes inside include/config_distro_bootcmd.h not the best solution for this issue.
> > I think it is better to change sysboot cmd and i have prepared another solution already!
> > https://patchwork.ozlabs.org/project/uboot/patch/20211013033912.3297227-1-art@khadas.com/
> > how about this ?
>
> Seem reasonable to me, but could you check out my syslinux refactor
> series? It could use some review.
>
> http://patchwork.ozlabs.org/project/uboot/list/?series=264265
>
>
> - Simon
>
> >
> > On Wed, Oct 13, 2021 at 5:07 AM Tom Rini <trini at konsulko.com> wrote:
> >>
> >> On Tue, Oct 12, 2021 at 02:31:18PM -0600, Simon Glass wrote:
> >> > Hi Tom,
> >> >
> >> > On Tue, 12 Oct 2021 at 13:44, Tom Rini <trini at konsulko.com> wrote:
> >> > >
> >> > > On Tue, Oct 12, 2021 at 04:55:44PM +0800, Artem Lapkin wrote:
> >> > >
> >> > > > Problem
> >> > > >
> >> > > > PXE cannot boot normally after Sysboot changed the bootfile env (called
> >> > > > from boot_extlinux) from the default "boot.scr.uimg" to
> >> > > > "/boot/extlinux/extlinux.conf".
> >> > > >
> >> > > > In addition, an unbootable extlinux configuration will also make the PXE
> >> > > > boot unbootable, because it will use the incorrect path "/boot/extlinux/"
> >> > > > from the bootfile env.
> >> > > >
> >> > > > Solution
> >> > > >
> >> > > > Save and restore default bootfile env value when boot_extlinux is used.
> >> > > >
> >> > > > Example
> >> > > > ================================================================
> >> > > > Hit SPACE in 2 seconds to stop autoboot
> >> > > > ... is now current device
> >> > > > Found /boot/extlinux/extlinux.conf
> >> > > > Retrieving file: /boot/extlinux/extlinux.conf
> >> > > > 413 bytes read in 2 ms (201.2 KiB/s)
> >> > > > Skipping Krescue for failure retrieving kernel
> >> > > > SCRIPT FAILED: continuing...
> >> > > > ...
> >> > > > Speed: 1000, full duplex
> >> > > > BOOTP broadcast 1
> >> > > > DHCP client bound to address 192.168.11.151 (8 ms)
> >> > > > Using ethernet at ff3f0000 device
> >> > > > TFTP from server 192.168.11.1; our IP address is 192.168.11.151
> >> > > > Filename '/boot/extlinux/pxelinux.cfg/default'.
> >> > > > Not retrying...
> >> > > > ================================================================
> >> > > >
> >> > > > Signed-off-by: Artem Lapkin <art at khadas.com>
> >> > > > ---
> >> > > >  include/config_distro_bootcmd.h | 2 ++
> >> > > >  1 file changed, 2 insertions(+)
> >> > > >
> >> > > > diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h
> >> > > > index 3f724aa10f..db3d1b2362 100644
> >> > > > --- a/include/config_distro_bootcmd.h
> >> > > > +++ b/include/config_distro_bootcmd.h
> >> > > > @@ -445,7 +445,9 @@
> >> > > >                               "${devnum}:${distro_bootpart} "           \
> >> > > >                               "${prefix}${boot_syslinux_conf}; then "   \
> >> > > >                       "echo Found ${prefix}${boot_syslinux_conf}; "     \
> >> > > > +                     "bootfile_bak=${bootfile}; "                      \
> >> > > >                       "run boot_extlinux; "                             \
> >> > > > +                     "setenv bootfile ${bootfile_bak}; "               \
> >> > > >                       "echo SCRIPT FAILED: continuing...; "             \
> >> > > >               "fi\0"                                                    \
> >> > > >       \
> >> > >
> >> > > We've had this kind of problem before, and the answer is that variables
> >> > > should be local, not global in scope.  In this case, I see that the way
> >> > > the pxe/sysboot code works is that we have to env_set("..") in one place
> >> > > to env_get("..") in another, so I don't see a way around this.
> >> > >
> >> > > Reviewed-by: Tom Rini <trini at konsulko.com>
> >> >
> >> > IMO a better approach will be the bootflow implementation. I hope to
> >> > get v2 out early next week.
> >>
> >> I'm not sure if the bootflow way of going here would or would not have
> >> the same problem, or perhaps a slightly different problem.  At heart
> >> here, "sysboot" calls env_set(...) and then calls in to the pxe code
> >> which does env_get(...).  So now I wonder how this would be fixed in
> >> bootflow, since we aren't dealing with the environment directly.
> >>
> >> --
> >> Tom