Add proper python logging, better kickstart URL validation, better create error handling.

src/virtManager/config.py

@@ -17,6 +17,8 @@
 # Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 #
 import gconf
+import os
+import logging
 
 import gtk.gdk
 import libvirt
@@ -31,10 +33,24 @@ class vmmConfig:
         self.conf = gconf.client_get_default()
         self.conf.add_dir (gconf_dir,
                            gconf.CLIENT_PRELOAD_NONE)
+        # set up logging
+        vm_dir = "%s/.virt-manager" % os.environ['HOME'] 
+        if not os.access(vm_dir,os.W_OK):
+            try:
+                os.mkdir(vm_dir)
+            except IOError, e:
+                raise RuntimeError, "Could not create %d directory: " % vm_dir, e
 
+        # XXX should get logging level from gconf
+        logging.basicConfig(level=logging.DEBUG,
+                            format="%(asctime)s %(levelname)-8s %(message)s",
+                            datefmt="%a, %d %b %Y %H:%M:%S",
+                            filename="%s/virt-manager.log" % vm_dir,
+                            filemode='w')
+        logging.debug("Initialized Python logger")
         self.glade_dir = glade_dir
         self.icon_dir = icon_dir
-        # We don;t create it straight away, since we don't want
+        # We don't create it straight away, since we don't want
         # to block the app pending user authorizaation to access
         # the keyring
         self.keyring = None

src/virtManager/create.py

@@ -26,6 +26,7 @@ import os, sys
 import subprocess
 import urlgrabber.grabber as grabber
 import tempfile
+import logging
 
 from rhpl.exception import installExceptionHandler
 from rhpl.translate import _, N_, textdomain, utf8
@@ -288,7 +289,7 @@ class vmmCreate(gobject.GObject):
             saddr = self.storage_partition_address
         else:
             saddr = self.storage_file_address
-        print "your vm properties: \n Name=" + self.vm_name + \
+        logging.debug("your vm properties: \n Name=" + self.vm_name + \
               "\n Virt method: " + `self.virt_method` + \
               "\n Install media type (fv): " + `self.install_fv_media_type` + \
               "\n Install media address: " + self.install_media_address + \
@@ -298,7 +299,7 @@ class vmmCreate(gobject.GObject):
               "\n Install storage file size: " + sfs + \
               "\n Install max kernel memory: " + `int(self.max_memory)` + \
               "\n Install startup kernel memory: " + `int(self.startup_memory)` + \
-              "\n Install vcpus: " + `int(self.vcpus)`
+              "\n Install vcpus: " + `int(self.vcpus)`)
         # end DEBUG STUFF
         
         # first things first, are we trying to create a fully virt guest?
@@ -315,8 +316,6 @@ class vmmCreate(gobject.GObject):
                 self.install_media_address = None
         else:
             guest = xeninst.ParaVirtGuest()
-            if self.install_kickstart_address != None and self.install_kickstart_address != "":
-                guest.extraargs = "ks=%s" % self.install_kickstart_address
             try:
                 guest.location = self.install_media_address
             except ValueError, e:
@@ -326,9 +325,16 @@ class vmmCreate(gobject.GObject):
             # we got this far, now let's see if the location is really valid
             valid = self._validate_pv_url(guest.location)
             if valid != None:
-                self._validation_error_box(_("Error locating PV media"), valid)
+                self._validation_error_box(_("Error locating PV install image"), valid)
                 return
-
+            # also check the kickstart URL if it exists
+            if self.install_kickstart_address != None and self.install_kickstart_address != "":
+                valid = self._validate_pv_ks_url(self.install_kickstart_address)
+                if valid != None:
+                    self._validation_error_box(_("Error locating PV kickstart file"),valid)
+                    return
+                guest.extraargs = "ks=%s" % self.install_kickstart_address
+                    
         # set the name
         try:
             guest.name = self.vm_name
@@ -385,12 +391,10 @@ class vmmCreate(gobject.GObject):
 
     def do_install(self, guest):
         try:
-            print "\n\nStarting install..."
+            guest.start_install(False)
-            r = guest.start_install(False)
         except RuntimeError, e:
             self.install_error = "ERROR: %s" % e
-            # XXX use log4j for this later
+            logging.exception(e)
-            print >> sys.stderr, "ERROR: %s" % e
             return
     
     def set_name(self, src, ignore=None):
@@ -586,3 +590,22 @@ class vmmCreate(gobject.GObject):
         initrd.close()
         return None
     
+    def _validate_pv_ks_url(self, url):
+        if url.startswith("http://") or \
+               url.startswith("ftp://"):
+            try:
+                ks = grabber.urlopen(url)
+            except IOError, e:
+                return "Invalid URL location given: %s" % e.args[1]
+        elif self.location.startswith("nfs:"):
+            nfsmntdir = tempfile.mkdtemp(prefix="xennfs.", dir="/var/lib/xen")
+            cmd = ["mount", "-o", "ro", self.location[4:], nfsmntdir]
+            ret = subprocess.call(cmd)
+            if ret != 0:
+                return "Unable to mount NFS location %s" % url
+            try:
+                ks = open(url, "r")
+            except IOError, e:
+                return "Invalid NFS location given: %s" % e.args[1]
+        ks.close()
+        return None