Blame SOURCES/virt-manager-urlfetcher-Add-xorriso-ISOReader-implementation.patch

daf011
From 2bda38a197a780a85e9ce448ea81a81fe866c981 Mon Sep 17 00:00:00 2001
daf011
From: Cole Robinson <crobinso@redhat.com>
daf011
Date: Wed, 7 Apr 2021 11:45:00 -0400
daf011
Subject: [PATCH] urlfetcher: Add xorriso ISOReader implementation
daf011
daf011
xorisso is the still maintained isoinfo alternative, and may be
daf011
the only iso reading tool in RHEL9, so we need to support it.
daf011
Make it the default for our spec file and test suite too
daf011
daf011
Signed-off-by: Cole Robinson <crobinso@redhat.com>
daf011
(cherry picked from commit f793986378f84bb409d2451bdb62ca08fd4cb5b4)
daf011
daf011
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1973236
daf011
daf011
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
daf011
---
daf011
 man/virt-install.rst           |  2 +-
daf011
 tests/test_cli.py              | 12 ++++-----
daf011
 virt-manager.spec              |  4 +--
daf011
 virtinst/install/urldetect.py  |  4 +--
daf011
 virtinst/install/urlfetcher.py | 48 +++++++++++++++++++++++++---------
daf011
 5 files changed, 47 insertions(+), 23 deletions(-)
daf011
daf011
diff --git a/man/virt-install.rst b/man/virt-install.rst
daf011
index 963f9564..f75af635 100644
daf011
--- a/man/virt-install.rst
daf011
+++ b/man/virt-install.rst
daf011
@@ -617,7 +617,7 @@ ftp://host/path
daf011
     An FTP server location containing an installable distribution image.
daf011
 
daf011
 ISO
daf011
-    Probe the ISO and extract files using 'isoinfo'
daf011
+    Extract files directly from the ISO path
daf011
 
daf011
 DIRECTORY
daf011
     Path to a local directory containing an installable distribution image.
daf011
diff --git a/tests/test_cli.py b/tests/test_cli.py
daf011
index 5e69a135..3534e0e2 100644
daf011
--- a/tests/test_cli.py
daf011
+++ b/tests/test_cli.py
daf011
@@ -39,7 +39,7 @@ MEDIA_DIR = os.path.relpath(utils.DATADIR + "/fakemedia", utils.TOPDIR)
daf011
 UNATTENDED_DIR = XMLDIR + "/unattended"
daf011
 OLD_OSINFO = utils.has_old_osinfo()
daf011
 NO_OSINFO_UNATTEND = not unattended.OSInstallScript.have_new_libosinfo()
daf011
-HAS_ISOINFO = shutil.which("isoinfo")
daf011
+HAS_xorriso = shutil.which("xorriso")
daf011
 
daf011
 # We use this check as a surrogate for a released libosinfo with a bug
daf011
 # fix we need to get full test coverage
daf011
@@ -100,9 +100,9 @@ def has_old_osinfo():
daf011
         return "osinfo is too old"
daf011
 
daf011
 
daf011
-def missing_isoinfo():
daf011
-    if not HAS_ISOINFO:
daf011
-        return "isoinfo not installed"
daf011
+def missing_xorriso():
daf011
+    if not HAS_xorriso:
daf011
+        return "xorriso not installed"
daf011
 
daf011
 
daf011
 def no_osinfo_unattend_cb():
daf011
@@ -995,8 +995,8 @@ c.add_compare("--connect " + utils.URIs.kvm_session + " --disk size=8 --os-varia
daf011
 c.add_valid("--connect " + utils.URIs.kvm_session + " --install fedora21", prerun_check=has_old_osinfo)  # hits some get_search_paths and media_upload code paths
daf011
 
daf011
 # misc KVM config tests
daf011
-c.add_compare("--disk none --location %(ISO-NO-OS)s,kernel=frib.img,initrd=/frob.img", "location-manual-kernel", prerun_check=missing_isoinfo)  # --location with an unknown ISO but manually specified kernel paths
daf011
-c.add_compare("--disk %(EXISTIMG1)s --location %(ISOTREE)s --nonetworks", "location-iso", prerun_check=missing_isoinfo)  # Using --location iso mounting
daf011
+c.add_compare("--disk none --location %(ISO-NO-OS)s,kernel=frib.img,initrd=/frob.img", "location-manual-kernel", prerun_check=missing_xorriso)  # --location with an unknown ISO but manually specified kernel paths
daf011
+c.add_compare("--disk %(EXISTIMG1)s --location %(ISOTREE)s --nonetworks", "location-iso", prerun_check=missing_xorriso)  # Using --location iso mounting
daf011
 c.add_compare("--disk %(EXISTIMG1)s --cdrom %(ISOLABEL)s", "cdrom-centos-label")  # Using --cdrom with centos CD label, should use virtio etc.
daf011
 c.add_compare("--disk %(EXISTIMG1)s --install bootdev=network --os-variant rhel5.4 --cloud-init none", "kvm-rhel5")  # RHEL5 defaults
daf011
 c.add_compare("--disk %(EXISTIMG1)s --install kernel=%(ISO-WIN7)s,initrd=%(ISOLABEL)s,kernel_args='foo bar' --os-variant rhel6.4 --unattended none", "kvm-rhel6")  # RHEL6 defaults. ISO paths are just to point at existing files
daf011
diff --git a/virt-manager.spec b/virt-manager.spec
daf011
index f523551b..3946c300 100644
daf011
--- a/virt-manager.spec
daf011
+++ b/virt-manager.spec
daf011
@@ -71,8 +71,8 @@ Requires: python3-requests
daf011
 Requires: libosinfo >= 0.2.10
daf011
 # Required for gobject-introspection infrastructure
daf011
 Requires: python3-gobject-base
daf011
-# Required for pulling files from iso media with isoinfo
daf011
-Requires: genisoimage
daf011
+# Required for pulling files from iso media
daf011
+Requires: xorriso
daf011
 
daf011
 %description common
daf011
 Common files used by the different virt-manager interfaces, as well as
daf011
diff --git a/virtinst/install/urldetect.py b/virtinst/install/urldetect.py
daf011
index a73b0bf1..f5ed0270 100644
daf011
--- a/virtinst/install/urldetect.py
daf011
+++ b/virtinst/install/urldetect.py
daf011
@@ -40,9 +40,9 @@ class _DistroCache(object):
daf011
         if path not in self._filecache:
daf011
             try:
daf011
                 content = self._fetcher.acquireFileContent(path)
daf011
-            except ValueError:
daf011
+            except ValueError as e:
daf011
                 content = None
daf011
-                log.debug("Failed to acquire file=%s", path)
daf011
+                log.debug("Failed to acquire file=%s: %s", path, e)
daf011
             self._filecache[path] = content
daf011
         return self._filecache[path]
daf011
 
daf011
diff --git a/virtinst/install/urlfetcher.py b/virtinst/install/urlfetcher.py
daf011
index 3cacab1a..835c9e40 100644
daf011
--- a/virtinst/install/urlfetcher.py
daf011
+++ b/virtinst/install/urlfetcher.py
daf011
@@ -26,7 +26,7 @@ class _ISOReader:
daf011
     def __init__(self, location):
daf011
         self._location = location
daf011
 
daf011
-    def grabFile(self, url):
daf011
+    def grabFile(self, url, scratchdir):
daf011
         raise NotImplementedError()
daf011
     def hasFile(self, url):
daf011
         raise NotImplementedError()
daf011
@@ -43,20 +43,50 @@ class _ISOinfoReader(_ISOReader):
daf011
     def _make_file_list(self):
daf011
         cmd = ["isoinfo", "-J", "-i", self._location, "-f"]
daf011
 
daf011
-        log.debug("Running isoinfo: %s", cmd)
daf011
+        log.debug("Generating iso filelist: %s", cmd)
daf011
         output = subprocess.check_output(cmd, stderr=subprocess.DEVNULL)
daf011
         return output.splitlines(False)
daf011
 
daf011
-    def grabFile(self, url):
daf011
+    def grabFile(self, url, scratchdir):
daf011
+        ignore = scratchdir
daf011
         cmd = ["isoinfo", "-J", "-i", self._location, "-x", url]
daf011
 
daf011
-        log.debug("Running isoinfo: %s", cmd)
daf011
+        log.debug("Extracting iso file: %s", cmd)
daf011
         return subprocess.check_output(cmd)
daf011
 
daf011
     def hasFile(self, url):
daf011
         return url.encode("ascii") in self._cache_file_list
daf011
 
daf011
 
daf011
+class _XorrisoReader(_ISOReader):
daf011
+    def __init__(self, location):
daf011
+        super().__init__(location)
daf011
+        self._cache_file_list = self._make_file_list()
daf011
+
daf011
+    def _make_file_list(self):
daf011
+        delim = "VIRTINST_BEGINLIST"
daf011
+        cmd = ["xorriso", "-indev", self._location, "-print", delim, "-find"]
daf011
+
daf011
+        log.debug("Generating iso filelist: %s", cmd)
daf011
+        output = subprocess.check_output(cmd,
daf011
+                stderr=subprocess.DEVNULL, text=True)
daf011
+        return output.split(delim, 1)[1].strip().splitlines()
daf011
+
daf011
+    def grabFile(self, url, scratchdir):
daf011
+        tmp = tempfile.NamedTemporaryFile(
daf011
+                prefix="virtinst-iso", suffix="-" + os.path.basename(url),
daf011
+                dir=scratchdir)
daf011
+
daf011
+        cmd = ["xorriso", "-osirrox", "on", "-indev", self._location,
daf011
+               "-extract", url, tmp.name]
daf011
+        log.debug("Extracting iso file: %s", cmd)
daf011
+        subprocess.check_output(cmd)
daf011
+        return open(tmp.name, "rb").read()
daf011
+
daf011
+    def hasFile(self, url):
daf011
+        return ("'.%s'" % url) in self._cache_file_list
daf011
+
daf011
+
daf011
 ###########################
daf011
 # Fetcher implementations #
daf011
 ###########################
daf011
@@ -349,23 +379,17 @@ class _ISOURLFetcher(_URLFetcher):
daf011
 
daf011
     def _get_isoreader(self):
daf011
         if not self._isoreader:
daf011
-            self._isoreader = _ISOinfoReader(self.location)
daf011
+            self._isoreader = _XorrisoReader(self.location)
daf011
         return self._isoreader
daf011
 
daf011
     def _grabber(self, url):
daf011
-        """
daf011
-        Use isoinfo to grab the file
daf011
-        """
daf011
         if not self._hasFile(url):
daf011
             raise RuntimeError("iso doesn't have file=%s" % url)
daf011
 
daf011
-        output = self._get_isoreader().grabFile(url)
daf011
+        output = self._get_isoreader().grabFile(url, self.scratchdir)
daf011
         return io.BytesIO(output), len(output)
daf011
 
daf011
     def _hasFile(self, url):
daf011
-        """
daf011
-        Use isoinfo to list and search for the file
daf011
-        """
daf011
         return self._get_isoreader().hasFile(url)
daf011
 
daf011
 
daf011
-- 
daf011
2.31.1
daf011