Re: [PATCH] virt-manager validation and error reporting improvements

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Cole Robinson wrote:
Hi All,

Attached is a patch that adds validation and error reporting improvements to virt-manager's creation wizard. It removes redundant checking and places the burden solely on virtinst, by plugging the parameters into a virtinst Guest object as it goes to get the error reporting and validation for free. This also simplifies the final setup before beginning the install.


The patch didn't apply properly since my repo wasn't fully up to date. Here is the fixed patch.

Signed-off-by: Cole Robinson <crobinso@xxxxxxxxxx>

Thanks,
Cole

--
Cole Robinson
crobinso@xxxxxxxxxx
diff -r 4df620fc9133 src/virtManager/create.py
--- a/src/virtManager/create.py	Tue Jun 19 13:20:29 2007 -0400
+++ b/src/virtManager/create.py	Mon Jun 18 15:30:35 2007 -0400
@@ -102,6 +102,11 @@ class vmmCreate(gobject.GObject):
 
         self.set_initial_state()
 
+        # Guest to fill in with values along the way
+        self._guest = virtinst.Guest(type=self.get_domain_type())
+        self._disk = None
+        self._net = None
+
     def show(self):
         self.topwin.show()
         self.reset_state()
@@ -477,97 +482,28 @@ class vmmCreate(gobject.GObject):
         return 0
 
     def finish(self, ignore=None):
-        # first things first, are we trying to create a fully virt guest?
-        if self.get_config_method() == VM_FULLY_VIRT:
-            guest = virtinst.FullVirtGuest(type=self.get_domain_type(), \
-                                           hypervisorURI=self.connection.get_uri(), \
-                                           arch=self.get_domain_arch())
-            try:
-                guest.cdrom = self.get_config_install_source()
-            except ValueError, e:
-                self._validation_error_box(_("Invalid FV media address"),e.args[0])
-            try:
-                if self.get_config_os_type() is not None and self.get_config_os_type() != "generic":
-                    logging.debug("OS Type: %s" % self.get_config_os_type())
-                    guest.os_type = self.get_config_os_type()
-            except ValueError, e:
-                self._validation_error_box(_("Invalid FV OS Type"),e.args[0])
-            try:
-                if self.get_config_os_variant() is not None and self.get_config_os_type() != "generic":
-                    logging.debug("OS Variant: %s" % self.get_config_os_variant())
-                    guest.os_variant = self.get_config_os_variant()
-            except ValueError, e:
-                self._validation_error_box(_("Invalid FV OS Variant"),e.args[0])
-
-        else:
-            guest = virtinst.ParaVirtGuest(type=self.get_domain_type(), hypervisorURI=self.connection.get_uri())
-            try:
-                guest.location = self.get_config_install_source()
-            except ValueError, e:
-                self._validation_error_box(_("Invalid PV media address"), e.args[0])
-                return
-            ks = self.get_config_kickstart_source()
-            if ks != None and len(ks) != 0:
-                guest.extraargs = "ks=%s" % ks
-
-        # set the name
+        # Validation should have mostly set up out guest. We just need
+        # to take care of a few pieces we didn't touch
+
+        guest = self._guest
+        guest.hypervisorURI = self.connection.get_uri()
+
+        # UUID, append disk and nic
         try:
-            guest.name = self.get_config_name()
+            guest.uuid = virtinst.util.uuidToString(virtinst.util.randomUUID())
+        except ValueError, E:
+            self._validation_error_box(_("UUID Error"), str(e))
+
+        try:
+            guest.disks.append(self._disk)
         except ValueError, e:
-            self._validation_error_box(_("Invalid system name"), e.args[0])
-            return
-
-        # set the memory
+            self._validation_error_box(_("Error Setting up Disk"), str(e))
+        
         try:
-            guest.memory = int(self.get_config_initial_memory())
-        except ValueError:
-            self._validation_error_box(_("Invalid memory setting"), e.args[0])
-            return
-
-        try:
-            guest.maxmemory = int(self.get_config_maximum_memory())
-        except ValueError:
-            self._validation_error_box(_("Invalid memory setting"), e.args[0])
-            return
-
-        # set vcpus
-        guest.vcpus = int(self.get_config_virtual_cpus())
-
-        # disks
-        filesize = None
-        if self.get_config_disk_size() != None:
-            filesize = self.get_config_disk_size() / 1024.0
-        try:
-            d = virtinst.VirtualDisk(self.get_config_disk_image(), filesize, sparse = self.is_sparse_file())
-            if d.type == virtinst.VirtualDisk.TYPE_FILE and \
-                   self.get_config_method() == VM_PARA_VIRT \
-                   and virtinst.util.is_blktap_capable():
-                d.driver_name = virtinst.VirtualDisk.DRIVER_TAP
-            if d.type == virtinst.VirtualDisk.TYPE_FILE and not \
-               self.is_sparse_file():
-                self.non_sparse = True
-            else:
-                self.non_sparse = False
+            guest.nics.append(self._net)
         except ValueError, e:
-            self._validation_error_box(_("Invalid storage address"), e.args[0])
-            return
-        guest.disks.append(d)
-
-        # uuid
-        guest.uuid = virtinst.util.uuidToString(virtinst.util.randomUUID())
-
-        # network
-        net = self.get_config_network()
-        mac = self.get_config_macaddr()
-        if net[0] == "bridge":
-            guest.nics.append(virtinst.VirtualNetworkInterface(macaddr=mac, type=net[0], bridge=net[1]))
-        elif net[0] == "network":
-            guest.nics.append(virtinst.VirtualNetworkInterface(macaddr=mac, type=net[0], network=net[1]))
-        elif net[0] == "user":
-            guest.nics.append(virtinst.VirtualNetworkInterface(macaddr=mac, type=net[0]))
-        else:
-            raise ValueError, "Unsupported networking type " + net[0]
-
+            self._validation_error_box(_("Error Setting up Network"), str(e))
+            
         # set up the graphics to use SDL
         import keytable
         keymap = None
@@ -597,7 +533,7 @@ class vmmCreate(gobject.GObject):
                       "\n  Memory: " + str(guest.memory) + \
                       "\n  Max Memory: " + str(guest.maxmemory) + \
                       "\n  # VCPUs: " + str(guest.vcpus) + \
-                      "\n  Filesize: " + str(filesize) + \
+                      "\n  Filesize: " + str(self._disk.size) + \
                       "\n  Disk image: " + str(self.get_config_disk_image()) +\
                       "\n  Non-sparse file: " + str(self.non_sparse))
 
@@ -787,8 +723,18 @@ class vmmCreate(gobject.GObject):
         startup_mem_adjustment.upper = max_memory
 
     def validate(self, page_num):
+
+        # Setting the values in the Guest/Disk/Network virtinst objects
+        # provides a lot of error checking for free, we just have to catch
+        # the messages
+
         if page_num == PAGE_NAME:
             name = self.window.get_widget("create-vm-name").get_text()
+            try:
+                self._guest.name = name
+            except ValueError, e:
+                self._validation_error_box(_("Invalid System Name"), str(e))
+
             if len(name) > 50 or len(name) == 0:
                 self._validation_error_box(_("Invalid System Name"), \
                                            _("System name must be non-blank and less than 50 characters"))
@@ -797,24 +743,30 @@ class vmmCreate(gobject.GObject):
                 self._validation_error_box(_("Invalid System Name"), \
                                            _("System name may contain alphanumeric, '_' and '-' characters only"))
                 return False
-
-
+                                           
         elif page_num == PAGE_TYPE:
-            if self.get_config_method() == VM_FULLY_VIRT and self.connection.get_type().startswith("Xen") and not virtinst.util.is_hvm_capable():
-                self._validation_error_box(_("Hardware Support Required"), \
-                                           _("Your hardware does not appear to support full virtualization. Only paravirtualized guests will be available on this hardware."))
-                return False
+
+            # Set up appropriate guest object dependent on selected type
+            tmpguest = self._guest
+            if self.get_config_method() == VM_PARA_VIRT:
+                self._guest = virtinst.ParaVirtGuest(\
+                                        type=self.get_domain_type())
+            else:
+                self._guest = virtinst.FullVirtGuest(\
+                                        type=self.get_domain_type(), \
+                                        arch=self.get_domain_arch())
+            
+            self._guest.name = tmpguest.get_name() # Transfer name over
 
         elif page_num == PAGE_FVINST:
+
             if self.window.get_widget("media-iso-image").get_active():
+
                 src = self.get_config_install_source()
-                if src == None or len(src) == 0:
-                    self._validation_error_box(_("ISO Path Required"), \
-                                               _("You must specify an ISO location for the guest installation"))
-                    return False
-                elif not(os.path.exists(src)):
-                    self._validation_error_box(_("ISO Path Not Found"), \
-                                               _("You must specify a valid path to the ISO image for guest installation"))
+                try:
+                    self._guest.cdrom = src
+                except ValueError, e:
+                    self._validation_error_box(_("ISO Path Not Found"), str(e))
                     return False
             else:
                 cdlist = self.window.get_widget("cd-path")
@@ -822,33 +774,93 @@ class vmmCreate(gobject.GObject):
                     self._validation_error_box(_("Install media required"), \
                                                _("You must select the CDROM install media for guest installation"))
                     return False
+                src = self.get_config_install_source()
+                try:
+                    self._guest.cdrom = src
+                except ValueError, e:
+                    self._validation_error_box(_("CD-ROM Path Error"), str(e))
+                    return False
+            
+            try:
+                if self.get_config_os_type() is not None \
+                   and self.get_config_os_type() != "generic":
+                    logging.debug("OS Type: %s" % self.get_config_os_type())
+                    guest.os_type = self.get_config_os_type()
+            except ValueError, e:
+                self._validation_error_box(_("Invalid FV OS Type"), str(e))
+
+            try:
+                if self.get_config_os_variant() is not None \
+                   and self.get_config_os_type() != "generic":
+                    logging.debug("OS Variant: %s" % self.get_config_os_variant())
+                    guest.os_variant = self.get_config_os_variant()
+            except ValueError, e:
+                self._validation_error_box(_("Invalid FV OS Variant"), str(e))
+
         elif page_num == PAGE_PVINST:
+
             src = self.get_config_install_source()
-            if src == None or len(src) == 0:
-                self._validation_error_box(_("URL Required"), \
-                                           _("You must specify a URL for the install image for the guest install"))
+            try:
+                self._guest.location = src
+            except ValueError, e:
+                self._validation_error_box(_("Install URL Required"), str(e))
                 return False
 
+            ks = self.get_config_kickstart_source()
+            if ks is not None and len(ks) != 0:
+                if not (ks.startswith("http://";) or ks.startswith("ftp://";) \
+                        or ks.startswith("nfs:")):
+                    self._validation_error_box(_("Kickstart URL Error"), \
+                                               _("Kickstart location must be an NFS, HTTP or FTP source"))
+                    return False
+                else:
+                    self._guest.extraargs = "ks=%s" % (ks,)
+
         elif page_num == PAGE_DISK:
+            
             disk = self.get_config_disk_image()
             if disk == None or len(disk) == 0:
                 self._validation_error_box(_("Storage Address Required"), \
                                            _("You must specify a partition or a file for storage for the guest install"))
                 return False
-
-            if not self.window.get_widget("storage-partition").get_active():
-                if os.path.isdir(disk):
-                    self._validation_error_box(_("Storage Address Is Directory"), \
-                                               _("You chose 'Simple File' storage for your storage method, but chose a directory instead of a file. Please enter a new filename or choose an existing file."))
-                    return False
-
-            d = virtinst.VirtualDisk(self.get_config_disk_image(), self.get_config_disk_size(), sparse = self.is_sparse_file())
-            if d.is_conflict_disk(self.connection.vmm) is True:
-               res = self._yes_no_box(_('Disk "%s" is already in use by another guest!' % disk), \
-                                               _("Do you really want to use the disk ?"))
+            
+            # Attempt to set disk
+            filesize = None
+            if self.get_config_disk_size() != None:
+                filesize = self.get_config_disk_size() / 1024.0
+            try:
+                if self.window.get_widget("storage-partition").get_active():
+                    type = virtinst.VirtualDisk.TYPE_BLOCK
+                else:
+                    type = virtinst.VirtualDisk.TYPE_FILE
+
+                self._disk = virtinst.VirtualDisk(\
+                                        self.get_config_disk_image(), \
+                                        filesize, \
+                                        sparse = self.is_sparse_file(), \
+                                        type=type)
+
+                if self._disk.type == virtinst.VirtualDisk.TYPE_FILE and \
+                   self.get_config_method() == VM_PARA_VIRT and \
+                   virtinst.util.is_blktap_capable():
+                    self._disk.driver_name = virtinst.VirtualDisk.DRIVER_TAP
+
+                if self._disk.type == virtinst.VirtualDisk.TYPE_FILE and not \
+                   self.is_sparse_file():
+                    self.non_sparse = True
+                else:
+                    self.non_sparse = False
+            except ValueError, e:
+                self._validation_error_box(_("Invalid Storage Address"), \
+                                            str(e))
+                return False
+
+            if self._disk.is_conflict_disk(self.connection.vmm) is True:
+               res = self._yes_no_box(_('Disk "%s" is already in use by another guest!' % disk), _("Do you really want to use the disk ?"))
                return res
 
         elif page_num == PAGE_NETWORK:
+
             if self.window.get_widget("net-type-network").get_active():
                 if self.window.get_widget("net-network").get_active() == -1:
                     self._validation_error_box(_("Virtual Network Required"),
@@ -860,28 +872,72 @@ class vmmCreate(gobject.GObject):
                                                _("You must select one of the physical devices"))
                     return False
 
+            net = self.get_config_network()
+
             if self.window.get_widget("mac-address").get_active():
                 mac = self.window.get_widget("create-mac-address").get_text()
-                if len(mac) != 17:
+                if mac is None or len(mac) == 0:
                     self._validation_error_box(_("Invalid MAC address"), \
-                                               _("MAC adrress must be 17 characters"))
+                                               _("No MAC Address was entered. Please enter a valid MAC address."))
                     return False
-                if re.match("^([0-9a-fA-F]{2}:){5}[0-9a-fA-F]{2}$",mac) == None:
-                    self._validation_error_box(_("Invalid MAC address"), \
-                                               _("MAC address must be a form such as AA:BB:CC:DD:EE:FF, and MAC adrress may contain numeric and alphabet of A-F(a-f) and ':' characters only"))
+                try:
+                    self._guest.mac = mac
+                except ValueError, e:
+                    self._validation_error_box(_("Invalid Mac Address"), \
+                                                str(e))
                     return False
+            
                 hostdevs = virtinst.util.get_host_network_devices()
                 for hostdev in hostdevs:
                     if mac.lower() == hostdev[4]:
-                        return self._yes_no_box(_('MAC adress "%s" is already in use by host!' % mac), \
-                                                _("Do you really want to use the MAC address ?"))
-                vms = []
-                for domains in self.connection.vms.values():
-                    vms.append(domains.vm)
-                vnic = virtinst.VirtualNetworkInterface(macaddr=mac)
-                if vnic.countMACaddr(vms) > 0:
-                    return self._yes_no_box(_('MAC adress "%s" is already in use by another guest!' % mac), \
-                                            _("Do you really want to use the MAC address ?"))
+                        return self._yes_no_box(_('MAC adress "%s" is already in use by host!' % mac), _("Do you really want to use the MAC address ?"))
+            else:
+                mac = None
+            try:    
+                if net[0] == "bridge":
+                    self._net = virtinst.VirtualNetworkInterface(macaddr=mac, \
+                                                                 type=net[0], \
+                                                                 bridge=net[1])
+                elif net[0] == "network":
+                    self._net = virtinst.VirtualNetworkInterface(macaddr=mac, \
+                                                                 type=net[0], \
+                                                                 network=net[1])
+                elif net[0] == "user":
+                    self._net = virtinst.VirtualNetworkInterface(macaddr=mac, \
+                                                                 type=net[0])
+            except ValueError, e:
+                self._validation_error_box(_("Network Parameter Error"), \
+                                            str(e))
+                return False
+
+            vms = []
+            for domains in self.connection.vms.values():
+                vms.append(domains.vm)
+            if self._net.countMACaddr(vms) > 0:
+                return self._yes_no_box(_('MAC adress "%s" is already in use by another guest!' % mac), _("Do you really want to use the MAC address ?"))
+
+        elif page_num == PAGE_CPUMEM:
+
+            # Set vcpus
+            try:
+                self._guest.vcpus = int(self.get_config_virtual_cpus())
+            except ValueError, e: 
+                self._validation_error_box(_("VCPU Count Error"), \
+                                            str(e))
+
+            # Set Memory
+            try:
+                self._guest.memory = int(self.get_config_initial_memory())
+            except ValueError, e: 
+                self._validation_error_box(_("Memory Amount Error"), \
+                                            str(e))
+
+            # Set Max Memory
+            try:
+                self._guest.maxmemory = int(self.get_config_maximum_memory())
+            except ValueError, e: 
+                self._validation_error_box(_("Max Memory Amount Error"), \
+                                            str(e))
 
         # do this always, since there's no "leaving a notebook page" event.
         self.window.get_widget("create-back").set_sensitive(True)
_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/et-mgmt-tools

[Index of Archives]     [Fedora Users]     [Fedora Legacy List]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux