Re: [PATCH V3 2/3] base-files: sysupgrade: use tar helper to include installed_packages.txt

2024-02-29 Thread Luiz Angelo Daros de Luca
>
> Replace mount + overlay with manually built tar archive that gets
> prepended to the actual config files backup. This allows more
> flexibility with including extra backup files. They can be included at
> any paths and don't require writing to flash or mounting an overlay
> which has its own limitations (mount points).
>
> Signed-off-by: Rafał Miłecki 
> ---
>  package/base-files/files/sbin/sysupgrade | 44 +---
>  1 file changed, 16 insertions(+), 28 deletions(-)
>
> diff --git a/package/base-files/files/sbin/sysupgrade 
> b/package/base-files/files/sbin/sysupgrade
> index 6b3fb0666f..a11e17615c 100755
> --- a/package/base-files/files/sbin/sysupgrade
> +++ b/package/base-files/files/sbin/sysupgrade
> @@ -237,8 +237,6 @@ include /lib/upgrade
>  create_backup_archive() {
> local conf_tar="$1"
>
> -   local umount_etcbackup_dir=0
> -
> [ "$(rootfs_type)" = "tmpfs" ] && {
> echo "Cannot save config while running from ramdisk." >&2
> ask_bool 0 "Abort" && exit
> @@ -248,41 +246,31 @@ create_backup_archive() {
> run_hooks "$CONFFILES" $sysupgrade_init_conffiles
> ask_bool 0 "Edit config file list" && vi "$CONFFILES"
>
> -   if [ "$SAVE_INSTALLED_PKGS" -eq 1 ]; then
> -   echo "${INSTALLED_PACKAGES}" >> "$CONFFILES"
> -   mkdir -p "$ETCBACKUP_DIR"
> -   # Avoid touching filesystem on each backup
> -   RAMFS="$(mktemp -d -t sysupgrade.XX)"
> -   mkdir -p "$RAMFS/upper" "$RAMFS/work"
> -   mount -t overlay overlay -o 
> lowerdir=$ETCBACKUP_DIR,upperdir=$RAMFS/upper,workdir=$RAMFS/work 
> $ETCBACKUP_DIR &&
> -   umount_etcbackup_dir=1 || {
> -   echo "Cannot mount '$ETCBACKUP_DIR' as tmpfs 
> to avoid touching disk while saving the list of installed packages." >&2
> -   ask_bool 0 "Abort" && exit
> -   }
> -
> -   # Format: pkg-name{rom,overlay,unknown}
> -   # rom is used for pkgs in /rom, even if updated later
> -   find /usr/lib/opkg/info -name "*.control" \( \
> -   \( -exec test -f /rom/{} \; -exec echo {} rom \; \) 
> -o \
> -   \( -exec test -f /overlay/upper/{} \; -exec echo {} 
> overlay \; \) -o \
> -   \( -exec echo {} unknown \; \) \
> -   \) | sed -e 's,.*/,,;s/\.control /\t/' > 
> ${INSTALLED_PACKAGES}
> -   fi
> -
> v "Saving config files..."
> [ "$VERBOSE" -gt 1 ] && TAR_V="v" || TAR_V=""
> sed -i -e 's,^/,,' "$CONFFILES"
> -   tar c${TAR_V}zf "$conf_tar" -C / -T "$CONFFILES"
> +   {
> +   # Part of archive with installed packages info
> +   if [ "$SAVE_INSTALLED_PKGS" -eq 1 ]; then
> +   # Format: pkg-name{rom,overlay,unknown}
> +   # rom is used for pkgs in /rom, even if updated later
> +   tar_print_member "$INSTALLED_PACKAGES" "$(find 
> /usr/lib/opkg/info -name "*.control" \( \
> +   \( -exec test -f /rom/{} \; -exec echo {} rom 
> \; \) -o \
> +   \( -exec test -f /overlay/upper/{} \; -exec 
> echo {} overlay \; \) -o \
> +   \( -exec echo {} unknown \; \) \
> +   \) | sed -e 's,.*/,,;s/\.control /\t/')"
> +   fi

I don't like the idea of running tar_print_member without checking for
errors. This is even more important for the following tar that
includes files specified by the user. In my opinion, it should abort
the process if tar fails.
As ash cannot read the pipe exit status, you could print the error and
touch a /tmp/${mypid}_abort, checking for it after the gzip to abort
the process (and erase the error flag file).

> +
> +   # Rest of archive with config files and ending padding
> +   tar c${TAR_V} -C / -T "$CONFFILES"
> +   } | gzip > "$conf_tar"
> +
> local err=$?
> if [ "$err" -ne 0 ]; then
> echo "Failed to create the configuration backup."
> rm -f "$conf_tar"
> fi
>
> -   [ "$umount_etcbackup_dir" -eq 1 ] && {
> -   umount "$ETCBACKUP_DIR"
> -   rm -rf "$RAMFS"
> -   }
> rm -f "$CONFFILES"
>
> return "$err"
> --
> 2.35.3
>

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH V3 1/3] base-files: sysupgrade: add tar.sh with helpers for building archives

2024-02-29 Thread Luiz Angelo Daros de Luca
Hello Rafał,

Sorry for the late review.

I know this will be used for a limited context but it can be
constructed in such a way it could be used for more cases without
increasing its complexity.

> From: Jo-Philipp Wich 
>
> This allows building uncompressed tar archives from shell scripts (and
> compressing them later if needed)
>
> Signed-off-by: Rafał Miłecki 
> ---
> V2: Simplify dd in __tar_print_padding (I still think helper is useful)
> Hardcode 0/0/ root/root for now as most likely it'll be enough
> Simplify name validation (leasing slash)
> Reorder some variables
> V3: Fix dd in __tar_print_padding
> Rename functions
> Drop unused functions
> Document usage
>
>  package/base-files/files/lib/upgrade/tar.sh | 71 +
>  1 file changed, 71 insertions(+)
>  create mode 100644 package/base-files/files/lib/upgrade/tar.sh
>
> diff --git a/package/base-files/files/lib/upgrade/tar.sh 
> b/package/base-files/files/lib/upgrade/tar.sh
> new file mode 100644
> index 00..a9d1d559e6
> --- /dev/null
> +++ b/package/base-files/files/lib/upgrade/tar.sh
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> +
> +# Example usage:
> +#
> +# {
> +# tar_print_member "date.txt" "It's $(date +"%Y")"
> +# tar_print_trailer
> +# } > test.tar
> +
> +__tar_print_padding() {
> +   dd if=/dev/zero bs=1 count=$1 2>/dev/null
> +}
> +
> +tar_print_member() {
> +   local name="$1"
> +   local content="$2"
> +   local mtime="${3:-$(date +%s)}"
> +   local mode=644
> +   local uid=0
> +   local gid=0
> +   local size=${#content}
> +   local type=0
> +   local link=""
> +   local username="root"
> +   local groupname="root"
> +
> +   # 100 byte of padding bytes, using 0x01 since the shell does not 
> tolerate null bytes in strings
> +   local 
> pad=$'\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1'

\1 is still a valid filename character. Shell cannot save a string
with \0 but printf can handle it nicely (as well as pipes). If you
build the header using a function at the moment you use it, you could
avoid the \1 hack.

print_header() {
   local filename=$1
   local mode=$2
   ...
   local checksum=$7
   ...
   printf '%s' "$filename"; printf '\x00%.0s' $(seq $((100 - ${#filename})))
   ...
}

print_header "$filename" "$mode" \
   "$uid" "$gid" .. | hexdump -ve '1/1 "%u

If you don't like the printf trick to repeat a char, you could use
__tar_print_padding (or use printf for both).

You could call it twice, first without a checksum to calculate it and
then a new one to generate the final output.

> +
> +   # validate name (strip leading slash if present)
> +   name=${name#/}
> +
> +   # truncate string header values to their maximum length
> +   name=${name:0:100}

It feels strange to simply truncate the name. If you need to truncate
it, you are implying it might be larger. In that case, it should fail
if it cannot fit a specific filename. Otherwise, you could end up
overwriting two filenames with the same first 100 bytes. And BTW,
shouldn't ustar support 255 or 256 filenames?

> +   link=${link:0:100}
> +   username=${username:0:32}
> +   groupname=${groupname:0:32}
> +
> +   # construct header part before checksum field
> +   local header1="${name}${pad:0:$((100 - ${#name}))}"
> +   header1="${header1}$(printf '%07d\1' $mode)"
> +   header1="${header1}$(printf '%07o\1' $uid)"
> +   header1="${header1}$(printf '%07o\1' $gid)"
> +   header1="${header1}$(printf '%011o\1' $size)"
> +   header1="${header1}$(printf '%011o\1' $mtime)"
> +
> +   # construct header part after checksum field
> +   local header2="$(printf '%d' $type)"
> +   header2="${header2}${link}${pad:0:$((100 - ${#link}))}"
> +   header2="${header2}ustar  ${pad:0:1}"
> +   header2="${header2}${username}${pad:0:$((32 - ${#username}))}"
> +   header2="${header2}${groupname}${pad:0:$((32 - ${#groupname}))}"

It seems to be a strange ustar format. Shouldn't it have an ustar
version before the username? And Device major/minor after groupname,
followed by more 150 filename prefix? It might still work if the
reader interprets it as a v7.

> +   # calculate checksum over header fields
> +   local checksum=0
> +   for byte in $(printf '%s%8s%s' "$header1" "" "$header2" | tr '\1' 
> '\0' | hexdump -ve '1/1 "%u "'); do
> +   checksum=$((checksum + byte))
> +   done
> +

Maybe use a single line like:

checksum=$(print header... | hexdump -ve '1/1 "%u\n"' | awk '{s+=$1}
END {print s}')

?

> +   # print member header, padded to 512 byte
> +   printf '%s%06o\0 %s' "$header1" $checksum "$header2" | tr '\1' '\0'
> +   __tar_print_padding 183
> +
> +   # print content data, 

Re: [OpenWrt-Devel] [PATCH 2/5] build: image: Add pad-to and pad-rootfs-squashfs helpers

2024-02-29 Thread Nishant Sharma

Hi,

Bumping this thread after years.

On 01/04/19 11:03, Jo-Philipp Wich wrote:

I would like to avoid adding generating padded images by default.



I just came up with this simple script, which takes an existing image and
pads it to full size: http://nbd.name/pad-image.pl With this, shipping
padded images should be unnecessary.


Ok, if that is preferred, it's fine with me. But we should probably add some
note somewhere, that in order to test this images in QEMU (x86, armvirt,
malta), the images should be padded with this script in order to provide
usable images.


Most other targets ship image artifacts which are usable ootb, requiring
one extra step to pad the combined images is a waste of user resources
every single time. It also causes recurring confusion among users
wanting to use x86 builds since the need for padding is neither
documented, nor obvious while a combined.img.gz makes it obvious that it
is an image file which requires decompression.

I really don't see the huge problem with conservatively padding the
combined image artifacts to something like 32 or 48MB, it must not even
be 256M or more.


I build large squashfs images (16GB, 32GB) for x86_64 running on Xeon 
processors for high workload gateways running VPN, Squid, Snort etc.


Till OpenWrt 19.07, I was able to build 32GB images on a machine with 
only 8GB of RAM and 8GB of swap. sysupgrade also worked fine on those 
images.


But since 21.02, I am getting the *memory exhausted* error.

Below is the build log snippet for 23.05.2:



dd 
if=/home/devuser/auto-build/hopbox-os/hopbox-openwrt/build_dir/target-x86_64_musl/linux-x86_64/root.squashfs 
>> 
/home/devuser/auto-build/hopbox-os/hopbox-openwrt/build_dir/target-x86_64_musl/linux-x86_64/tmp/hopbox-arthur-23.05.2.1-x86-64-generic-squashfs-rootfs.img.gz


39891+1 records in
39891+1 records out
20424365 bytes (20 MB, 19 MiB) copied, 0.0445292 s, 459 MB/s

dd 
if=/home/devuser/auto-build/hopbox-os/hopbox-openwrt/build_dir/target-x86_64_musl/linux-x86_64/tmp/hopbox-arthur-23.05.2.1-x86-64-generic-squashfs-rootfs.img.gz 
of=/home/devuser/auto-build/hopbox-os/hopbox-openwrt/build_dir/target-x86_64_musl/linux-x86_64/tmp/hopbox-arthur-23.05.2.1-x86-64-generic-squashfs-rootfs.img.gz.new 
bs=16642998272 conv=sync


dd: memory exhausted by input buffer of size 16642998272 bytes (16 GiB)



Here the block size is set to 16GiB to pad rootfs and there are not 
enough system resources available on the build host to be able honour that.


I don't see any special case to handle x86/x86_64 in image-commands.mk

Is there a way I can disable padding for x86_64?

Thanks in advance.

Regards,
Nishant

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH V3 3/3] base-files: sysupgrade: add uci-defaults script disabling services #2

2024-02-29 Thread Jo-Philipp Wich

Hi,


[...]
Extend sysupgrade to check for disabled services, generate uci-defaults
script disabling them and include it in backup.

Cc: Christian Marangi 
Cc: Jo-Philipp Wich 
Cc: Jonas Gorski 
Signed-off-by: Rafał Miłecki 


Acked-by: Jo-Philipp Wich 

~ Jo

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH V3 2/3] base-files: sysupgrade: use tar helper to include installed_packages.txt

2024-02-29 Thread Jo-Philipp Wich

Hi,


[...]
Replace mount + overlay with manually built tar archive that gets
prepended to the actual config files backup. This allows more
flexibility with including extra backup files. They can be included at
any paths and don't require writing to flash or mounting an overlay
which has its own limitations (mount points).

Signed-off-by: Rafał Miłecki 


Acked-by: Jo-Philipp Wich 

~ Jo

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH V3 1/3] base-files: sysupgrade: add tar.sh with helpers for building archives

2024-02-29 Thread Jo-Philipp Wich

Hi,

[...] 
This allows building uncompressed tar archives from shell scripts (and

compressing them later if needed)

Signed-off-by: Rafał Miłecki 


Signed-off-by: Jo-Philipp Wich 


~ Jo

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


RE: ubus running key status

2024-02-29 Thread Ravi Paluri (QUIC)
Thanks, Paul for the reply.
Can you help us with a sample usage of "ubus subscribe" OR "ubus listen" OR 
"ubus monitor", which helps us to learn, "running" flag change to false from 
true.

We are polling the status of "running" flag as below.
ubus call service list '{"name":"test*.init","verbose":"True"}' | grep -i 
\"running\"

This is working when script execution takes some time.
For some scripts that take very less time (for e.g, a single instruction such 
as "uci set .."), we are not able to detect the transition of "running" flag to 
false from true as the flag changes it's value quickly.

Thanks,
Ravi
-Original Message-
From: openwrt-devel  On Behalf Of Paul 
D
Sent: Friday, February 23, 2024 7:35 PM
To: openwrt-devel@lists.openwrt.org
Subject: Re: ubus running key status

WARNING: This email originated from outside of Qualcomm. Please be wary of any 
links or attachments, and do not enable macros.

On 2024-02-23 13:43, Ravi Paluri (QUIC) wrote:
> Can you let us know, is there a way to get a notification when the "running" 
> key value moves to "false" from "true"?
> OR is there any ubus API to which we can register a callback, which will then 
> be invoked when the execution is complete?

Not certain about callbacks - but there is "ubus monitor" "ubus subscribe" 
"ubus listen" and "ubus wait_for"

They're blocking calls, at least.



___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


RE: ubus running key status

2024-02-29 Thread Ravi Paluri via openwrt-devel
The sender domain has a DMARC Reject/Quarantine policy which disallows
sending mailing list messages using the original "From" header.

To mitigate this problem, the original message has been wrapped
automatically by the mailing list software.--- Begin Message ---
Thanks, Paul for the reply.
Can you help us with a sample usage of "ubus subscribe" OR "ubus listen" OR 
"ubus monitor", which helps us to learn, "running" flag change to false from 
true.

We are polling the status of "running" flag as below.
ubus call service list '{"name":"test*.init","verbose":"True"}' | grep -i 
\"running\"

This is working when script execution takes some time.
For some scripts that take very less time (for e.g, a single instruction such 
as "uci set .."), we are not able to detect the transition of "running" flag to 
false from true as the flag changes it's value quickly.

Thanks,
Ravi
-Original Message-
From: openwrt-devel  On Behalf Of Paul 
D
Sent: Friday, February 23, 2024 7:35 PM
To: openwrt-devel@lists.openwrt.org
Subject: Re: ubus running key status

WARNING: This email originated from outside of Qualcomm. Please be wary of any 
links or attachments, and do not enable macros.

On 2024-02-23 13:43, Ravi Paluri (QUIC) wrote:
> Can you let us know, is there a way to get a notification when the "running" 
> key value moves to "false" from "true"?
> OR is there any ubus API to which we can register a callback, which will then 
> be invoked when the execution is complete?

Not certain about callbacks - but there is "ubus monitor" "ubus subscribe" 
"ubus listen" and "ubus wait_for"

They're blocking calls, at least.



___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

--- End Message ---
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


[PATCH RFC/PoC] scripts: {package-}metadata: use 'SUBMENU' as subcategory path

2024-02-29 Thread Piotr Dymacz
Motivation for this change is to allow for a more nested menu structure
when manually configuring build system with 'make {menu,n,x}config'.

The current solution provides only 2 levels of nesting (without using
external/custom config/Config.in):
  - top level menu/category (Makefile variable 'CATEGORY')
  - 2nd level sub menu/category (Makefile variable 'SUBMENU')

To allow placing packages in a more nested menu structure, the 'SUBMENU'
variable is treated as a menu path with '\' character as separator*.

Defining 'SUBMENU' as 'Foo\Bar\Qux' for package 'Bob' and 'Foo\Bar' for
package 'Ben' with this change would create menu structure as follows:

  top level menu 'CATEGORY'  --->
  ..menu 'Foo'  --->
  menu 'Bar'  --->
  ..menu 'Qux'  --->
  [] package 'Bob'
  ..[] package 'Ben'

Currently existing menus and packages sorting methods were preserved
(menu first, packages below, alphabetical order) and the only difference
applies to the usage of 'SUBMENUDEP' Makefile variable which will affect
only last path element (the one package is placed under).

* The backslash instead of a more natural slash '/' was selected due to
  the fact that some of already existing core and community packages use
  slash in 'SUBMENU' variables. As this is more a RFC/PoC, final change
  might use different character or approach for defining menu path.

Signed-off-by: Piotr Dymacz 
---
 scripts/metadata.pm |  6 ++-
 scripts/package-metadata.pl | 96 -
 2 files changed, 68 insertions(+), 34 deletions(-)

diff --git a/scripts/metadata.pm b/scripts/metadata.pm
index ecfe42c0bc..21e1a4b250 100644
--- a/scripts/metadata.pm
+++ b/scripts/metadata.pm
@@ -283,7 +283,11 @@ sub parse_package_metadata($) {
defined $category{$1}{$src->{name}} or 
$category{$1}{$src->{name}} = [];
push @{$category{$1}{$src->{name}}}, $pkg;
};
-   /^Description: \s*(.*)\s*$/ and $pkg->{description} = "\t\t 
$1\n". get_multiline(*FILE, "\t\t ");
+   /^Description: \s*(.*)\s*$/ and do {
+   my $tabs = 2;
+   $pkg->{submenu} and $tabs += scalar(split(/\\/, 
$pkg->{submenu}));
+   $pkg->{description} = ("\t" x $tabs)." 
$1\n".get_multiline(*FILE, ("\t" x $tabs)." ");
+   };
/^Type: \s*(.+)\s*$/ and do {
$pkg->{type} = [ split /\s+/, $1 ];
undef $pkg->{tristate};
diff --git a/scripts/package-metadata.pl b/scripts/package-metadata.pl
index 9e0e6dd9e5..fa6b007f38 100755
--- a/scripts/package-metadata.pl
+++ b/scripts/package-metadata.pl
@@ -143,6 +143,7 @@ sub package_depends($$) {
 }
 
 sub mconf_depends {
+   my $tabs = shift;
my $pkgname = shift;
my $depends = shift;
my $only_dep = shift;
@@ -233,24 +234,25 @@ sub mconf_depends {
}
 
foreach my $tdep (@t_depends) {
-   mconf_depends($pkgname, $tdep->[0], 1, $dep, $seen, $tdep->[1]);
+   mconf_depends($tabs, $pkgname, $tdep->[0], 1, $dep, $seen, 
$tdep->[1]);
}
 
foreach my $depend (sort keys %$dep) {
my $m = $dep->{$depend};
-   $res .= "\t\t$m $depend\n";
+   $res .= ("\t" x $tabs)."$m $depend\n";
}
return $res;
 }
 
 sub mconf_conflicts {
+   my $tabs = shift;
my $pkgname = shift;
my $depends = shift;
my $res = "";
 
foreach my $depend (@$depends) {
next unless $package{$depend};
-   $res .= "\t\tdepends on m || (PACKAGE_$depend != y)\n";
+   $res .= ("\t" x $tabs)."depends on m || (PACKAGE_$depend != 
y)\n";
}
return $res;
 }
@@ -265,34 +267,69 @@ sub print_package_config_category($) {
print "menu \"$cat\"\n\n";
my %spkg = %{$category{$cat}};
 
+   $menus{$cat} = {};
+
foreach my $spkg (sort {uc($a) cmp uc($b)} keys %spkg) {
foreach my $pkg (@{$spkg{$spkg}}) {
next if $pkg->{buildonly};
-   my $menu = $pkg->{submenu};
-   if ($menu) {
-   $menu_dep{$menu} or $menu_dep{$menu} = 
$pkg->{submenudep};
+   my $menu = $menus{$cat};
+   if ($pkg->{submenu}) {
+   my @submenus = split(/\\/, $pkg->{submenu});
+   foreach (@submenus) {
+   next unless length($_);
+   if (!$menu->{$_}) {
+   $menu->{$_} = {};
+   }
+
+   $menu = $menu->{$_};
+
+   if ($_ eq $submenus[-1]) {
+   $pkg->{submenudep} and 
$menu->{'_deps'} =