public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Stefano Brivio <sbrivio@redhat.com>
Cc: passt-dev@passt.top, lvivier@redhat.com,
	David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH v2 2/4] Remove stale archivemount support
Date: Fri, 26 Apr 2024 12:01:06 +1000	[thread overview]
Message-ID: <20240426020108.1522375-3-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20240426020108.1522375-1-david@gibson.dropbear.id.au>

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 appears the archivemount approach was there to allow some later changes
to the archive without re-running mbuto.  However, it's essentially
impossible to use this safely - if archivemount still has the archive open
for writing, it's not safe to read it concurrently.  This appears to have
worked by accident with some earlier versions, but doesn't work now.

Just remove the archivemount approach entirely.

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


  parent reply	other threads:[~2024-04-26  2:01 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-26  2:01 [PATCH v2 0/4] mbuto: Assorted fixes and simplifications David Gibson
2024-04-26  2:01 ` [PATCH v2 1/4] ${wd} is always set, no need to test for it David Gibson
2024-04-26  2:01 ` David Gibson [this message]
2024-04-26  2:01 ` [PATCH v2 3/4] Split "auto" compression mode into its own path David Gibson
2024-04-26  2:01 ` [PATCH v2 4/4] Remove unnecessary cpio_init function David Gibson
2024-04-26  6:14 ` [PATCH v2 0/4] mbuto: Assorted fixes and simplifications Stefano Brivio

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240426020108.1522375-3-david@gibson.dropbear.id.au \
    --to=david@gibson.dropbear.id.au \
    --cc=lvivier@redhat.com \
    --cc=passt-dev@passt.top \
    --cc=sbrivio@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).