Cole Robinson wrote:
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>
This seems to work fine now, with a couple of corrections (below). I do
have one question about hanging onto the guest name, though, for which
see below...
------------------------------------------------------------------------
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
@@ -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()
Needed to change "guest.os_type" to "self._guest.os_type" here and
below, works fine with that change.
- 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
NIT: Seems silly to copy the entire self._guest object here merely so
you can hang onto the name. Why not just "name = self._guest.get_name()"
followed later by "self._guest.name = name"?
+
+ # 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
Otherwise looks pretty good. If you agree to the changes above I will
apply it.
Take care,
--Hugh
--
Red Hat Virtualization Group http://redhat.com/virtualization
Hugh Brock | virt-manager http://virt-manager.org
hbrock@xxxxxxxxxx | virtualization library http://libvirt.org
_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/et-mgmt-tools
