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.kudie...@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@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
