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.
Signed-off-by: Cole Robinson <crobinso@xxxxxxxxxx>
Thanks,
Cole
--
Cole Robinson
crobinso@xxxxxxxxxx
diff -r 27ad8c7fbc3e src/virtManager/create.py
--- a/src/virtManager/create.py Thu May 24 16:49:13 2007 -0400
+++ b/src/virtManager/create.py Sun Jun 17 15:47:53 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()
@@ -476,97 +481,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
@@ -596,7 +532,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))
@@ -786,34 +722,42 @@ 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()
- 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"))
+ try:
+ self._guest.name = name
+ except ValueError, e:
+ self._validation_error_box(_("Invalid System Name"), str(e))
return False
- if re.match("^[a-zA-Z0-9_-]*$", name) == None:
- 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")
@@ -821,33 +765,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"),
@@ -859,28 +863,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)
