[OpenWrt-Devel] [PATCH v2 1/2] base-files: improve lib/upgrade/common.sh

[Christian Lamparter]

On Sunday, May 5, 2019 10:11:40 AM CEST Klaus Kudielka wrote:
> On 04.05.19 16:51, Christian Lamparter wrote:
> > On Friday, May 3, 2019 9:38:46 PM CEST Petr Štetiar wrote:
> >> Klaus Kudielka <klaus.kudielka at gmail.com> [2019-05-03 20:16:39]:
> >>
> >>> Let me remind you that the common one *alone* breaks sysupgrade for those
> >>> four targets, as Tomasz already pointed out earlier.
> >> Well, how could I know what was wrong with v1 if you didn't included the
> >> changes between v1 -> v2 in your v2 patch :-)
> >>
> >> Anyway, thanks for the explanation, it wasn't that much clear to me from the
> >> commit message, so if you don't mind, I'll include the details there as well
> >> in order to help it better understand to other folks.
> >>
> >> Merged into my staging tree https://git.openwrt.org/?p=openwrt/staging/ynezz.git;a=commit;h=195178f88ee7b3815f9bea66a2454ccfdf2135a5
> >>
> >>> In more detail:
> >>>
> >>> The root of the problem is that the *existing* export_bootdevice in
> >>> /lib/upgrade/common.sh behaves differently, if the kernel is booted with
> >>> root=/dev/..., or if it is booted with root=PARTUUID=...
> >>>
> >>> In the former case, it reports back major/minor of the root partition,
> >>> in the latter case it reports back major/minor of the complete boot disk.
> >>>
> >>> The targets mentioned above have added workarounds to this behaviour, by
> >>> specifying *negative* increments to the export_partdevice function.
> >>>
> >>> And then came the mvebu target to use export_bootdevice /
> >>> export_partdevice as well. Now, different subtargets boot differently,
> >>> and the workaround would be even more complex.
> >>>
> >>> I think now is the time to make export_bootdevice behave consistently,
> >>> and to report major/minor of the boot disk, period.
> > Just a note here:
> >
> > The export_bootdevice with it's PARTUUID-02 / sd[a-z]2 handling is not that
> > great. Ideally the fixed partition should be avoided altogether in favour of
> > a unique filesystem label or (less ideal) a filesystem UUID.
> >
> > Trouble is that squashfs does not support either. So that's where the fixed
> > PARTUUID and sdX/mmcX device names come into play because otherwise the devices
> > wouldn't boot.  Sadly I think changes like this will probably go on until
> > either squashfs gets these metadata image features or something replaces
> > squashfs that has it.
> >
> >>> Consequently, those targets, which boot with root=/dev/... *and* use
> >>> export_bootdevice / export_partdevice, have to be adapted to use
> >>> positive increments, otherwise they are broken by the change
> >>> to export_bootdevice.
> >>>
> >>> The targets affected were easy to spot with find & grep.
> > True, it would have been great if the commit message included that
> > export_bootdevice now consistenly works on those devices when the
> > root= in the cmdline matches that PARTUUID-02, sd[a-z]2 or mmcblk[0-9]p2
> > and nothing else.
> >
> > Because there are still a few devices (I think Gemini DIR-685, DIR-313
> > and SQ201, and a Kirkwood GoFlex Home) that have the root= on sda1 or
> > sda4 and could be converted to use the export_bootdevice for sysupgrade.
> >
> > But as of yet, I don't see that any of these devices have sysupgrade support.
> > So your proposed patch is fine, unless you want to come up with a solution
> > that can deal with the odd-balls.. Because that would be awesome!
> Well, the primary goal of this patch was (and still is) to fix sysupgrade
> for Turris Omnia, without inventing yet another workaround for the rather
> schizophrenic behaviour.
Well, from afar it really looks like your patch replaces *common*.sh code
that works for every *generic* cmdline root=/dev/* with something that
only considers root=/dev/mmcblk[0-9]p2|/dev/sd[a-z]2 .

Don't you see why this a point of contention?

> Personally I would prepare for potential future use cases in a separate
> patch. To deal with non-standard partition layouts, it could be as simple
> as replacing the trailing "2" with "[1-4]" in the six patterns of the
> $rootpart case statement... if this is what you mean?
I think it's very device specificy to say that the second partition is the
root partition. Ideally, the a uuid or fs label should be used as root=.
But in case of squashfs it's not possible, only PARTUUID or device file
name works.

Cheers,
Christian



_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel