Hi all,
Attached is a patch that cleans up and improves some error reporting in
virtinst. Includes error checking for setting memory and vcpu, improved
install location and disk location errors, and a couple other fixes.
Signed-off-by: Cole Robinson <crobinso@xxxxxxxxxx>
Thanks,
Cole
--
Cole Robinson
crobinso@xxxxxxxxxx
diff -r 3e2bf458732a virt-install
--- a/virt-install Thu Jun 14 08:53:12 2007 -0400
+++ b/virt-install Sat Jun 16 14:29:36 2007 -0400
@@ -124,7 +124,11 @@ def get_vcpus(vcpus, check_cpu, guest, c
except ValueError, e:
print "ERROR: ", e
if vcpus:
+ try:
guest.vcpus = vcpus
+ except ValueError, e:
+ print "ERROR: ", e
+
def get_disk(disk, size, sparse, guest, hvm, conn):
# FIXME: need to handle a list of disks at some point
diff -r 3e2bf458732a virtinst/DistroManager.py
--- a/virtinst/DistroManager.py Thu Jun 14 08:53:12 2007 -0400
+++ b/virtinst/DistroManager.py Sun Jun 17 16:17:14 2007 -0400
@@ -69,7 +69,9 @@ class URIImageFetcher(ImageFetcher):
text = "Verifying install location...")
return True
except IOError, e:
- logging.debug("Validation failed for " + self.location + " " + str(e))
+ msg = "Opening URL " + self.location + " failed."
+ logging.debug(msg + " " + str(e))
+ raise ValueError(msg)
return False
def acquireFile(self, filename, progresscb):
@@ -117,6 +119,9 @@ class MountedImageFetcher(ImageFetcher):
ret = subprocess.call(cmd)
if ret != 0:
self.cleanupLocation()
+ msg = "Mounting " + self.location + " failed."
+ logging.debug(msg)
+ raise ValueError(msg)
return False
return True
@@ -564,8 +569,11 @@ def _storeForDistro(fetcher, baseuri, ty
# Method to fetch a krenel & initrd pair for a particular distro / HV type
def acquireKernel(baseuri, progresscb, scratchdir="/var/tmp", type=None, distro=None):
fetcher = _fetcherForURI(baseuri, scratchdir)
- if not fetcher.prepareLocation(progresscb):
- raise RuntimeError, "Invalid install location"
+
+ try:
+ fetcher.prepareLocation(progresscb)
+ except ValueError, e:
+ raise RuntimeError, "Invalid install location: " + str(e)
try:
store = _storeForDistro(fetcher=fetcher, baseuri=baseuri, type=type, \
@@ -577,8 +585,11 @@ def acquireKernel(baseuri, progresscb, s
# Method to fetch a bootable ISO image for a particular distro / HV type
def acquireBootDisk(baseuri, progresscb, scratchdir="/var/tmp", type=None, distro=None):
fetcher = _fetcherForURI(baseuri, scratchdir)
- if not fetcher.prepareLocation(progresscb):
- raise RuntimeError, "Invalid install location"
+
+ try:
+ fetcher.prepareLocation(progresscb)
+ except ValueError, e:
+ raise RuntimeError, "Invalid install location: " + str(e)
try:
store = _storeForDistro(fetcher=fetcher, baseuri=baseuri, type=type, \
diff -r 3e2bf458732a virtinst/Guest.py
--- a/virtinst/Guest.py Thu Jun 14 08:53:12 2007 -0400
+++ b/virtinst/Guest.py Sun Jun 17 16:46:08 2007 -0400
@@ -24,8 +24,6 @@ import util
import logging
-import urlgrabber.progress as progress
-
class VirtualDisk:
DRIVER_FILE = "file"
DRIVER_PHY = "phy"
@@ -51,22 +49,39 @@ class VirtualDisk:
self.path = os.path.abspath(path)
if os.path.isdir(self.path):
- raise ValueError, "Must provide a file, not a directory for the disk"
+ raise ValueError, \
+ "The disk path must be a file/device, not a directory"
+
+ if not self.path.startswith("/"):
+ raise ValueError, \
+ "The disk path must be an absolute path location, beginning with '/'"
if type is None:
if not os.path.exists(self.path):
- if size is None:
- raise ValueError, "Must provide a size for non-existent disks"
- if size <= 0:
- raise ValueError, "Size of the disk image must be greater than 0"
+ logging.debug("Disk path not found: Assuming file disk type.");
self._type = VirtualDisk.TYPE_FILE
else:
if stat.S_ISBLK(os.stat(self.path)[stat.ST_MODE]):
+ logging.debug(\
+ "Path is block file: Assuming Block disk type.");
self._type = VirtualDisk.TYPE_BLOCK
else:
self._type = VirtualDisk.TYPE_FILE
else:
self._type = type
+
+ if self._type == VirtualDisk.TYPE_FILE:
+ 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:
+ raise ValueError, \
+ "The size of the disk image must be 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."
+ if not stat.S_ISBLK(os.stat(self.path)[stat.ST_MODE]):
+ raise ValueError, "The specified path is not a block device."
self._readOnly = readOnly
self._device = device
@@ -187,10 +202,11 @@ class XenDisk(VirtualDisk):
class VirtualNetworkInterface:
def __init__(self, macaddr = None, type="bridge", bridge = None, network=None):
+
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, "Invalid value for MAC address"
+ raise ValueError("MAC address must be of the format AA:BB:CC:DD:EE:FF")
self.macaddr = macaddr
self.type = type
self.bridge = bridge
@@ -451,16 +467,14 @@ class Guest(object):
def get_name(self):
return self._name
def set_name(self, val):
- if len(val) == 0:
- raise ValueError, "Domain name must be nonempty"
+ if len(val) > 50 or len(val) == 0:
+ raise ValueError, "System name must be greater than 0 and no more than 50 characters"
if re.match("^[0-9]+$", val):
- raise ValueError, "Domain name must not be numeric only"
+ raise ValueError, "System name must not be only numeric characters"
if re.match("^[a-zA-Z0-9_-]+$", val) == None:
- raise ValueError, "Domain name must be alphanumeric or _ or -"
- if len(val) > 50:
- raise ValueError, "Domain name must be less than or equal to 50 characters"
+ raise ValueError, "System name can only contain alphanumeric, '_' or '-' characters"
if type(val) != type("string"):
- raise ValueError, "Domain name must be a string"
+ raise ValueError, "System name must be a string"
self._name = val
name = property(get_name, set_name)
@@ -469,6 +483,8 @@ 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):
+ raise ValueError, "Memory value must be an integer greater than 0"
self._memory = val
if self._maxmemory is None or self._maxmemory < val:
self._maxmemory = val
@@ -478,6 +494,8 @@ 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):
+ raise ValueError, "Max Memory value must be an integer greater than 0"
self._maxmemory = val
maxmemory = property(get_maxmemory, set_maxmemory)
@@ -489,11 +507,15 @@ class Guest(object):
# need better validation
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)
+ form = re.match("[a-fA-F0-9]{32}$", val)
if form is None:
- raise ValueError, "UUID must be a 32-digit hexadecimal number. It may take the form XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX or may omit hyphens altogether."
- else:
- val=val[0:8] + "-" + val[8:12] + "-" + val[12:16] + "-" + val[16:20] + "-" + val[20:32]
+ raise ValueError, "UUID must be a 32-digit hexadecimal " + \
+ "number. It may take the form XXXXXXXX-XXXX-XXXX-" + \
+ "XXXX-XXXXXXXXXXXX or may omit hyphens altogether."
+
+ 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]
self._uuid = val
uuid = property(get_uuid, set_uuid)
@@ -502,6 +524,10 @@ class Guest(object):
def get_vcpus(self):
return self._vcpus
def set_vcpus(self, val):
+ maxvcpus = util.get_max_vcpus(self.conn)
+ if val < 1 or val > get_max_vcpus:
+ raise ValueError, \
+ "Number of vcpus must be in the range of 1-%d" % (maxvcpus,)
self._vcpus = val
vcpus = property(get_vcpus, set_vcpus)
@@ -585,10 +611,15 @@ class Guest(object):
extraargs = property(get_extraargs, set_extraargs)
def get_cdrom(self):
- if self._installer.location is not None and self._installer.location.startswith("/"):
+ if self._installer.location is not None and \
+ self._installer.location.startswith("/"):
return self._installer.location
return None
def set_cdrom(self, val):
+ if val is None or len(val) == 0:
+ raise ValueError, "You must specify an ISO location for the guest installation"
+ if not os.path.exists(val):
+ raise ValueError, "You must specify a valid path to the ISO image for guest installation"
self._installer.location = os.path.abspath(val)
cdrom = property(get_cdrom, set_cdrom)
diff -r 3e2bf458732a virtinst/util.py
--- a/virtinst/util.py Thu Jun 14 08:53:12 2007 -0400
+++ b/virtinst/util.py Sat Jun 16 16:12:34 2007 -0400
@@ -177,3 +177,12 @@ def get_host_network_devices():
if words[i] == "hwaddr":
device.append(words)
return device
+
+def get_max_vcpus(conn):
+ """@conn libvirt connection to poll for max possible vcpus"""
+ try:
+ max = conn.getMaxVcpus(conn.getType())
+ except:
+ print >> stderr, "Couldn't determine max vcpus. Using 32."
+ max = 32
+ return max
