[PATCH 0/4] mbuto: Assorted fixes and simplifications

[PATCH 0/4] mbuto: Assorted fixes and simplifications

[David Gibson]

While debugging a problem Laurent Vivier was having with the passt
test suite, I discovered mbuto's archivemount support is entirely
broken: if archivemount is on the system, mbuto won't work.  While
fixing that, I discovered some slightly related simplfications that
can be made.  Here they all are.

David Gibson (4):
  ${wd} is always set, no need to test for it
  Remove stale archivemount support
  Split "auto" compression mode into its own path
  Remove unnecessary cpio_init function

 mbuto | 120 +++++++++++++++-------------------------------------------
 1 file changed, 30 insertions(+), 90 deletions(-)

-- 
2.44.0

[PATCH 1/4] ${wd} is always set, no need to test for it

[David Gibson]

We unconditionally set ${wd} to a temporary directory, but there are some
places where we test if it is empty.  This appears to be stale code from
some earlier version, so simply remove it.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 mbuto | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mbuto b/mbuto
index 0609e7e..a6f80ab 100755
--- a/mbuto
+++ b/mbuto
@@ -369,7 +369,7 @@ cmd_check() {
 # cleanup() - Remove left-overs on exit, used from trap
 cleanup() {
 	[ -n "${ARCHIVEMOUNT}" ] && "${UMOUNT}" "${wd}" 2>/dev/null
-	[ -n "${wd}" ] && "${RM}" -rf "${wd}"
+	"${RM}" -rf "${wd}"
 	[ -n "${pkg_tmp}" ] && "${RM}" -f "${pkg_tmp}"
 	[ -n "${compress_test1}" ] && "${RM}" -f "${compress_test1}"
 	[ -n "${compress_test2}" ] && "${RM}" -f "${compress_test2}"
@@ -1044,7 +1044,6 @@ build() {
 # add() - Add contents to existing image
 # $1:	Item to be added
 add() {
-	[ -z "${wd}" ] && cpio_init
 	kmod_init
 
 	[ -e "${1}" ] && __path="${1}" || __path=$(command -v "${1}") || :
-- 
@@ -369,7 +369,7 @@ cmd_check() {
 # cleanup() - Remove left-overs on exit, used from trap
 cleanup() {
 	[ -n "${ARCHIVEMOUNT}" ] && "${UMOUNT}" "${wd}" 2>/dev/null
-	[ -n "${wd}" ] && "${RM}" -rf "${wd}"
+	"${RM}" -rf "${wd}"
 	[ -n "${pkg_tmp}" ] && "${RM}" -f "${pkg_tmp}"
 	[ -n "${compress_test1}" ] && "${RM}" -f "${compress_test1}"
 	[ -n "${compress_test2}" ] && "${RM}" -f "${compress_test2}"
@@ -1044,7 +1044,6 @@ build() {
 # add() - Add contents to existing image
 # $1:	Item to be added
 add() {
-	[ -z "${wd}" ] && cpio_init
 	kmod_init
 
 	[ -e "${1}" ] && __path="${1}" || __path=$(command -v "${1}") || :
-- 
2.44.0

[PATCH 2/4] Remove stale archivemount support

[David Gibson]

mbuto has two ways of building the initramfs.  One is the typical approach
of staging its contents in a temporary directory, then building the
initramfs with cpio.  The other is to create an empty initramfs, mount
it with archivemount, and copy things into the mounted archive.

However, the archivemount approach is broken.  I'm not entirely sure why,
but it appears not to properly unmount the archive and retrieve the final.
filled version.  The upshot is that if archivemount is installed, then
mbuto generates an empty, gzip-compressed initramfs instead of whatever it
was supposed to.  It looks like this has bitrotted from some earlier
working version.

The archivemount approach is not necessary, and honestly a pretty strange
and roundabout way of building the initramfs.  Remove it.

Reported-by: Laurent Vivier <lvivier@redhat.com>

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 mbuto | 41 +++++------------------------------------
 1 file changed, 5 insertions(+), 36 deletions(-)

diff --git a/mbuto b/mbuto
index a6f80ab..49d032c 100755
--- a/mbuto
+++ b/mbuto
@@ -368,7 +368,6 @@ cmd_check() {
 
 # cleanup() - Remove left-overs on exit, used from trap
 cleanup() {
-	[ -n "${ARCHIVEMOUNT}" ] && "${UMOUNT}" "${wd}" 2>/dev/null
 	"${RM}" -rf "${wd}"
 	[ -n "${pkg_tmp}" ] && "${RM}" -f "${pkg_tmp}"
 	[ -n "${compress_test1}" ] && "${RM}" -f "${compress_test1}"
@@ -564,16 +563,6 @@ cpio_init() {
 		else
 			OUT="$("${REALPATH}" "${OUT}")"
 		fi
-
-		if [ -n "${ARCHIVEMOUNT}" ]; then
-			: | "${CPIO}" --create -H newc --quiet | \
-				"${GZIP}" > "${OUT}"
-		fi
-	fi
-
-	if [ -n "${ARCHIVEMOUNT}" ]; then
-		"${ARCHIVEMOUNT}" "${OUT}" "${wd}"
-		info "Mounted CPIO archive ${OUT} at ${wd}"
 	fi
 }
 
@@ -1099,26 +1088,17 @@ cmds() {
 
 	[ "${NOSTRIP}" != "y" ] && strip_all
 
-	if [ -z "${ARCHIVEMOUNT}" ]; then
-		( __out="$("${REALPATH}" "${OUT}")"
-		  "${CD}" "${wd}"
-		  "${FIND}" . | "${CPIO}" --create -H newc --quiet > "${OUT}"
-		  cpio_compress "${__out}"
-		)
-	else
-		"${SYNC}"
-	fi
+	( __out="$("${REALPATH}" "${OUT}")"
+		"${CD}" "${wd}"
+		"${FIND}" . | "${CPIO}" --create -H newc --quiet > "${OUT}"
+		cpio_compress "${__out}"
+	)
 
 	stats
 
 	printf "%s" "${OUTPUT}"						| \
 		sed 's,__INITRD__,'"$("${REALPATH}" "${OUT}")"',g'	| \
 		sed 's,__KERNEL__,/boot/vmlinuz-'"${KERNEL}"',g'
-
-	if [ -n "${ARCHIVEMOUNT}" ]; then
-		trap - EXIT
-		notice "initramfs mounted at: ${wd}"
-	fi
 }
 
 # usage() - Print usage and exit
@@ -1271,17 +1251,6 @@ else
 	eval "profile_${PROFILE}" || err "profile ${PROFILE} not found"
 fi
 
-# Check if we can keep the CPIO mounted for convenience as we exit. This isn't
-# safe with fakeroot, as contents can't be touched before the environment
-# save-file is loaded again, so it needs root (and archivemount).
-if [ "${LD_PRELOAD}" = "libfakeroot-sysv.so" ]; then
-	if command -v archivemount >/dev/null 2>&1; then
-		notice "Not running as root, won't keep cpio mounted"
-	fi
-elif ! ARCHIVEMOUNT="$(command -v archivemount)"; then
-	warn "archivemount not available, won't keep cpio mounted"
-fi
-
 trap cleanup EXIT
 
 cmds "$@"
-- 
@@ -368,7 +368,6 @@ cmd_check() {
 
 # cleanup() - Remove left-overs on exit, used from trap
 cleanup() {
-	[ -n "${ARCHIVEMOUNT}" ] && "${UMOUNT}" "${wd}" 2>/dev/null
 	"${RM}" -rf "${wd}"
 	[ -n "${pkg_tmp}" ] && "${RM}" -f "${pkg_tmp}"
 	[ -n "${compress_test1}" ] && "${RM}" -f "${compress_test1}"
@@ -564,16 +563,6 @@ cpio_init() {
 		else
 			OUT="$("${REALPATH}" "${OUT}")"
 		fi
-
-		if [ -n "${ARCHIVEMOUNT}" ]; then
-			: | "${CPIO}" --create -H newc --quiet | \
-				"${GZIP}" > "${OUT}"
-		fi
-	fi
-
-	if [ -n "${ARCHIVEMOUNT}" ]; then
-		"${ARCHIVEMOUNT}" "${OUT}" "${wd}"
-		info "Mounted CPIO archive ${OUT} at ${wd}"
 	fi
 }
 
@@ -1099,26 +1088,17 @@ cmds() {
 
 	[ "${NOSTRIP}" != "y" ] && strip_all
 
-	if [ -z "${ARCHIVEMOUNT}" ]; then
-		( __out="$("${REALPATH}" "${OUT}")"
-		  "${CD}" "${wd}"
-		  "${FIND}" . | "${CPIO}" --create -H newc --quiet > "${OUT}"
-		  cpio_compress "${__out}"
-		)
-	else
-		"${SYNC}"
-	fi
+	( __out="$("${REALPATH}" "${OUT}")"
+		"${CD}" "${wd}"
+		"${FIND}" . | "${CPIO}" --create -H newc --quiet > "${OUT}"
+		cpio_compress "${__out}"
+	)
 
 	stats
 
 	printf "%s" "${OUTPUT}"						| \
 		sed 's,__INITRD__,'"$("${REALPATH}" "${OUT}")"',g'	| \
 		sed 's,__KERNEL__,/boot/vmlinuz-'"${KERNEL}"',g'
-
-	if [ -n "${ARCHIVEMOUNT}" ]; then
-		trap - EXIT
-		notice "initramfs mounted at: ${wd}"
-	fi
 }
 
 # usage() - Print usage and exit
@@ -1271,17 +1251,6 @@ else
 	eval "profile_${PROFILE}" || err "profile ${PROFILE} not found"
 fi
 
-# Check if we can keep the CPIO mounted for convenience as we exit. This isn't
-# safe with fakeroot, as contents can't be touched before the environment
-# save-file is loaded again, so it needs root (and archivemount).
-if [ "${LD_PRELOAD}" = "libfakeroot-sysv.so" ]; then
-	if command -v archivemount >/dev/null 2>&1; then
-		notice "Not running as root, won't keep cpio mounted"
-	fi
-elif ! ARCHIVEMOUNT="$(command -v archivemount)"; then
-	warn "archivemount not available, won't keep cpio mounted"
-fi
-
 trap cleanup EXIT
 
 cmds "$@"
-- 
2.44.0

Re: [PATCH 2/4] Remove stale archivemount support

[Stefano Brivio]

On Fri, 22 Mar 2024 13:27:37 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> mbuto has two ways of building the initramfs.  One is the typical approach
> of staging its contents in a temporary directory, then building the
> initramfs with cpio.  The other is to create an empty initramfs, mount
> it with archivemount, and copy things into the mounted archive.
> 
> However, the archivemount approach is broken.  I'm not entirely sure why,
> but it appears not to properly unmount the archive and retrieve the final.
> filled version.  The upshot is that if archivemount is installed, then
> mbuto generates an empty, gzip-compressed initramfs instead of whatever it
> was supposed to.  It looks like this has bitrotted from some earlier
> working version.
> 
> The archivemount approach is not necessary, and honestly a pretty strange
> and roundabout way of building the initramfs.  Remove it.

There were two reasons behind that: first off, I mistakenly assumed
that the kernel could see changes made to the initramfs after boot.

Second, it was actually convenient for developing this tool as I could
just make directories and copies in half-working images. I also had
half a mind about some usage with QEMU rebooting the guest, and the
initramfs would change across reboots without having to call mbuto
again, for bisections or suchlike.

But sure, it turned out to be quite complicated to maintain, and it
looks like it has been broken on Fedora for a while now (probably due
to differences between fakeroot and fakeroot-ng I didn't take into
account), so, with some regrets, I'm fine to drop this now.

-- 
Stefano

Re: [PATCH 2/4] Remove stale archivemount support

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2327 bytes --]

On Fri, Apr 05, 2024 at 08:09:32PM +0200, Stefano Brivio wrote:
> On Fri, 22 Mar 2024 13:27:37 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > mbuto has two ways of building the initramfs.  One is the typical approach
> > of staging its contents in a temporary directory, then building the
> > initramfs with cpio.  The other is to create an empty initramfs, mount
> > it with archivemount, and copy things into the mounted archive.
> > 
> > However, the archivemount approach is broken.  I'm not entirely sure why,
> > but it appears not to properly unmount the archive and retrieve the final.
> > filled version.  The upshot is that if archivemount is installed, then
> > mbuto generates an empty, gzip-compressed initramfs instead of whatever it
> > was supposed to.  It looks like this has bitrotted from some earlier
> > working version.
> > 
> > The archivemount approach is not necessary, and honestly a pretty strange
> > and roundabout way of building the initramfs.  Remove it.
> 
> There were two reasons behind that: first off, I mistakenly assumed
> that the kernel could see changes made to the initramfs after boot.

Yeah, that was never going to work.

> Second, it was actually convenient for developing this tool as I could
> just make directories and copies in half-working images. I also had
> half a mind about some usage with QEMU rebooting the guest, and the
> initramfs would change across reboots without having to call mbuto
> again, for bisections or suchlike.

That's not really dependent on using archivemount in any case.  If
qemu re-reads the initrd on each boot, then you can still do this by
rebuilding the image between boots (which is all that archivemount
would do anyway).  If qemu doesn't re-read it, then this won't work
even if you are using archivemount.

> But sure, it turned out to be quite complicated to maintain, and it
> looks like it has been broken on Fedora for a while now (probably due
> to differences between fakeroot and fakeroot-ng I didn't take into
> account), so, with some regrets, I'm fine to drop this now.
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/4] Remove stale archivemount support

[Stefano Brivio]

On Sat, 6 Apr 2024 14:02:58 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Fri, Apr 05, 2024 at 08:09:32PM +0200, Stefano Brivio wrote:
> > On Fri, 22 Mar 2024 13:27:37 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >   
> > > mbuto has two ways of building the initramfs.  One is the typical approach
> > > of staging its contents in a temporary directory, then building the
> > > initramfs with cpio.  The other is to create an empty initramfs, mount
> > > it with archivemount, and copy things into the mounted archive.
> > > 
> > > However, the archivemount approach is broken.  I'm not entirely sure why,
> > > but it appears not to properly unmount the archive and retrieve the final.
> > > filled version.  The upshot is that if archivemount is installed, then
> > > mbuto generates an empty, gzip-compressed initramfs instead of whatever it
> > > was supposed to.  It looks like this has bitrotted from some earlier
> > > working version.
> > > 
> > > The archivemount approach is not necessary, and honestly a pretty strange
> > > and roundabout way of building the initramfs.  Remove it.  
> > 
> > There were two reasons behind that: first off, I mistakenly assumed
> > that the kernel could see changes made to the initramfs after boot.  
> 
> Yeah, that was never going to work.
> 
> > Second, it was actually convenient for developing this tool as I could
> > just make directories and copies in half-working images. I also had
> > half a mind about some usage with QEMU rebooting the guest, and the
> > initramfs would change across reboots without having to call mbuto
> > again, for bisections or suchlike.  
> 
> That's not really dependent on using archivemount in any case.  If
> qemu re-reads the initrd on each boot, then you can still do this by
> rebuilding the image between boots (which is all that archivemount
> would do anyway).

That was exactly my point: with archivemount, the user doesn't have to
call mbuto again -- archivemount does it, implicitly.

> If qemu doesn't re-read it, then this won't work even if you are using
> archivemount.

It looks like QEMU doesn't do that, at least not on x86, so yes, this
part never worked anyway.

-- 
Stefano

Re: [PATCH 2/4] Remove stale archivemount support

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3394 bytes --]

On Thu, Apr 25, 2024 at 06:47:38AM +0200, Stefano Brivio wrote:
> On Sat, 6 Apr 2024 14:02:58 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Fri, Apr 05, 2024 at 08:09:32PM +0200, Stefano Brivio wrote:
> > > On Fri, 22 Mar 2024 13:27:37 +1100
> > > David Gibson <david@gibson.dropbear.id.au> wrote:
> > >   
> > > > mbuto has two ways of building the initramfs.  One is the typical approach
> > > > of staging its contents in a temporary directory, then building the
> > > > initramfs with cpio.  The other is to create an empty initramfs, mount
> > > > it with archivemount, and copy things into the mounted archive.
> > > > 
> > > > However, the archivemount approach is broken.  I'm not entirely sure why,
> > > > but it appears not to properly unmount the archive and retrieve the final.
> > > > filled version.  The upshot is that if archivemount is installed, then
> > > > mbuto generates an empty, gzip-compressed initramfs instead of whatever it
> > > > was supposed to.  It looks like this has bitrotted from some earlier
> > > > working version.
> > > > 
> > > > The archivemount approach is not necessary, and honestly a pretty strange
> > > > and roundabout way of building the initramfs.  Remove it.  
> > > 
> > > There were two reasons behind that: first off, I mistakenly assumed
> > > that the kernel could see changes made to the initramfs after boot.  
> > 
> > Yeah, that was never going to work.
> > 
> > > Second, it was actually convenient for developing this tool as I could
> > > just make directories and copies in half-working images. I also had
> > > half a mind about some usage with QEMU rebooting the guest, and the
> > > initramfs would change across reboots without having to call mbuto
> > > again, for bisections or suchlike.  
> > 
> > That's not really dependent on using archivemount in any case.  If
> > qemu re-reads the initrd on each boot, then you can still do this by
> > rebuilding the image between boots (which is all that archivemount
> > would do anyway).
> 
> That was exactly my point: with archivemount, the user doesn't have to
> call mbuto again -- archivemount does it, implicitly.

Not in a way that you can use safely.  Archives are not a filesystem,
let alone a filesystem designed for concurrent access from multiple
parties.  So, it's basically never going to be safe to read the
archive independently while archivemount has it open for writing.  If
you're lucky, you might get what you want, but you could also get old
contents because archivemount hasn't flushed out the changes yet
(which is exactly what we saw).  Or, worse, you could see a partly
written archive, because you read when archivemount had started but
not finished writing.  Or, worse still, you could see inconsistent
garbage, because you were reading partway through, then archivemount
rewrote from the beginning.

If you want this behaviour, you need something like virtio-fs or NFS.

> > If qemu doesn't re-read it, then this won't work even if you are using
> > archivemount.
> 
> It looks like QEMU doesn't do that, at least not on x86, so yes, this
> part never worked anyway.
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 3/4] Split "auto" compression mode into its own path

[David Gibson]

mbuto supports "auto" compression mode where we detect the fastest
compressor and use it.  This is structured a bit oddly - cpio_compress()
first handles the case of an explicitly selected compressor, then handles
the auto-detected case, redundantly actually implementing the compression
once it has picked one.

Make this a bit clearer: first handle the "auto" case by calling out to
the testing code, and using that to set the parameter for the specific
compression path.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 mbuto | 53 ++++++++++++++++++++++++-----------------------------
 1 file changed, 24 insertions(+), 29 deletions(-)

diff --git a/mbuto b/mbuto
index 49d032c..550f76e 100755
--- a/mbuto
+++ b/mbuto
@@ -566,31 +566,11 @@ cpio_init() {
 	fi
 }
 
-# cpio_compress() - Compress archive, test available methods if none is selected
+# compress_select() - Try compressors and pick the fastest
 # $1:	Existing CPIO archive
-cpio_compress() {
-	{ [ -z "${COMPRESS}" ] || [ "${COMPRESS}" = "none" ]; } && return
-
-	info "Compressing CPIO archive ${1}"
-
-	if [ "${COMPRESS}" != "auto" ]; then
-		[ "${COMPRESS}" = "lzo" ] && __cmd="lzop" || __cmd="${COMPRESS}"
-
-		cmd_check "${__cmd}"
-		if [ "${__cmd}" = "lz4" ]; then
-			"${__cmd}" -l -f -q -9 "${1}" "${1}.lz4"
-		else
-			"${__cmd}" -f -q -9 -S .lz4 "${1}"
-		fi
-
-		mv "${1}.lz4" "${1}"
-
-		return
-	fi
-
+compress_select() {
 	if [ ! -f "/boot/config-${KERNEL}" ]; then
-		"${GZIP}" -9 "${1}"
-		"${MV}" "${1}.gz" "${1}"
+		echo "gzip"
 		return
 	fi
 
@@ -639,14 +619,29 @@ cpio_compress() {
 		       "${__a}:" "${__size} bytes" "${__time}s"
 	done
 
-	[ "${__pick}" = "lzo" ] && __cmd="lzop" || __cmd="${__pick}"
-	[ "${__cmd}" = "lz4" ] && __opt="-l" || __opt=""
-
-	"${__cmd}" ${__opt} -q -9 -c "${1}" > "${compress_test1}"
 	notice "Picked ${__pick} compression for CPIO"
+	rm ${compress_test1} ${compress_test2}
+	echo "${__pick}"
+}
+
+# cpio_compress() - Compress archive, test available methods if none is selected
+# $1:	Existing CPIO archive
+cpio_compress() {
+	{ [ -z "${COMPRESS}" ] || [ "${COMPRESS}" = "none" ]; } && return
+	[ "${COMPRESS}" = "auto" ] && COMPRESS=$(compress_select "$1")
+
+	info "Compressing CPIO archive ${1}"
+
+	[ "${COMPRESS}" = "lzo" ] && __cmd="lzop" || __cmd="${COMPRESS}"
+
+	cmd_check "${__cmd}"
+	if [ "${__cmd}" = "lz4" ]; then
+		"${__cmd}" -l -f -q -9 "${1}" "${1}.lz4"
+	else
+		"${__cmd}" -f -q -9 -S .lz4 "${1}"
+	fi
 
-	mv "${compress_test1}" "${OUT}"
-	rm "${compress_test2}"
+	mv "${1}.lz4" "${1}"
 }
 
 ################################################################################
-- 
@@ -566,31 +566,11 @@ cpio_init() {
 	fi
 }
 
-# cpio_compress() - Compress archive, test available methods if none is selected
+# compress_select() - Try compressors and pick the fastest
 # $1:	Existing CPIO archive
-cpio_compress() {
-	{ [ -z "${COMPRESS}" ] || [ "${COMPRESS}" = "none" ]; } && return
-
-	info "Compressing CPIO archive ${1}"
-
-	if [ "${COMPRESS}" != "auto" ]; then
-		[ "${COMPRESS}" = "lzo" ] && __cmd="lzop" || __cmd="${COMPRESS}"
-
-		cmd_check "${__cmd}"
-		if [ "${__cmd}" = "lz4" ]; then
-			"${__cmd}" -l -f -q -9 "${1}" "${1}.lz4"
-		else
-			"${__cmd}" -f -q -9 -S .lz4 "${1}"
-		fi
-
-		mv "${1}.lz4" "${1}"
-
-		return
-	fi
-
+compress_select() {
 	if [ ! -f "/boot/config-${KERNEL}" ]; then
-		"${GZIP}" -9 "${1}"
-		"${MV}" "${1}.gz" "${1}"
+		echo "gzip"
 		return
 	fi
 
@@ -639,14 +619,29 @@ cpio_compress() {
 		       "${__a}:" "${__size} bytes" "${__time}s"
 	done
 
-	[ "${__pick}" = "lzo" ] && __cmd="lzop" || __cmd="${__pick}"
-	[ "${__cmd}" = "lz4" ] && __opt="-l" || __opt=""
-
-	"${__cmd}" ${__opt} -q -9 -c "${1}" > "${compress_test1}"
 	notice "Picked ${__pick} compression for CPIO"
+	rm ${compress_test1} ${compress_test2}
+	echo "${__pick}"
+}
+
+# cpio_compress() - Compress archive, test available methods if none is selected
+# $1:	Existing CPIO archive
+cpio_compress() {
+	{ [ -z "${COMPRESS}" ] || [ "${COMPRESS}" = "none" ]; } && return
+	[ "${COMPRESS}" = "auto" ] && COMPRESS=$(compress_select "$1")
+
+	info "Compressing CPIO archive ${1}"
+
+	[ "${COMPRESS}" = "lzo" ] && __cmd="lzop" || __cmd="${COMPRESS}"
+
+	cmd_check "${__cmd}"
+	if [ "${__cmd}" = "lz4" ]; then
+		"${__cmd}" -l -f -q -9 "${1}" "${1}.lz4"
+	else
+		"${__cmd}" -f -q -9 -S .lz4 "${1}"
+	fi
 
-	mv "${compress_test1}" "${OUT}"
-	rm "${compress_test2}"
+	mv "${1}.lz4" "${1}"
 }
 
 ################################################################################
-- 
2.44.0

Re: [PATCH 3/4] Split "auto" compression mode into its own path

[Stefano Brivio]

On Fri, 22 Mar 2024 13:27:38 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> mbuto supports "auto" compression mode where we detect the fastest
> compressor and use it.  This is structured a bit oddly - cpio_compress()
> first handles the case of an explicitly selected compressor, then handles
> the auto-detected case, redundantly actually implementing the compression
> once it has picked one.

Yes, we don't want to risk keeping around several formats if the
archives are big... but you're not changing this, right?

> Make this a bit clearer: first handle the "auto" case by calling out to
> the testing code, and using that to set the parameter for the specific
> compression path.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  mbuto | 53 ++++++++++++++++++++++++-----------------------------
>  1 file changed, 24 insertions(+), 29 deletions(-)
> 
> diff --git a/mbuto b/mbuto
> index 49d032c..550f76e 100755
> --- a/mbuto
> +++ b/mbuto
> @@ -566,31 +566,11 @@ cpio_init() {
>  	fi
>  }
>  
> -# cpio_compress() - Compress archive, test available methods if none is selected
> +# compress_select() - Try compressors and pick the fastest

Now we can say we pick the fastest, but it's not clear in what: we care
about decompression, so perhaps:

# compress_select() - Try compressors and pick the fastest to decompress image

?

-- 
Stefano

Re: [PATCH 3/4] Split "auto" compression mode into its own path

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2251 bytes --]

On Fri, Apr 05, 2024 at 08:10:02PM +0200, Stefano Brivio wrote:
> On Fri, 22 Mar 2024 13:27:38 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > mbuto supports "auto" compression mode where we detect the fastest
> > compressor and use it.  This is structured a bit oddly - cpio_compress()
> > first handles the case of an explicitly selected compressor, then handles
> > the auto-detected case, redundantly actually implementing the compression
> > once it has picked one.
> 
> Yes, we don't want to risk keeping around several formats if the
> archives are big... but you're not changing this, right?

I don't believe so, no.

> > Make this a bit clearer: first handle the "auto" case by calling out to
> > the testing code, and using that to set the parameter for the specific
> > compression path.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  mbuto | 53 ++++++++++++++++++++++++-----------------------------
> >  1 file changed, 24 insertions(+), 29 deletions(-)
> > 
> > diff --git a/mbuto b/mbuto
> > index 49d032c..550f76e 100755
> > --- a/mbuto
> > +++ b/mbuto
> > @@ -566,31 +566,11 @@ cpio_init() {
> >  	fi
> >  }
> >  
> > -# cpio_compress() - Compress archive, test available methods if none is selected
> > +# compress_select() - Try compressors and pick the fastest
> 
> Now we can say we pick the fastest, but it's not clear in what: we care
> about decompression, so perhaps:
> 
> # compress_select() - Try compressors and pick the fastest to decompress image
> 
> ?

I don't think that's accurate.  AFAICT the code is selecting the
fastest to compress, not decompress, and I didn't change that.  I
agree that optimizing decompression speed would make more sense.

Honestly, I'm not really convinced that the auto mode is useful
anyway: even if we changed to decompression speed, the speed on the
location running mbuto isn't necessarily the same as on the target
doing the decompression (though given mbuto's usage model, it's
lkely).


-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/4] Split "auto" compression mode into its own path

[Stefano Brivio]

On Sat, 6 Apr 2024 14:06:39 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Fri, Apr 05, 2024 at 08:10:02PM +0200, Stefano Brivio wrote:
> > On Fri, 22 Mar 2024 13:27:38 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >   
> > > mbuto supports "auto" compression mode where we detect the fastest
> > > compressor and use it.  This is structured a bit oddly - cpio_compress()
> > > first handles the case of an explicitly selected compressor, then handles
> > > the auto-detected case, redundantly actually implementing the compression
> > > once it has picked one.  
> > 
> > Yes, we don't want to risk keeping around several formats if the
> > archives are big... but you're not changing this, right?  
> 
> I don't believe so, no.
> 
> > > Make this a bit clearer: first handle the "auto" case by calling out to
> > > the testing code, and using that to set the parameter for the specific
> > > compression path.
> > > 
> > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > > ---
> > >  mbuto | 53 ++++++++++++++++++++++++-----------------------------
> > >  1 file changed, 24 insertions(+), 29 deletions(-)
> > > 
> > > diff --git a/mbuto b/mbuto
> > > index 49d032c..550f76e 100755
> > > --- a/mbuto
> > > +++ b/mbuto
> > > @@ -566,31 +566,11 @@ cpio_init() {
> > >  	fi
> > >  }
> > >  
> > > -# cpio_compress() - Compress archive, test available methods if none is selected
> > > +# compress_select() - Try compressors and pick the fastest  
> > 
> > Now we can say we pick the fastest, but it's not clear in what: we care
> > about decompression, so perhaps:
> > 
> > # compress_select() - Try compressors and pick the fastest to decompress image
> > 
> > ?  
> 
> I don't think that's accurate.  AFAICT the code is selecting the
> fastest to compress, not decompress,

No, why? Look:

                __start=$("${CAT}" /proc/uptime)
                __start="${__start% *}"

                for _ in $("${SEQ}" 1 5); do
                        "${CP}" "${compress_test1}" "${compress_test2}"
                        ${__cmd} --force -d -c "${compress_test2}" > /dev/null
                done

                __end=$("${CAT}" /proc/uptime)
                __end="${__end% *}"

see the '-d' there ('-c' is usually '--stdout').

> and I didn't change that.  I agree that optimizing decompression
> speed would make more sense.
> 
> Honestly, I'm not really convinced that the auto mode is useful
> anyway: even if we changed to decompression speed, the speed on the
> location running mbuto isn't necessarily the same as on the target
> doing the decompression (though given mbuto's usage model, it's
> lkely).

Well, it's exactly the intended usage -- any other usage would be
rather tricky (at least, anything with a different architecture for the
guest). That's how the current passt test suite uses it, too. And it
really makes a difference if you're working with kselftests.

-- 
Stefano

Re: [PATCH 3/4] Split "auto" compression mode into its own path

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3452 bytes --]

On Thu, Apr 25, 2024 at 06:46:05AM +0200, Stefano Brivio wrote:
> On Sat, 6 Apr 2024 14:06:39 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Fri, Apr 05, 2024 at 08:10:02PM +0200, Stefano Brivio wrote:
> > > On Fri, 22 Mar 2024 13:27:38 +1100
> > > David Gibson <david@gibson.dropbear.id.au> wrote:
> > >   
> > > > mbuto supports "auto" compression mode where we detect the fastest
> > > > compressor and use it.  This is structured a bit oddly - cpio_compress()
> > > > first handles the case of an explicitly selected compressor, then handles
> > > > the auto-detected case, redundantly actually implementing the compression
> > > > once it has picked one.  
> > > 
> > > Yes, we don't want to risk keeping around several formats if the
> > > archives are big... but you're not changing this, right?  
> > 
> > I don't believe so, no.
> > 
> > > > Make this a bit clearer: first handle the "auto" case by calling out to
> > > > the testing code, and using that to set the parameter for the specific
> > > > compression path.
> > > > 
> > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > > > ---
> > > >  mbuto | 53 ++++++++++++++++++++++++-----------------------------
> > > >  1 file changed, 24 insertions(+), 29 deletions(-)
> > > > 
> > > > diff --git a/mbuto b/mbuto
> > > > index 49d032c..550f76e 100755
> > > > --- a/mbuto
> > > > +++ b/mbuto
> > > > @@ -566,31 +566,11 @@ cpio_init() {
> > > >  	fi
> > > >  }
> > > >  
> > > > -# cpio_compress() - Compress archive, test available methods if none is selected
> > > > +# compress_select() - Try compressors and pick the fastest  
> > > 
> > > Now we can say we pick the fastest, but it's not clear in what: we care
> > > about decompression, so perhaps:
> > > 
> > > # compress_select() - Try compressors and pick the fastest to decompress image
> > > 
> > > ?  
> > 
> > I don't think that's accurate.  AFAICT the code is selecting the
> > fastest to compress, not decompress,
> 
> No, why? Look:
> 
>                 __start=$("${CAT}" /proc/uptime)
>                 __start="${__start% *}"
> 
>                 for _ in $("${SEQ}" 1 5); do
>                         "${CP}" "${compress_test1}" "${compress_test2}"
>                         ${__cmd} --force -d -c "${compress_test2}" > /dev/null
>                 done
> 
>                 __end=$("${CAT}" /proc/uptime)
>                 __end="${__end% *}"
> 
> see the '-d' there ('-c' is usually '--stdout').

Oh, yes.  My mistake.

> > and I didn't change that.  I agree that optimizing decompression
> > speed would make more sense.
> > 
> > Honestly, I'm not really convinced that the auto mode is useful
> > anyway: even if we changed to decompression speed, the speed on the
> > location running mbuto isn't necessarily the same as on the target
> > doing the decompression (though given mbuto's usage model, it's
> > lkely).
> 
> Well, it's exactly the intended usage -- any other usage would be
> rather tricky (at least, anything with a different architecture for the
> guest). That's how the current passt test suite uses it, too. And it
> really makes a difference if you're working with kselftests.

Hm, ok.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 4/4] Remove unnecessary cpio_init function

[David Gibson]

The cpio_init function is now only every called with the "discard" option.
But, moreover, what it does is create an initial mostly empty archive which
will just get overwritten by the final archive.

So, it's entirely unnecessary except for one subtlety.  Our use of realpath
when generating the final output requires that a file already exist in the
output location.  We can fix that by shuffling some things out of a
subshell, removing the need for realpath.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 mbuto | 31 ++++---------------------------
 1 file changed, 4 insertions(+), 27 deletions(-)

diff --git a/mbuto b/mbuto
index 550f76e..0c51e18 100755
--- a/mbuto
+++ b/mbuto
@@ -544,28 +544,6 @@ subopts_get() {
 
 ### CPIO #######################################################################
 
-# cpio_init() - Source existing CPIO archive, or create if needed
-# $1:	Path to CPIO archive, might exist, might be discarded if existing
-cpio_init() {
-	if [ -f "${OUT}" ] && [ "${1}" != "discard" ]; then
-		info "Sourcing CPIO archive from ${OUT}"
-
-		if ! "${GZIP}" -dfc "${OUT}" |
-		   "${CPIO}" --quiet -iD "${wd}"; then
-			err "Invalid CPIO archive ${OUT}"
-		fi
-	else
-		info "Creating new CPIO archive"
-
-		if [ -z "${OUT}" ]; then
-			OUT="$("${MKTEMP}")"
-			notice "Creating image: ${OUT}"
-		else
-			OUT="$("${REALPATH}" "${OUT}")"
-		fi
-	fi
-}
-
 # compress_select() - Try compressors and pick the fastest
 # $1:	Existing CPIO archive
 compress_select() {
@@ -987,7 +965,6 @@ pkg_add() {
 
 # build() - Build a new image, sourcing contents
 build() {
-	cpio_init discard
 	kmod_init
 
 	for __d in ${DIRS}; do
@@ -1083,11 +1060,11 @@ cmds() {
 
 	[ "${NOSTRIP}" != "y" ] && strip_all
 
-	( __out="$("${REALPATH}" "${OUT}")"
+	(
 		"${CD}" "${wd}"
-		"${FIND}" . | "${CPIO}" --create -H newc --quiet > "${OUT}"
-		cpio_compress "${__out}"
-	)
+		"${FIND}" . | "${CPIO}" --create -H newc --quiet
+	) > "${OUT}"
+	cpio_compress "${OUT}"
 
 	stats
 
-- 
@@ -544,28 +544,6 @@ subopts_get() {
 
 ### CPIO #######################################################################
 
-# cpio_init() - Source existing CPIO archive, or create if needed
-# $1:	Path to CPIO archive, might exist, might be discarded if existing
-cpio_init() {
-	if [ -f "${OUT}" ] && [ "${1}" != "discard" ]; then
-		info "Sourcing CPIO archive from ${OUT}"
-
-		if ! "${GZIP}" -dfc "${OUT}" |
-		   "${CPIO}" --quiet -iD "${wd}"; then
-			err "Invalid CPIO archive ${OUT}"
-		fi
-	else
-		info "Creating new CPIO archive"
-
-		if [ -z "${OUT}" ]; then
-			OUT="$("${MKTEMP}")"
-			notice "Creating image: ${OUT}"
-		else
-			OUT="$("${REALPATH}" "${OUT}")"
-		fi
-	fi
-}
-
 # compress_select() - Try compressors and pick the fastest
 # $1:	Existing CPIO archive
 compress_select() {
@@ -987,7 +965,6 @@ pkg_add() {
 
 # build() - Build a new image, sourcing contents
 build() {
-	cpio_init discard
 	kmod_init
 
 	for __d in ${DIRS}; do
@@ -1083,11 +1060,11 @@ cmds() {
 
 	[ "${NOSTRIP}" != "y" ] && strip_all
 
-	( __out="$("${REALPATH}" "${OUT}")"
+	(
 		"${CD}" "${wd}"
-		"${FIND}" . | "${CPIO}" --create -H newc --quiet > "${OUT}"
-		cpio_compress "${__out}"
-	)
+		"${FIND}" . | "${CPIO}" --create -H newc --quiet
+	) > "${OUT}"
+	cpio_compress "${OUT}"
 
 	stats
 
-- 
2.44.0

Re: [PATCH 4/4] Remove unnecessary cpio_init function

[Stefano Brivio]

On Fri, 22 Mar 2024 13:27:39 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> The cpio_init function is now only every called with the "discard" option.
> But, moreover, what it does is create an initial mostly empty archive which
> will just get overwritten by the final archive.
> 
> So, it's entirely unnecessary except for one subtlety.  Our use of realpath
> when generating the final output requires that a file already exist in the
> output location.  We can fix that by shuffling some things out of a
> subshell, removing the need for realpath.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  mbuto | 31 ++++---------------------------
>  1 file changed, 4 insertions(+), 27 deletions(-)
> 
> diff --git a/mbuto b/mbuto
> index 550f76e..0c51e18 100755
> --- a/mbuto
> +++ b/mbuto
> @@ -544,28 +544,6 @@ subopts_get() {
>  
>  ### CPIO #######################################################################
>  
> -# cpio_init() - Source existing CPIO archive, or create if needed
> -# $1:	Path to CPIO archive, might exist, might be discarded if existing
> -cpio_init() {
> -	if [ -f "${OUT}" ] && [ "${1}" != "discard" ]; then
> -		info "Sourcing CPIO archive from ${OUT}"
> -
> -		if ! "${GZIP}" -dfc "${OUT}" |
> -		   "${CPIO}" --quiet -iD "${wd}"; then
> -			err "Invalid CPIO archive ${OUT}"
> -		fi
> -	else

This, and the second part of 1/4, remove a functionality which I
accidentally broke in commit b87e4f2e6595 ("mbuto: Create working
directory before profiles are sourced"): it was once possible to add
contents to an existing initramfs archive.

I guess I might be the only interactive user of mbuto at the moment, so I
got slightly annoyed by the fact it didn't work anymore but I didn't
really investigate further. I used it whenever an initramfs took more than
5-10 seconds to build, and I'd keep forgetting to add stuff.

Note that the help message still describes this mode of operation:

  "Build initramfs image unless an existing one is passed."

and in the "Examples":

	./mbuto -f kata.img zsh_5.6.2-3_amd64.deb
		Install zsh package to pre-existing kata.img

but I also realised it's not convenient to have this as default, because,
especially for initramfs images that can be built faster, one might just
want to rebuild a single binary and the image, repeatedly.

Perhaps, instead of dropping this altogether, we could introduce a new
-K ("keep") option that skips the unconditional wd assignment and calls
cpio_init() (which becomes cpio_load() or something like that).

-- 
Stefano

Re: [PATCH 4/4] Remove unnecessary cpio_init function

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3415 bytes --]

On Fri, Apr 05, 2024 at 08:08:53PM +0200, Stefano Brivio wrote:
> On Fri, 22 Mar 2024 13:27:39 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > The cpio_init function is now only every called with the "discard" option.
> > But, moreover, what it does is create an initial mostly empty archive which
> > will just get overwritten by the final archive.
> > 
> > So, it's entirely unnecessary except for one subtlety.  Our use of realpath
> > when generating the final output requires that a file already exist in the
> > output location.  We can fix that by shuffling some things out of a
> > subshell, removing the need for realpath.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  mbuto | 31 ++++---------------------------
> >  1 file changed, 4 insertions(+), 27 deletions(-)
> > 
> > diff --git a/mbuto b/mbuto
> > index 550f76e..0c51e18 100755
> > --- a/mbuto
> > +++ b/mbuto
> > @@ -544,28 +544,6 @@ subopts_get() {
> >  
> >  ### CPIO #######################################################################
> >  
> > -# cpio_init() - Source existing CPIO archive, or create if needed
> > -# $1:	Path to CPIO archive, might exist, might be discarded if existing
> > -cpio_init() {
> > -	if [ -f "${OUT}" ] && [ "${1}" != "discard" ]; then
> > -		info "Sourcing CPIO archive from ${OUT}"
> > -
> > -		if ! "${GZIP}" -dfc "${OUT}" |
> > -		   "${CPIO}" --quiet -iD "${wd}"; then
> > -			err "Invalid CPIO archive ${OUT}"
> > -		fi
> > -	else
> 
> This, and the second part of 1/4, remove a functionality which I
> accidentally broke in commit b87e4f2e6595 ("mbuto: Create working
> directory before profiles are sourced"): it was once possible to add
> contents to an existing initramfs archive.

Yeah, I didn't track it to a specific point, but I assumed that it was
built to add to an existing archive in the past.  It looked fairly
thoroughly broken to me.

> I guess I might be the only interactive user of mbuto at the moment, so I
> got slightly annoyed by the fact it didn't work anymore but I didn't
> really investigate further. I used it whenever an initramfs took more than
> 5-10 seconds to build, and I'd keep forgetting to add stuff.

So... in fact the kernel accepts not just a single cpio archive, but
multiple.  So rather than doing anything with archivemount or mangling
the cpio, you could just append a secondary cpio with the additions.

> Note that the help message still describes this mode of operation:
> 
>   "Build initramfs image unless an existing one is passed."
> 
> and in the "Examples":
> 
> 	./mbuto -f kata.img zsh_5.6.2-3_amd64.deb
> 		Install zsh package to pre-existing kata.img

Ah, yes, they should be updated to match.

> but I also realised it's not convenient to have this as default, because,
> especially for initramfs images that can be built faster, one might just
> want to rebuild a single binary and the image, repeatedly.
> 
> Perhaps, instead of dropping this altogether, we could introduce a new
> -K ("keep") option that skips the unconditional wd assignment and calls
> cpio_init() (which becomes cpio_load() or something like that).
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 4/4] Remove unnecessary cpio_init function

[Stefano Brivio]

On Sat, 6 Apr 2024 14:11:09 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Fri, Apr 05, 2024 at 08:08:53PM +0200, Stefano Brivio wrote:
> > On Fri, 22 Mar 2024 13:27:39 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >   
> > > The cpio_init function is now only every called with the "discard" option.
> > > But, moreover, what it does is create an initial mostly empty archive which
> > > will just get overwritten by the final archive.
> > > 
> > > So, it's entirely unnecessary except for one subtlety.  Our use of realpath
> > > when generating the final output requires that a file already exist in the
> > > output location.  We can fix that by shuffling some things out of a
> > > subshell, removing the need for realpath.
> > > 
> > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > > ---
> > >  mbuto | 31 ++++---------------------------
> > >  1 file changed, 4 insertions(+), 27 deletions(-)
> > > 
> > > diff --git a/mbuto b/mbuto
> > > index 550f76e..0c51e18 100755
> > > --- a/mbuto
> > > +++ b/mbuto
> > > @@ -544,28 +544,6 @@ subopts_get() {
> > >  
> > >  ### CPIO #######################################################################
> > >  
> > > -# cpio_init() - Source existing CPIO archive, or create if needed
> > > -# $1:	Path to CPIO archive, might exist, might be discarded if existing
> > > -cpio_init() {
> > > -	if [ -f "${OUT}" ] && [ "${1}" != "discard" ]; then
> > > -		info "Sourcing CPIO archive from ${OUT}"
> > > -
> > > -		if ! "${GZIP}" -dfc "${OUT}" |
> > > -		   "${CPIO}" --quiet -iD "${wd}"; then
> > > -			err "Invalid CPIO archive ${OUT}"
> > > -		fi
> > > -	else  
> > 
> > This, and the second part of 1/4, remove a functionality which I
> > accidentally broke in commit b87e4f2e6595 ("mbuto: Create working
> > directory before profiles are sourced"): it was once possible to add
> > contents to an existing initramfs archive.  
> 
> Yeah, I didn't track it to a specific point, but I assumed that it was
> built to add to an existing archive in the past.  It looked fairly
> thoroughly broken to me.

Well, minus that change, that was the only way I used this tool for a
while.

> > I guess I might be the only interactive user of mbuto at the moment, so I
> > got slightly annoyed by the fact it didn't work anymore but I didn't
> > really investigate further. I used it whenever an initramfs took more than
> > 5-10 seconds to build, and I'd keep forgetting to add stuff.  
> 
> So... in fact the kernel accepts not just a single cpio archive, but
> multiple.  So rather than doing anything with archivemount or mangling
> the cpio, you could just append a secondary cpio with the additions.

Okay, good to know, but at least at a glance, I couldn't find out how
to do this. I suppose we would need to decompress and compress the
archive anyway, right?

> > Note that the help message still describes this mode of operation:
> > 
> >   "Build initramfs image unless an existing one is passed."
> > 
> > and in the "Examples":
> > 
> > 	./mbuto -f kata.img zsh_5.6.2-3_amd64.deb
> > 		Install zsh package to pre-existing kata.img  
> 
> Ah, yes, they should be updated to match.

Right, with this updated I'm fine with this patch (and with the whole
series with my comment to 3/4 addressed). I'm still convinced we should
fix that and add it back, but it's not made more complicated by removing
it now.

> > but I also realised it's not convenient to have this as default, because,
> > especially for initramfs images that can be built faster, one might just
> > want to rebuild a single binary and the image, repeatedly.
> > 
> > Perhaps, instead of dropping this altogether, we could introduce a new
> > -K ("keep") option that skips the unconditional wd assignment and calls
> > cpio_init() (which becomes cpio_load() or something like that).

-- 
Stefano