[OpenWrt-Devel] [PATCH] upgrade: nand: fix board_name assumtions

[Jeff Kletsky]

(imgtec.com addresses removed as mail to them bounces)

On 5/20/19 6:42 AM, Jeff Kletsky wrote:

> On 5/20/19 3:14 AM, Bjørn Mork wrote:
>> nand_do_platform_check assumes that the current board name is
>> used as-is in the tar file sysupgrade directory.  This fails
>> for any image supporting multiple device names, and it also
>> fails if the board_name contains a comma.
>>
>> Signed-off-by: Bjørn Mork<bjorn at mork.no>
>> ---
>> This is a local workaround I'va had lying around for a while.  Please
>> consider if it makes any sense at all, or if there are better ways
>> to solve the problem.
>>
>> I don't have an actual upstreamed usecase, as this is an issue I've
>> met while trying to prepare for a never-finished ZyXEL WAP6805
>> support...
>>
>>
>>
>> Bjørn
>>
>>   package/base-files/files/lib/upgrade/nand.sh | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/package/base-files/files/lib/upgrade/nand.sh b/package/base-files/files/lib/upgrade/nand.sh
>> index 99916a4e96fc..14856357989e 100644
>> --- a/package/base-files/files/lib/upgrade/nand.sh
>> +++ b/package/base-files/files/lib/upgrade/nand.sh
>> @@ -320,7 +320,9 @@ nand_do_upgrade() {
>>   nand_do_platform_check() {
>>   	local board_name="$1"
>>   	local tar_file="$2"
>> -	local control_length=`(tar xf $tar_file sysupgrade-$board_name/CONTROL -O | wc -c) 2> /dev/null`
>> +	local board_dir=$(tar tf $tar_file | grep -m 1 '^sysupgrade-.*/$')
>> +	board_dir=${board_dir%/}
>> +	local control_length=`(tar xf $tar_file ${board_dir}/CONTROL -O | wc -c) 2> /dev/null`
>>   	local file_type="$(identify $2)"
>>   
>>   	[ "$control_length" = 0 -a "$file_type" != "ubi" -a "$file_type" != "ubifs" ] && {
> Your timing as good as I've been wrestling with this as well
> in context of providing ath79 support for SPI-NAND[1].
>
> At least for me, the use case is the equivalent of
>
>      nand_do_platform_check $(board_name) ${upgrade_file_name}
>
> without jumping through hoops of translating the output of $(board_name)
> to the expected tar-dir name for each and every instance of the call.
>
>
> [...]
>
> * I can see a desire to check for a *specific* tar-dir name,
>    rather than just "any" tar-dir name (wrestled with this for a while)
>
> * Only one board seems to prevent a first-comma-to-underscore approach,
>    the `img,pistachio-marduk` uses `sysupgrade-img,pistachio-marduk`
>
>   [...]

Having spent quite a bit of time today thinking about the impact of
board-name changes, I can see use cases where the running system
doesn't know a priori what the range of acceptable tar-dir values are.

Take the case where `mfgr,board-name` gets a new, compatible variant,
`mfgr,board-name-special-purpose`. If you're running `mfgr,board-name`,
a strict check for `mfgr_board-name` would fail, even though the
sysupgrade-tar was applicable. This kind of board-compatibility check
should arguably be done with image metadata, either or both appended
or in the CONTROL section.

As such, I would welcome this change, as proposed by Bjørn.

I would suggest an update to the comment to reflect the change as
`has to include "sysupgrade-BOARD" directory` is no longer correct.




Jeff
    

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/openwrt-devel/attachments/20190520/6f24736c/attachment.htm>
-------------- next part --------------
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel