[et-mgmt-tools] [PATCH] virtinst: make hasFile not grab files

Cole Robinson crobinso at redhat.com
Fri Feb 1 23:19:24 UTC 2008


The attached patch changes the implementation of hasFile from
ImageFetcher classes to not actually acquire the file it
is looking for, rather just test for its existence. http
will use a HEAD request, local mounts will check the path,
and ftp will query the size (which will work for files), 
and if that fails will try to cd to the location.

I gave all the cases a test but I may be missing something,
so if anyone has suggestions, please recommend :) This will
be nice to have as it opens up some options, ex. quickly
probing a tree for a paravirt kernel.

Thanks,
Cole

# HG changeset patch
# User "Cole Robinson <crobinso at redhat.com>"
# Date 1201905006 18000
# Node ID 96dd51a7e06b2bdf0dbc9195d3047885f9f5eb2e
# Parent  375d3f83844c8e374347d391fdc874c7b24fef1b
Re-implement hasFile to not pull down any files, just test they exist.

diff -r 375d3f83844c -r 96dd51a7e06b virtinst/DistroManager.py
--- a/virtinst/DistroManager.py	Fri Feb 01 09:56:47 2008 -0500
+++ b/virtinst/DistroManager.py	Fri Feb 01 17:30:06 2008 -0500
@@ -32,7 +32,8 @@ from virtinst import _virtinst as _
 from virtinst import _virtinst as _
 
 from ImageFetcher import MountedImageFetcher
-from ImageFetcher import URIImageFetcher
+from ImageFetcher import FTPImageFetcher
+from ImageFetcher import HTTPImageFetcher
 from ImageFetcher import DirectImageFetcher
 
 from OSDistro import FedoraDistro
@@ -45,8 +46,10 @@ from OSDistro import MandrivaDistro
 from OSDistro import MandrivaDistro
 
 def _fetcherForURI(uri, scratchdir=None):
-    if uri.startswith("http://") or uri.startswith("ftp://"):
-        return URIImageFetcher(uri, scratchdir)
+    if uri.startswith("http://"): 
+        return HTTPImageFetcher(uri, scratchdir)
+    elif uri.startswith("ftp://"):
+        return FTPImageFetcher(uri, scratchdir)
     elif uri.startswith("nfs://"):
         return MountedImageFetcher(uri, scratchdir)
     else:
@@ -57,6 +60,7 @@ def _fetcherForURI(uri, scratchdir=None)
 
 def _storeForDistro(fetcher, baseuri, type, progresscb, distro=None, scratchdir=None):
     stores = []
+    logging.debug("Attempting to detect distro:")
     if distro == "fedora" or distro is None:
         stores.append(FedoraDistro(baseuri, type, scratchdir))
     if distro == "rhel" or distro is None:
@@ -81,7 +85,7 @@ def _storeForDistro(fetcher, baseuri, ty
     raise ValueError, _("Could not find an installable distribution at '%s'" % baseuri) 
 
 
-# Method to fetch a krenel & initrd pair for a particular distro / HV type
+# Method to fetch a kernel & initrd pair for a particular distro / HV type
 def acquireKernel(baseuri, progresscb, scratchdir="/var/tmp", type=None, distro=None):
     fetcher = _fetcherForURI(baseuri, scratchdir)
     
diff -r 375d3f83844c -r 96dd51a7e06b virtinst/ImageFetcher.py
--- a/virtinst/ImageFetcher.py	Fri Feb 01 09:56:47 2008 -0500
+++ b/virtinst/ImageFetcher.py	Fri Feb 01 17:30:06 2008 -0500
@@ -25,6 +25,9 @@ import subprocess
 import subprocess
 import urlgrabber.grabber as grabber
 import urlgrabber.progress as progress
+import urllib2
+import urlparse
+import ftplib
 import tempfile
 from virtinst import _virtinst as _
 
@@ -58,10 +61,10 @@ class ImageFetcher:
     def acquireFile(self, src, progresscb):
         raise "Must be implemented in subclass"
 
-    def hasFile(self, src, progresscb):
+    def hasFile(self, src):
         raise "Must be implemented in subclass"
 
-# This is a fetcher capable of downloading from FTP / HTTP
+# Base class for downloading from FTP / HTTP
 class URIImageFetcher(ImageFetcher):
 
     def prepareLocation(self, progresscb):
@@ -72,7 +75,8 @@ class URIImageFetcher(ImageFetcher):
             return True
         except IOError, e:
             logging.debug("Opening URL %s failed." % (self.location,) + " " + str(e))
-            raise ValueError(_("Opening URL %s failed.") % (self.location,))
+            raise ValueError(_("Opening URL %s failed: %s") % \
+                              (self.location, e))
             return False
 
     def acquireFile(self, filename, progresscb):
@@ -85,7 +89,7 @@ class URIImageFetcher(ImageFetcher):
                                        progress_obj = progresscb, \
                                        text = _("Retrieving file %s...") % base)
             except IOError, e:
-                raise ValueError, _("Invalid URL location given: %s %s") %\
+                raise ValueError, _("Couldn't aquire file %s: %s") %\
                                   ((self.location + "/" + filename), str(e))
             tmpname = self.saveTemp(file, prefix=base + ".")
             logging.debug("Saved file to " + tmpname)
@@ -94,15 +98,35 @@ class URIImageFetcher(ImageFetcher):
             if file:
                 file.close()
 
-    def hasFile(self, filename, progresscb):
-        try:
-            tmpfile = self.acquireFile(filename, progresscb)
-            os.unlink(tmpfile)
-            return True
+class HTTPImageFetcher(URIImageFetcher):
+    
+    def hasFile(self, filename):
+        try:
+            request = urllib2.Request(self.location + "/" + filename)
+            request.get_method = lambda: "HEAD"
+            http_file = urllib2.urlopen(request)
         except Exception, e:
-            logging.debug("Cannot find file %s" % filename)
-            return False
-
+            logging.debug("HTTP hasFile: didn't find %s" % \
+                          (self.location + "/" + filename))
+            return False
+        return True
+
+class FTPImageFetcher(URIImageFetcher):
+    
+    def hasFile(self, filename):
+        url = urlparse.urlparse(self.location + "/" + filename)
+        try:
+            ftp = ftplib.FTP(url[1])
+            ftp.login()
+            try:
+                ftp.size(url[2])   # If a file
+            except ftplib.all_errors, e:
+                ftp.cwd(url[2])    # If a dir
+        except ftplib.all_errors, e:
+            logging.debug("FTP hasFile: couldn't access %s/%s" % \
+                          (url[1], url[2]))
+            return False
+        return True
 
 class LocalImageFetcher(ImageFetcher):
 
@@ -132,14 +156,12 @@ class LocalImageFetcher(ImageFetcher):
             if file:
                 file.close()
 
-    def hasFile(self, filename, progresscb):
-        try:
-            tmpfile = self.acquireFile(filename, progresscb)
-            if tmpfile is not None:
-                os.unlink(tmpfile)
+    def hasFile(self, filename):
+        if os.path.exists(os.path.abspath(self.srcdir + "/" + filename)):
             return True
-        except Exception, e:
-            logging.debug("Cannot find file %s" % filename)
+        else:
+            logging.debug("local hasFile: Couldn't find %s" % \
+                          (self.srcdir + "/" + filename))
             return False
 
 # This is a fetcher capable of extracting files from a NFS server
diff -r 375d3f83844c -r 96dd51a7e06b virtinst/OSDistro.py
--- a/virtinst/OSDistro.py	Fri Feb 01 09:56:47 2008 -0500
+++ b/virtinst/OSDistro.py	Fri Feb 01 17:30:06 2008 -0500
@@ -26,7 +26,6 @@ import tempfile
 import tempfile
 
 from virtinst import _virtinst as _
-from ImageFetcher import ImageFetcher
 
 # An image store is a base class for retrieving either a bootable
 # ISO image, or a kernel+initrd  pair for a particular OS distribution
@@ -80,10 +79,10 @@ class RedHatDistro(Distro):
 # Fedora distro check
 class FedoraDistro(RedHatDistro):
     def isValidStore(self, fetcher, progresscb):
-        if fetcher.hasFile("fedora.css", progresscb):
+        if fetcher.hasFile("fedora.css"):
             logging.debug("Detected a Fedora distro")
             return True
-        if fetcher.hasFile("Fedora", progresscb):
+        if fetcher.hasFile("Fedora"):
             logging.debug("Detected a Fedora distro")
             return True
         return False
@@ -91,13 +90,13 @@ class FedoraDistro(RedHatDistro):
 # Fedora distro check
 class RHELDistro(RedHatDistro):
     def isValidStore(self, fetcher, progresscb):
-        if fetcher.hasFile("Server", progresscb):
+        if fetcher.hasFile("Server"):
             logging.debug("Detected a RHEL 5 Server distro")
             return True
-        if fetcher.hasFile("Client", progresscb):
+        if fetcher.hasFile("Client"):
             logging.debug("Detected a RHEL 5 Client distro")
             return True
-        if fetcher.hasFile("RedHat", progresscb):
+        if fetcher.hasFile("RedHat"):
             logging.debug("Detected a RHEL 4 distro")
             return True
         return False
@@ -105,7 +104,7 @@ class RHELDistro(RedHatDistro):
 # CentOS distro check
 class CentOSDistro(RedHatDistro):
     def isValidStore(self, fetcher, progresscb):
-        if fetcher.hasFile("CentOS", progresscb):
+        if fetcher.hasFile("CentOS"):
             logging.debug("Detected a CentOS distro")
             return True
         return False
@@ -186,9 +185,9 @@ class SuseDistro(Distro):
                                 kernelrpm = dir + "/" + filename
 
             if kernelrpm is None:
-                raise _("Unable to determine kernel RPM path")
+                raise Exception(_("Unable to determine kernel RPM path"))
             if installinitrdrpm is None:
-                raise _("Unable to determine install-initrd RPM path")
+                raise Exception(_("Unable to determine install-initrd RPM path"))
             return (kernelrpm, installinitrdrpm)
         finally:
             filelistData.close()
@@ -309,18 +308,9 @@ class SuseDistro(Distro):
     def isValidStore(self, fetcher, progresscb):
         # Suse distros always have a 'directory.yast' file in the top
         # level of install tree, which we use as the magic check
-        ignore = None
-        try:
-            try:
-                ignore = fetcher.acquireFile("directory.yast", progresscb)
-                logging.debug("Detected a Suse distro")
-                return True
-            except ValueError, e:
-                logging.debug("Doesn't look like a Suse distro " + str(e))
-                pass
-        finally:
-            if ignore is not None:
-                os.unlink(ignore)
+        if fetcher.hasFile("directory.yast"):
+            logging.debug("Detected a Suse distro.")
+            return True
         return False
 
 
@@ -333,7 +323,14 @@ class DebianDistro(Distro):
         file = None
         try:
             try:
-                file = fetcher.acquireFile("current/images/MANIFEST", progresscb)
+                file = None
+                if fetcher.hasFile("current/images/MANIFEST"):
+                    file = fetcher.acquireFile("current/images/MANIFEST", 
+                                               progresscb)
+                else:
+                    logging.debug("Doesn't look like a Debian distro.")
+                    return False
+                    
             except ValueError, e:
                 logging.debug("Doesn't look like a Debian distro " + str(e))
                 return False




More information about the et-mgmt-tools mailing list