[elbe-devel] [PATCH] elbepack: unify losetup execution into context manager
Thomas Weißschuh
thomas.weissschuh at linutronix.de
Tue Apr 23 07:31:37 CEST 2024
Context managers simplify errorhandling significantly.
Furthermore it removes some strewn around usage of shellhelper APIs.
Signed-off-by: Thomas Weißschuh <thomas.weissschuh at linutronix.de>
---
debian/python3-elbe-common.install | 1 +
elbepack/finetuning.py | 9 +--
elbepack/fstab.py | 17 ++--
elbepack/hdimg.py | 161 +++++++++++++++++--------------------
elbepack/imgutils.py | 18 +++++
5 files changed, 101 insertions(+), 105 deletions(-)
diff --git a/debian/python3-elbe-common.install b/debian/python3-elbe-common.install
index 1ea515548424..d45ecf8ea42e 100644
--- a/debian/python3-elbe-common.install
+++ b/debian/python3-elbe-common.install
@@ -24,6 +24,7 @@
./usr/lib/python3.*/*-packages/elbepack/egpg.py
./usr/lib/python3.*/*-packages/elbepack/hashes.py
./usr/lib/python3.*/*-packages/elbepack/hdimg.py
+./usr/lib/python3.*/*-packages/elbepack/imgutils.py
./usr/lib/python3.*/*-packages/elbepack/initvmaction.py
./usr/lib/python3.*/*-packages/elbepack/isooptions.py
./usr/lib/python3.*/*-packages/elbepack/licencexml.py
diff --git a/elbepack/finetuning.py b/elbepack/finetuning.py
index b1dd912f6810..ada8b07290ee 100644
--- a/elbepack/finetuning.py
+++ b/elbepack/finetuning.py
@@ -16,10 +16,11 @@ from gpg.constants import PROTOCOL_OpenPGP
from elbepack.egpg import unlock_key
from elbepack.filesystem import ImgMountFilesystem
+from elbepack.imgutils import losetup
from elbepack.packers import default_packer, packers
from elbepack.repomanager import UpdateRepo
from elbepack.rpcaptcache import get_rpcaptcache
-from elbepack.shellhelper import chroot, do, get_command_out
+from elbepack.shellhelper import chroot, do
class FinetuningException(Exception):
@@ -551,15 +552,11 @@ class LosetupAction(FinetuningAction):
def execute_prj(self, buildenv, target, builddir):
imgname = self.node.et.attrib['img']
imgpath = os.path.join(builddir, imgname)
- cmd = f'losetup --find --show --partscan "{imgpath}"'
- loop_dev = get_command_out(cmd).decode().strip()
- try:
+ with losetup(imgpath) as loop_dev:
for i in self.node:
action = ImageFinetuningAction(i)
action.execute_img(buildenv, target, builddir, loop_dev)
- finally:
- do(f'losetup --detach "{loop_dev}"')
@FinetuningAction.register('img_convert')
diff --git a/elbepack/fstab.py b/elbepack/fstab.py
index c3f181c5bc34..cad1715ad5ac 100644
--- a/elbepack/fstab.py
+++ b/elbepack/fstab.py
@@ -7,7 +7,8 @@ import os
import subprocess
import time
-from elbepack.shellhelper import do, get_command_out
+from elbepack.imgutils import losetup
+from elbepack.shellhelper import do
def get_mtdnum(xml, label):
@@ -89,23 +90,17 @@ class hdpart:
self.number = f'{disk.type}{ppart.number}'
def losetup(self):
-
- cmd = (f'losetup --offset {self.offset} --sizelimit {self.size} '
- f'--find --show "{self.filename}"')
-
while True:
-
try:
- loopdev = get_command_out(cmd)
+ return losetup(self.filename, [
+ '--offset', str(self.offset),
+ '--sizelimit', str(self.size),
+ ])
except subprocess.CalledProcessError as e:
if e.returncode != 1:
raise
do('sync')
time.sleep(1)
- else:
- break
-
- return loopdev.decode().rstrip('\n')
class fstabentry(hdpart):
diff --git a/elbepack/hdimg.py b/elbepack/hdimg.py
index df5b4f5be181..8726438c78b8 100644
--- a/elbepack/hdimg.py
+++ b/elbepack/hdimg.py
@@ -11,7 +11,8 @@ import parted
from elbepack.filesystem import Filesystem, size_to_int
from elbepack.fstab import fstabentry, hdpart, mountpoint_dict
-from elbepack.shellhelper import chroot, do, get_command_out
+from elbepack.imgutils import losetup
+from elbepack.shellhelper import chroot, do
def mkfs_mtd(mtd, fslabel, target):
@@ -136,11 +137,6 @@ class grubinstaller_base:
def install(self, target, user_args):
pass
- @staticmethod
- def losetup(f):
- loopdev = get_command_out(f'losetup --find --show --partscan "{f}"')
- return loopdev.decode().rstrip('\n')
-
class grubinstaller202(grubinstaller_base):
@@ -151,45 +147,44 @@ class grubinstaller202(grubinstaller_base):
imagemnt = os.path.join(target, 'imagemnt')
imagemntfs = Filesystem(imagemnt)
try:
- loopdev = self.losetup(self.fs['/'].filename)
-
- for entry in self.fs.depthlist():
- do(
- 'mount '
- f'{loopdev}p{entry.partnum} '
- f'{imagemntfs.fname(entry.mountpoint)}')
-
- do(f"mount --bind /dev {imagemntfs.fname('dev')}")
- do(f"mount --bind /proc {imagemntfs.fname('proc')}")
- do(f"mount --bind /sys {imagemntfs.fname('sys')}")
-
- do(f'mkdir -p "{imagemntfs.fname("boot/grub")}"')
-
- devmap = open(imagemntfs.fname('boot/grub/device.map'), 'w')
- devmap.write(f'(hd0) {loopdev}\n')
- devmap.close()
-
- chroot(imagemnt, 'update-grub2')
-
- if 'efi' in self.fw_type:
- grub_tgt = next(t for t in self.fw_type if t.endswith('-efi'))
- do(
- f'chroot {imagemnt} '
- f'grub-install {user_args} --target={grub_tgt} --removable '
- f'--no-floppy {loopdev}')
- if 'shimfix' in self.fw_type:
- # grub-install is heavily dependent on the running system having
- # a BIOS or EFI. The initvm is BIOS-based, so fix the resulting
- # shim installation.
- do(f"chroot {imagemnt} /bin/bash -c '"
- 'cp -r /boot/efi/EFI/BOOT /boot/efi/EFI/debian && '
- 'cd /usr/lib/shim && f=( shim*.efi.signed ) && cp '
- "${f[0]} /boot/efi/EFI/debian/${f[0]%%.signed}'")
- if not self.fw_type or 'bios' in self.fw_type:
- do(
- f'chroot {imagemnt} '
- f'grub-install {user_args} --target=i386-pc '
- f'--no-floppy {loopdev}')
+ with losetup(self.fs['/'].filename) as loopdev:
+ for entry in self.fs.depthlist():
+ do(
+ 'mount '
+ f'{loopdev}p{entry.partnum} '
+ f'{imagemntfs.fname(entry.mountpoint)}')
+
+ do(f"mount --bind /dev {imagemntfs.fname('dev')}")
+ do(f"mount --bind /proc {imagemntfs.fname('proc')}")
+ do(f"mount --bind /sys {imagemntfs.fname('sys')}")
+
+ do(f'mkdir -p "{imagemntfs.fname("boot/grub")}"')
+
+ devmap = open(imagemntfs.fname('boot/grub/device.map'), 'w')
+ devmap.write(f'(hd0) {loopdev}\n')
+ devmap.close()
+
+ chroot(imagemnt, 'update-grub2')
+
+ if 'efi' in self.fw_type:
+ grub_tgt = next(t for t in self.fw_type if t.endswith('-efi'))
+ do(
+ f'chroot {imagemnt} '
+ f'grub-install {user_args} --target={grub_tgt} --removable '
+ f'--no-floppy {loopdev}')
+ if 'shimfix' in self.fw_type:
+ # grub-install is heavily dependent on the running system having
+ # a BIOS or EFI. The initvm is BIOS-based, so fix the resulting
+ # shim installation.
+ do(f"chroot {imagemnt} /bin/bash -c '"
+ 'cp -r /boot/efi/EFI/BOOT /boot/efi/EFI/debian && '
+ 'cd /usr/lib/shim && f=( shim*.efi.signed ) && cp '
+ "${f[0]} /boot/efi/EFI/debian/${f[0]%%.signed}'")
+ if not self.fw_type or 'bios' in self.fw_type:
+ do(
+ f'chroot {imagemnt} '
+ f'grub-install {user_args} --target=i386-pc '
+ f'--no-floppy {loopdev}')
except subprocess.CalledProcessError as E:
logging.error('Fail installing grub device: %s', E)
@@ -205,8 +200,6 @@ class grubinstaller202(grubinstaller_base):
f'umount {loopdev}p{entry.partnum}',
allow_fail=True)
- do(f'losetup -d {loopdev}', allow_fail=True)
-
class grubinstaller97(grubinstaller_base):
@@ -217,47 +210,47 @@ class grubinstaller97(grubinstaller_base):
imagemnt = os.path.join(target, 'imagemnt')
imagemntfs = Filesystem(imagemnt)
try:
- loopdev = self.losetup(self.fs['/'].filename)
+ with losetup(self.fs['/'].filename) as loopdev:
- bootentry = 0
+ bootentry = 0
- for entry in self.fs.depthlist():
- if entry.mountpoint.startswith('/boot'):
+ for entry in self.fs.depthlist():
+ if entry.mountpoint.startswith('/boot'):
+ bootentry_label = entry.label
+ bootentry = int(entry.partnum)
+ do(
+ 'mount '
+ f'{loopdev}p{entry.partnum} '
+ f'{imagemntfs.fname(entry.mountpoint)}')
+
+ if not bootentry:
bootentry_label = entry.label
bootentry = int(entry.partnum)
- do(
- 'mount '
- f'{loopdev}p{entry.partnum} '
- f'{imagemntfs.fname(entry.mountpoint)}')
- if not bootentry:
- bootentry_label = entry.label
- bootentry = int(entry.partnum)
+ do(f"mount --bind /dev {imagemntfs.fname('dev')}")
+ do(f"mount --bind /proc {imagemntfs.fname('proc')}")
+ do(f"mount --bind /sys {imagemntfs.fname('sys')}")
- do(f"mount --bind /dev {imagemntfs.fname('dev')}")
- do(f"mount --bind /proc {imagemntfs.fname('proc')}")
- do(f"mount --bind /sys {imagemntfs.fname('sys')}")
+ do(f'mkdir -p "{imagemntfs.fname("boot/grub")}"')
- do(f'mkdir -p "{imagemntfs.fname("boot/grub")}"')
+ devmap = open(imagemntfs.fname('boot/grub/device.map'), 'w')
+ devmap.write(f'(hd0) {loopdev}\n')
+ devmap.close()
- devmap = open(imagemntfs.fname('boot/grub/device.map'), 'w')
- devmap.write(f'(hd0) {loopdev}\n')
- devmap.close()
+ # Replace groot and kopt because else they will be given
+ # bad values
+ #
+ # FIXME - Pylint says: Using possibly undefined loop
+ # variable 'entry' (undefined-loop-variable). entry is
+ # defined in the previous for-loop.
+ do(rf'chroot {imagemnt} sed -in "s/^# groot=.*$/# groot=\(hd0,{bootentry - 1}\)/" /boot/grub/menu.lst') # noqa: E501
+ do(rf'chroot {imagemnt} sed -in "s/^# kopt=.*$/# kopt=root=LABEL={bootentry_label}/" /boot/grub/menu.lst') # noqa: E501
- # Replace groot and kopt because else they will be given
- # bad values
- #
- # FIXME - Pylint says: Using possibly undefined loop
- # variable 'entry' (undefined-loop-variable). entry is
- # defined in the previous for-loop.
- do(rf'chroot {imagemnt} sed -in "s/^# groot=.*$/# groot=\(hd0,{bootentry - 1}\)/" /boot/grub/menu.lst') # noqa: E501
- do(rf'chroot {imagemnt} sed -in "s/^# kopt=.*$/# kopt=root=LABEL={bootentry_label}/" /boot/grub/menu.lst') # noqa: E501
+ chroot(imagemnt, 'update-grub')
- chroot(imagemnt, 'update-grub')
-
- do(
- f'chroot {imagemnt} '
- f'grub-install {user_args} --no-floppy {loopdev}')
+ do(
+ f'chroot {imagemnt} '
+ f'grub-install {user_args} --no-floppy {loopdev}')
except subprocess.CalledProcessError as E:
logging.error('Fail installing grub device: %s', E)
@@ -273,8 +266,6 @@ class grubinstaller97(grubinstaller_base):
f'umount {loopdev}p{entry.partnum}',
allow_fail=True)
- do(f'losetup -d {loopdev}', allow_fail=True)
-
class simple_fstype:
def __init__(self, typ):
@@ -333,7 +324,7 @@ def create_label(disk, part, ppart, fslabel, target, grub):
loopdev = entry.losetup()
- try:
+ with entry.losetup() as loopdev:
do(
f'mkfs.{entry.fstype} {entry.mkfsopt} {entry.get_label_opt()} '
f'{loopdev}')
@@ -352,8 +343,6 @@ def create_label(disk, part, ppart, fslabel, target, grub):
allow_fail=True)
finally:
do(f'umount {loopdev}')
- finally:
- do(f'losetup -d {loopdev}')
return ppart
@@ -373,9 +362,7 @@ def create_binary(disk, part, ppart, target):
entry = hdpart()
entry.set_geometry(ppart, disk)
- loopdev = entry.losetup()
-
- try:
+ with entry.losetup() as loopdev:
# copy from buildenv if path starts with /
if part.text('binary')[0] == '/':
tmp = target + '/' + 'chroot' + part.text('binary')
@@ -384,8 +371,6 @@ def create_binary(disk, part, ppart, target):
tmp = target + '/' + part.text('binary')
do(f'dd if="{tmp}" of="{loopdev}"')
- finally:
- do(f'losetup -d "{loopdev}"')
def create_logical_partitions(disk,
diff --git a/elbepack/imgutils.py b/elbepack/imgutils.py
new file mode 100644
index 000000000000..c6c4f57bd240
--- /dev/null
+++ b/elbepack/imgutils.py
@@ -0,0 +1,18 @@
+# ELBE - Debian Based Embedded Rootfilesystem Builder
+# SPDX-License-Identifier: GPL-3.0-or-later
+# SPDX-FileCopyrightText: 2024 Linutronix GmbH
+
+import contextlib
+
+from elbepack.shellhelper import do, get_command_out
+
+
+ at contextlib.contextmanager
+def losetup(dev, extra_args=[]):
+ loopdev = get_command_out(
+ f'losetup --find --show --partscan {" ".join(extra_args)} "{dev}"'
+ ).decode('ascii').rstrip('\n')
+
+ yield loopdev
+
+ do(f'losetup --detach {loopdev}', allow_fail=True)
---
base-commit: 9d8411747148c0bc88935c39f13387fcae5238fb
change-id: 20240422-losetup-485984b9213d
Best regards,
--
Thomas Weißschuh <thomas.weissschuh at linutronix.de>
More information about the elbe-devel
mailing list