Hi all,
This patch fixes some validation errors in virtinst uncovered by the
tests I just sent. I sent this a couple months ago but it must have just
fallen through the cracks, so I reworked it against the current tip.
This stuff is pretty simple and handy for useful error reporting.
Thanks,
Cole
--
Cole Robinson
crobinso@xxxxxxxxxx
diff -r 6dbccf449182 virtinst/CloneManager.py
--- a/virtinst/CloneManager.py Tue Sep 25 11:36:43 2007 -0400
+++ b/virtinst/CloneManager.py Fri Sep 28 11:01:49 2007 -0400
@@ -70,7 +70,7 @@ class CloneDesign(object):
def get_original_guest(self):
return self._original_guest
def set_original_guest(self, original_guest):
- if len(original_guest) == 0:
+ if type(original_guest) is not type("str") or len(original_guest)==0:
raise ValueError, _("Name or UUID of guest to clone is required")
try:
@@ -102,7 +102,7 @@ class CloneDesign(object):
self._clone_uuid = uuid
def get_clone_uuid(self):
return self._clone_uuid
- clone_uuid = property(get_clone_uuid)
+ clone_uuid = property(get_clone_uuid, set_clone_uuid)
def set_clone_devices(self, devices):
if len(devices) == 0:
@@ -117,14 +117,14 @@ class CloneDesign(object):
self._clone_devices.append(devices)
def get_clone_devices(self):
return self._clone_devices
- clone_devices = property(get_clone_devices)
+ clone_devices = property(get_clone_devices, set_clone_devices)
def set_clone_mac(self, mac):
Guest.VirtualNetworkInterface(mac)
self._clone_mac.append(mac)
def get_clone_mac(self):
return self._clone_mac
- clone_mac = property(get_clone_mac)
+ clone_mac = property(get_clone_mac, set_clone_mac)
def get_clone_bs(self):
return self._clone_bs
diff -r 6dbccf449182 virtinst/Guest.py
--- a/virtinst/Guest.py Tue Sep 25 11:36:43 2007 -0400
+++ b/virtinst/Guest.py Fri Sep 28 12:57:14 2007 -0400
@@ -19,15 +19,10 @@ import urlgrabber.progress as progress
import urlgrabber.progress as progress
import util
import libvirt
+import __builtin__
from virtinst import _virtinst as _
import logging
-
-
-#print "YO %s" % (virtinst.gettext_virtinst("YO"))
-
-#def _(msg):
-# gettext_virtinst(msg)
class VirtualDisk:
DRIVER_FILE = "file"
@@ -51,19 +46,18 @@ class VirtualDisk:
self.size = size
self.sparse = sparse
self.transient = transient
- if path != None:
- self.path = os.path.abspath(path)
- else:
- self.path = None
- type = VirtualDisk.TYPE_FILE # Arbitrary choice but avoids the null-path null-type case
-
+ self.path = path
+
+ if __builtin__.type(self.path) is not __builtin__.type("string"):
+ raise ValueError, _("The disk path must be a string.")
+ self.path = os.path.abspath(self.path)
+
if self.path != None and os.path.isdir(self.path):
raise ValueError, \
_("The disk path must be a file or a device, not a directory")
- if self.path != None and not self.path.startswith("/"):
- raise ValueError, \
- _("The disk path must be an absolute path location, beginning with '/'")
+ if not os.path.exists(os.path.dirname(path)):
+ raise ValueError, _("The specified directory must exist.")
if type is None:
if not os.path.exists(self.path):
@@ -83,9 +77,13 @@ class VirtualDisk:
if size is None and not os.path.exists(self.path):
raise ValueError, \
_("A size must be provided for non-existent disks")
- if size is not None and size <= 0:
+ if size is not None and \
+ (__builtin__.type(size) is not __builtin__.type(1) and \
+ __builtin__.type(size) is not __builtin__.type(1.0)):
+ raise ValueError, _("Disk size must be an int or a float.")
+ if size <= 0:
raise ValueError, \
- _("The size of the disk image must be greater than 0")
+ _("Disk size must be an integer greater than 0.")
elif self._type == VirtualDisk.TYPE_BLOCK:
if not os.path.exists(self.path):
raise ValueError, _("The specified block device does not exist.")
@@ -213,10 +211,15 @@ class VirtualNetworkInterface:
class VirtualNetworkInterface:
def __init__(self, macaddr = None, type="bridge", bridge = None, network=None):
+ if macaddr is not None and \
+ __builtin__.type(macaddr) is not __builtin__.type("string"):
+ raise ValueError, "MAC address must be a string."
+
if macaddr is not None:
form = re.match("^([0-9a-fA-F]{2}:){5}[0-9a-fA-F]{2}$",macaddr)
if form is None:
- raise ValueError(_("MAC address must be of the format AA:BB:CC:DD:EE:FF"))
+ raise ValueError, \
+ _("MAC address must be of the format AA:BB:CC:DD:EE:FF")
self.macaddr = macaddr
self.type = type
self.bridge = bridge
@@ -411,6 +414,8 @@ class Installer(object):
def get_cdrom(self):
return self._cdrom
def set_cdrom(self, enable):
+ if __builtin__.type(enable) is not __builtin__.type(True):
+ raise ValueError, _("Guest.cdrom must be a boolean type")
self._cdrom = enable
cdrom = property(get_cdrom, set_cdrom)
@@ -438,6 +443,8 @@ class Installer(object):
if len(val) != 2:
raise ValueError, _("Must pass both a kernel and initrd")
self._boot = {"kernel": val[0], "initrd": val[1]}
+ else:
+ raise ValueError, _("Kernel and initrd must be specified by a list, dict, or tuple.")
boot = property(get_boot, set_boot)
# extra arguments to pass to the guest installer
@@ -459,6 +466,7 @@ class Guest(object):
self._vcpus = None
self._graphics = { "enabled": False }
self._keymap = None
+ self._URI = hypervisorURI
self.domain = None
self.conn = connection
@@ -487,14 +495,12 @@ class Guest(object):
def get_name(self):
return self._name
def set_name(self, val):
- if len(val) > 50 or len(val) == 0:
- raise ValueError, _("System name must be greater than 0 and no more than 50 characters")
+ if type(val) is not type("string") or len(val) > 50 or len(val) == 0:
+ raise ValueError, _("System name must be a string greater than 0 and no more than 50 characters")
if re.match("^[0-9]+$", val):
raise ValueError, _("System name must not be only numeric characters")
if re.match("^[a-zA-Z0-9._-]+$", val) == None:
raise ValueError, _("System name can only contain alphanumeric, '_', '.', or '-' characters")
- if type(val) != type("string"):
- raise ValueError, _("System name must be a string")
self._name = val
name = property(get_name, set_name)
@@ -503,7 +509,7 @@ class Guest(object):
def get_memory(self):
return self._memory
def set_memory(self, val):
- if (type(val) is not type(1) or val < 0):
+ if (type(val) is not type(1) or val <= 0):
raise ValueError, _("Memory value must be an integer greater than 0")
self._memory = val
if self._maxmemory is None or self._maxmemory < val:
@@ -514,7 +520,7 @@ class Guest(object):
def get_maxmemory(self):
return self._maxmemory
def set_maxmemory(self, val):
- if (type(val) is not type(1) or val < 0):
+ if (type(val) is not type(1) or val <= 0):
raise ValueError, _("Max Memory value must be an integer greater than 0")
self._maxmemory = val
maxmemory = property(get_maxmemory, set_maxmemory)
@@ -525,6 +531,9 @@ class Guest(object):
return self._uuid
def set_uuid(self, val):
# need better validation
+ if type(val) is not type("string"):
+ raise ValueError, _("UUID must be a string.")
+
form = re.match("[a-fA-F0-9]{8}[-]([a-fA-F0-9]{4}[-]){3}[a-fA-F0-9]{12}$", val)
if form is None:
form = re.match("[a-fA-F0-9]{32}$", val)
@@ -533,7 +542,7 @@ class Guest(object):
else: # UUID had no dashes, so add them in
val=val[0:8] + "-" + val[8:12] + "-" + val[12:16] + \
- "-" + val[16:20] + "-" + val[20:32]
+ "-" + val[16:20] + "-" + val[20:32]
self._uuid = val
uuid = property(get_uuid, set_uuid)
@@ -629,9 +638,13 @@ class Guest(object):
extraargs = property(get_extraargs, set_extraargs)
def get_cdrom(self):
- return self.location
+ return self._installer.location
def set_cdrom(self, val):
- self.location = val
+ if val is None or type(val) is not type("string") or len(val) == 0:
+ raise ValueError, _("You must specify a valid ISO or CD-ROM location for the installation")
+ if not os.path.exists(val):
+ raise ValueError, _("The specified media path does not exist.")
+ self._installer.location = os.path.abspath(val)
self._installer.cdrom = True
cdrom = property(get_cdrom, set_cdrom)
diff -r 6dbccf449182 virtinst/__init__.py
--- a/virtinst/__init__.py Tue Sep 25 11:36:43 2007 -0400
+++ b/virtinst/__init__.py Fri Sep 28 11:27:51 2007 -0400
@@ -15,3 +15,4 @@ from DistroManager import DistroInstalle
from DistroManager import DistroInstaller, PXEInstaller
from LiveCDInstaller import LiveCDInstaller
from ImageManager import ImageInstaller
+from CloneManager import CloneDesign
_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/et-mgmt-tools
