From 01f01006fc38a563b5e3ffa116846e0504c4e064 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= Date: Tue, 16 Jan 2018 09:21:50 +0100 Subject: [PATCH] Add option to force fallback from guestmount --- ...mount-if-guestmount-is-not-available.patch | 217 ++++++++++++++++++ ...nstall_use_guestmount-boolean-option.patch | 65 ++++++ pungi.spec | 9 +- 3 files changed, 290 insertions(+), 1 deletion(-) create mode 100644 0001-Fall-back-to-mount-if-guestmount-is-not-available.patch create mode 100644 0002-Add-buildinstall_use_guestmount-boolean-option.patch diff --git a/0001-Fall-back-to-mount-if-guestmount-is-not-available.patch b/0001-Fall-back-to-mount-if-guestmount-is-not-available.patch new file mode 100644 index 00000000..18cff429 --- /dev/null +++ b/0001-Fall-back-to-mount-if-guestmount-is-not-available.patch @@ -0,0 +1,217 @@ +From a07674254acb562332579a42a25c44c1a0e0bac6 Mon Sep 17 00:00:00 2001 +From: Ondrej Nosek +Date: Thu, 21 Dec 2017 03:18:00 +0100 +Subject: [PATCH 1/2] Fall back to mount if guestmount is not available + +Relates: https://pagure.io/pungi/issue/803 + +Signed-off-by: Ondrej Nosek +--- + pungi.spec | 1 - + pungi/phases/product_img.py | 9 +++++++-- + pungi/util.py | 6 +----- + pungi/wrappers/iso.py | 19 ++++++++++++++----- + tests/test_iso_wrapper.py | 37 +++++++++++++++++++++++++++++++------ + tests/test_util.py | 2 +- + 6 files changed, 54 insertions(+), 20 deletions(-) + +diff --git a/pungi.spec b/pungi.spec +index 545e409..82d305f 100644 +--- a/pungi.spec ++++ b/pungi.spec +@@ -42,7 +42,6 @@ Requires: gettext + Requires: syslinux + Requires: git + Requires: python-jsonschema +-Requires: libguestfs-tools-c + Requires: python-enum34 + Requires: python2-dnf + Requires: python2-multilib +diff --git a/pungi/phases/product_img.py b/pungi/phases/product_img.py +index 9a21eed..c956674 100644 +--- a/pungi/phases/product_img.py ++++ b/pungi/phases/product_img.py +@@ -131,6 +131,9 @@ def create_product_img(compose, arch, variant): + + shutil.rmtree(po_tmp) + ++ ret, __ = run(["which", "guestmount"], can_fail=True) ++ guestmount_available = not bool(ret) # return code 0 means that guestmount is available ++ + mount_tmp = compose.mkdtemp(prefix="product_img_mount_") + cmds = [ + # allocate image +@@ -139,7 +142,8 @@ def create_product_img(compose, arch, variant): + "mke2fs -F %s" % shlex_quote(image), + # use guestmount to mount the image, which doesn't require root privileges + # LIBGUESTFS_BACKEND=direct: running qemu directly without libvirt +- "LIBGUESTFS_BACKEND=direct guestmount -a %s -m /dev/sda %s" % (shlex_quote(image), shlex_quote(mount_tmp)), ++ "LIBGUESTFS_BACKEND=direct guestmount -a %s -m /dev/sda %s" % (shlex_quote(image), shlex_quote(mount_tmp)) if guestmount_available ++ else "mount -o loop %s %s" % (shlex_quote(image), shlex_quote(mount_tmp)), + "mkdir -p %s/run/install/product" % shlex_quote(mount_tmp), + "cp -rp %s/* %s/run/install/product/" % (shlex_quote(product_tmp), shlex_quote(mount_tmp)), + "mkdir -p %s/run/install/product/pyanaconda" % shlex_quote(mount_tmp), +@@ -149,7 +153,8 @@ def create_product_img(compose, arch, variant): + "ln -s run/install/product/locale %s" % shlex_quote(mount_tmp), + # compat symlink: run/install/product/pyanaconda/installclasses -> ../installclasses + "ln -s ../installclasses %s/run/install/product/pyanaconda/installclasses" % shlex_quote(mount_tmp), +- "fusermount -u %s" % shlex_quote(mount_tmp), ++ "fusermount -u %s" % shlex_quote(mount_tmp) if guestmount_available ++ else "umount %s" % shlex_quote(mount_tmp), + # tweak last mount path written in the image + "tune2fs -M /run/install/product %s" % shlex_quote(image), + ] +diff --git a/pungi/util.py b/pungi/util.py +index 2d2df82..b74f280 100644 +--- a/pungi/util.py ++++ b/pungi/util.py +@@ -36,6 +36,7 @@ from productmd.common import get_major_version + # Patterns that match all names of debuginfo packages + DEBUG_PATTERNS = ["*-debuginfo", "*-debuginfo-*", "*-debugsource"] + ++ + def _doRunCommand(command, logger, rundir='/tmp', output=subprocess.PIPE, error=subprocess.PIPE, env=None): + """Run a command and log the output. Error out if we get something on stderr""" + +@@ -609,11 +610,6 @@ def temp_dir(log=None, *args, **kwargs): + log.warning('Error removing %s: %s', dir, exc.strerror) + + +-def fusermount(path, **kwargs): +- """Run fusermount -u on a given path.""" +- run_unmount_cmd(['fusermount', '-u', path], path=path, **kwargs) +- +- + def run_unmount_cmd(cmd, max_retries=10, path=None, logger=None): + """Attempt to run the command to unmount an image. + +diff --git a/pungi/wrappers/iso.py b/pungi/wrappers/iso.py +index 7e2c710..9ed3f24 100644 +--- a/pungi/wrappers/iso.py ++++ b/pungi/wrappers/iso.py +@@ -385,11 +385,17 @@ def mount(image, logger=None): + The yielded path will only be valid in the with block and is removed once + the image is unmounted. + """ +- # use guestmount to mount the image, which doesn't require root privileges +- # LIBGUESTFS_BACKEND=direct: running qemu directly without libvirt + with util.temp_dir(prefix='iso-mount-') as mount_dir: +- env = {'LIBGUESTFS_BACKEND': 'direct', 'LIBGUESTFS_DEBUG': '1', 'LIBGUESTFS_TRACE': '1'} +- cmd = ["guestmount", "-a", image, "-m", "/dev/sda", mount_dir] ++ ret, __ = run(["which", "guestmount"], can_fail=True) ++ guestmount_available = not bool(ret) # return code 0 means that guestmount is available ++ if guestmount_available: ++ # use guestmount to mount the image, which doesn't require root privileges ++ # LIBGUESTFS_BACKEND=direct: running qemu directly without libvirt ++ env = {'LIBGUESTFS_BACKEND': 'direct', 'LIBGUESTFS_DEBUG': '1', 'LIBGUESTFS_TRACE': '1'} ++ cmd = ["guestmount", "-a", image, "-m", "/dev/sda", mount_dir] ++ else: ++ env = {} ++ cmd = ["mount", "-o", "loop", image, mount_dir] + ret, out = run(cmd, env=env, can_fail=True, universal_newlines=True) + if ret != 0: + # The mount command failed, something is wrong. Log the output and raise an exception. +@@ -400,4 +406,7 @@ def mount(image, logger=None): + try: + yield mount_dir + finally: +- util.fusermount(mount_dir, logger=logger) ++ if guestmount_available: ++ util.run_unmount_cmd(['fusermount', '-u', mount_dir], path=mount_dir) ++ else: ++ util.run_unmount_cmd(['umount', mount_dir], path=mount_dir) +diff --git a/tests/test_iso_wrapper.py b/tests/test_iso_wrapper.py +index a4a00a1..7ebc2c3 100644 +--- a/tests/test_iso_wrapper.py ++++ b/tests/test_iso_wrapper.py +@@ -20,6 +20,7 @@ INCORRECT_OUTPUT = '''This should never happen: File not found''' + + + class TestIsoUtils(unittest.TestCase): ++ + @mock.patch('pungi.wrappers.iso.run') + def test_get_implanted_md5_correct(self, mock_run): + mock_run.return_value = (0, CORRECT_OUTPUT) +@@ -44,24 +45,48 @@ class TestIsoUtils(unittest.TestCase): + @mock.patch('pungi.util.run_unmount_cmd') + @mock.patch('pungi.wrappers.iso.run') + def test_mount_iso(self, mock_run, mock_unmount): +- mock_run.return_value = (0, '') ++ # first tuple is return value for command 'which guestmount' ++ # value determines type of the mount/unmount command ('1' - guestmount is not available) ++ # for approach as a root, pair commands mount-umount are used ++ mock_run.side_effect = [(1, ''), (0, '')] ++ with iso.mount('dummy') as temp_dir: ++ self.assertTrue(os.path.isdir(temp_dir)) ++ self.assertEqual(len(mock_run.call_args_list), 2) ++ mount_call_str = str(mock_run.call_args_list[1]) ++ self.assertTrue(mount_call_str.startswith("call(['mount'")) ++ self.assertEqual(len(mock_unmount.call_args_list), 1) ++ unmount_call_str = str(mock_unmount.call_args_list[0]) ++ self.assertTrue(unmount_call_str.startswith("call(['umount'")) ++ self.assertFalse(os.path.isdir(temp_dir)) ++ ++ @mock.patch('pungi.util.run_unmount_cmd') ++ @mock.patch('pungi.wrappers.iso.run') ++ def test_guestmount(self, mock_run, mock_unmount): ++ # first tuple is return value for command 'which guestmount' ++ # value determines type of the mount/unmount command ('0' - guestmount is available) ++ # for approach as a non-root, pair commands guestmount-fusermount are used ++ mock_run.side_effect = [(0, ''), (0, '')] + with iso.mount('dummy') as temp_dir: + self.assertTrue(os.path.isdir(temp_dir)) +- self.assertEqual(len(mock_run.call_args_list), 1) ++ self.assertEqual(len(mock_run.call_args_list), 2) ++ mount_call_str = str(mock_run.call_args_list[1]) ++ self.assertTrue(mount_call_str.startswith("call(['guestmount'")) + self.assertEqual(len(mock_unmount.call_args_list), 1) ++ unmount_call_str = str(mock_unmount.call_args_list[0]) ++ self.assertTrue(unmount_call_str.startswith("call(['fusermount'")) + self.assertFalse(os.path.isdir(temp_dir)) + + @mock.patch('pungi.util.run_unmount_cmd') + @mock.patch('pungi.wrappers.iso.run') + def test_mount_iso_always_unmounts(self, mock_run, mock_unmount): +- mock_run.return_value = (0, '') ++ mock_run.side_effect = [(1, ''), (0, '')] + try: + with iso.mount('dummy') as temp_dir: + self.assertTrue(os.path.isdir(temp_dir)) + raise RuntimeError() + except RuntimeError: + pass +- self.assertEqual(len(mock_run.call_args_list), 1) ++ self.assertEqual(len(mock_run.call_args_list), 2) + self.assertEqual(len(mock_unmount.call_args_list), 1) + self.assertFalse(os.path.isdir(temp_dir)) + +@@ -69,11 +94,11 @@ class TestIsoUtils(unittest.TestCase): + @mock.patch('pungi.wrappers.iso.run') + def test_mount_iso_raises_on_error(self, mock_run, mock_unmount): + log = mock.Mock() +- mock_run.return_value = (1, 'Boom') ++ mock_run.side_effect = [(1, ''), (1, 'Boom')] + with self.assertRaises(RuntimeError): + with iso.mount('dummy', logger=log) as temp_dir: + self.assertTrue(os.path.isdir(temp_dir)) +- self.assertEqual(len(mock_run.call_args_list), 1) ++ self.assertEqual(len(mock_run.call_args_list), 2) + self.assertEqual(len(mock_unmount.call_args_list), 0) + self.assertEqual(len(log.mock_calls), 1) + +diff --git a/tests/test_util.py b/tests/test_util.py +index e0dd2b1..7529a25 100644 +--- a/tests/test_util.py ++++ b/tests/test_util.py +@@ -446,7 +446,7 @@ class TestUnmountCmd(unittest.TestCase): + self._fakeProc(0, out='It is very busy'), + self._fakeProc(1, out='lsof output')] + with self.assertRaises(RuntimeError) as ctx: +- util.fusermount('/path', max_retries=3, logger=logger) ++ util.run_unmount_cmd(['fusermount', '-u', '/path'], path='/path', max_retries=3, logger=logger) + cmd = ['fusermount', '-u', '/path'] + expected = [mock.call(cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE, + universal_newlines=True), +-- +2.13.6 + diff --git a/0002-Add-buildinstall_use_guestmount-boolean-option.patch b/0002-Add-buildinstall_use_guestmount-boolean-option.patch new file mode 100644 index 00000000..fa546e2c --- /dev/null +++ b/0002-Add-buildinstall_use_guestmount-boolean-option.patch @@ -0,0 +1,65 @@ +From 91ce7c7169b9313c94e9f72b2d9374851747f424 Mon Sep 17 00:00:00 2001 +From: Jan Kaluza +Date: Tue, 16 Jan 2018 09:13:55 +0100 +Subject: [PATCH 2/2] Add buildinstall_use_guestmount boolean option + +Signed-off-by: Jan Kaluza +--- + pungi/checks.py | 1 + + pungi/phases/buildinstall.py | 4 +++- + pungi/wrappers/iso.py | 5 +++-- + 3 files changed, 7 insertions(+), 3 deletions(-) + +diff --git a/pungi/checks.py b/pungi/checks.py +index 9fc40b8..5438204 100644 +--- a/pungi/checks.py ++++ b/pungi/checks.py +@@ -708,6 +708,7 @@ def make_schema(): + }, + "buildinstall_topdir": {"type": "string"}, + "buildinstall_kickstart": {"$ref": "#/definitions/str_or_scm_dict"}, ++ "buildinstall_use_guestmount": {"type": "boolean", "default": True}, + + "global_ksurl": {"type": "string"}, + "global_version": {"type": "string"}, +diff --git a/pungi/phases/buildinstall.py b/pungi/phases/buildinstall.py +index bbc06ae..e2c8d79 100644 +--- a/pungi/phases/buildinstall.py ++++ b/pungi/phases/buildinstall.py +@@ -305,7 +305,9 @@ def tweak_buildinstall(compose, src, dst, arch, variant, label, volid, kickstart + if not os.path.isfile(image): + continue + +- with iso.mount(image, logger=compose._logger) as mount_tmp_dir: ++ with iso.mount(image, logger=compose._logger, ++ use_guestmount=compose.conf.get("buildinstall_use_guestmount") ++ ) as mount_tmp_dir: + for config in BOOT_CONFIGS: + config_path = os.path.join(tmp_dir, config) + config_in_image = os.path.join(mount_tmp_dir, config) +diff --git a/pungi/wrappers/iso.py b/pungi/wrappers/iso.py +index 9ed3f24..9bca337 100644 +--- a/pungi/wrappers/iso.py ++++ b/pungi/wrappers/iso.py +@@ -379,7 +379,7 @@ def graft_point_sort_key(x): + + + @contextlib.contextmanager +-def mount(image, logger=None): ++def mount(image, logger=None, use_guestmount=True): + """Mount an image and make sure it's unmounted. + + The yielded path will only be valid in the with block and is removed once +@@ -387,7 +387,8 @@ def mount(image, logger=None): + """ + with util.temp_dir(prefix='iso-mount-') as mount_dir: + ret, __ = run(["which", "guestmount"], can_fail=True) +- guestmount_available = not bool(ret) # return code 0 means that guestmount is available ++ # return code 0 means that guestmount is available ++ guestmount_available = use_guestmount and not bool(ret) + if guestmount_available: + # use guestmount to mount the image, which doesn't require root privileges + # LIBGUESTFS_BACKEND=direct: running qemu directly without libvirt +-- +2.13.6 + diff --git a/pungi.spec b/pungi.spec index a432e247..6be4229a 100644 --- a/pungi.spec +++ b/pungi.spec @@ -1,6 +1,6 @@ Name: pungi Version: 4.1.21 -Release: 3%{?dist} +Release: 4%{?dist} Summary: Distribution compose tool Group: Development/Tools @@ -8,6 +8,8 @@ License: GPLv2 URL: https://pagure.io/pungi Source0: https://pagure.io/releases/%{name}/%{name}-%{version}.tar.bz2 Patch0: 0001-gather-Fix-checking-string-type.patch +Patch1: 0001-Fall-back-to-mount-if-guestmount-is-not-available.patch +Patch2: 0002-Add-buildinstall_use_guestmount-boolean-option.patch BuildRequires: python3-nose BuildRequires: python3-mock BuildRequires: python3-devel @@ -79,6 +81,8 @@ notification to Fedora Message Bus. %prep %setup -q %patch0 -p1 +%patch1 -p1 +%patch2 -p1 %build %{__python3} setup.py build @@ -125,6 +129,9 @@ nosetests-3 --exe %{_bindir}/%{name}-wait-for-signed-ostree-handler %changelog +* Tue Jan 16 2018 Lubomír Sedlář - 4.1.21-4 +- Add option to force fallback from guestmount + * Wed Jan 10 2018 Lubomír Sedlář - 4.1.21-3 - Fix checking string type in nodeps method