diff --git a/pungi.spec b/pungi.spec index 545e4092..82d305f0 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 9a21eed6..c9566748 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 2d2df820..b74f2803 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 f4ff99d4..c88473ba 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 a4a00a14..7ebc2c3e 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), 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(['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), 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 e0dd2b1e..7529a259 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),