Hey,
Here's a fixed version of this which passes the test suite.
Cheers,
Mark.
Subject: Add Installer and re-factor existing code into DistroInstaller
From: Mark McLoughlin <markmc@xxxxxxxxxx>
This patch re-factors things so as to introduce the concept of
an "installer type". The idea is that we can create a VM using
not only a typical distribution installer, but also e.g. a livecd
installer or just a pre-built system image.
To that end an Installer class is added which is orthogonal
to the existing Guest class - i.e. the choice of installer is
independant of the choice of fully virt vs. paravirt.
In more detail the patch does the following:
+ Adds the Installer base class - sub-classes are expected to
implement the prepare() and get_os_blob() methods
+ Moves the Guest type, scratchdir, boot, extraargs, location,
and cdrom properties to the installer, but chains the original
properties to the installer in order to maintain compatibility
+ Rather than having Guest sub-classes implement get_runtime_xml()
and get_install_xml(), they now implement get_osblob() and chain
up to Installer.get_osblob() passing the installer parameters
like hvm or arch
+ Likewise, Guest sub-class implement a prepare_install() method
which chains up to Installer.prepare() passing it parameters
like need_bootdev and guest
+ All the existing prepare() and get_osblob() logic from FullyVirtGuest
and ParaVirtGuest is consolidated in a single DistroInstaller class
+ In FullyVirtGuest we append the features XML to the osblob
returned by the installer
+ If ParaVirtGuest or FullyVirtGuest is not passed an installer
instance, they create a DistroInstaller instance as a fallback
Signed-off-by: Mark McLoughlin <markmc@xxxxxxxxxx>
Index: virtinst--devel/virtinst/DistroManager.py
===================================================================
--- virtinst--devel.orig/virtinst/DistroManager.py
+++ virtinst--devel/virtinst/DistroManager.py
@@ -22,6 +22,7 @@ import subprocess
import urlgrabber.grabber as grabber
import urlgrabber.progress as progress
import tempfile
+import Guest
# This is a generic base class for fetching/extracting files from
@@ -585,3 +586,115 @@ def acquireBootDisk(baseuri, progresscb,
finally:
fetcher.cleanupLocation()
+class DistroInstaller(Guest.Installer):
+ def __init__(self, type = "xen", location = None, boot = None, extraargs = None):
+ Guest.Installer.__init__(self, type, location, boot, extraargs)
+
+ def get_location(self):
+ return self._location
+ def set_location(self, val):
+ if not (val.startswith("http://";) or val.startswith("ftp://";) or
+ val.startswith("nfs:") or val.startswith("/")):
+ raise ValueError("Install location must be an NFS, HTTP or FTP " +
+ "network install source, or local file/device")
+ if os.geteuid() != 0 and val.startswith("nfs:"):
+ raise ValueError("NFS installations are only supported as root")
+ self._location = val
+ location = property(get_location, set_location)
+
+ def _prepare_cdrom(self, guest, distro, meter):
+ if self.location.startswith("/"):
+ # Huzzah, a local file/device
+ cdrom = self.location
+ else:
+ # Xen needs a boot.iso if its a http://, ftp://, or nfs:/ url
+ cdrom = DistroManager.acquireBootDisk(self.location,
+ meter,
+ scratchdir = self.scratchdir,
+ distro = distro)
+ self._tmpfiles.append(cdrom)
+
+ guest.disks.append(Guest.VirtualDisk(cdrom,
+ device=Guest.VirtualDisk.DEVICE_CDROM,
+ readOnly=True,
+ transient=True))
+
+ def _prepare_kernel_and_initrd(self, guest, distro, meter):
+ if self.boot is not None:
+ # Got a local kernel/initrd already
+ self.install["kernel"] = self.boot["kernel"]
+ self.install["initrd"] = self.boot["initrd"]
+ if not self.extraargs is None:
+ self.install["extraargs"] = self.extraargs
+ else:
+ # Need to fetch the kernel & initrd from a remote site, or
+ # out of a loopback mounted disk image/device
+ (kernelfn, initrdfn, args) = acquireKernel(self.location,
+ meter,
+ scratchdir = self.scratchdir,
+ type = self.type,
+ distro = distro)
+ self.install["kernel"] = kernelfn
+ self.install["initrd"] = initrdfn
+ if not self.extraargs is None:
+ self.install["extraargs"] = self.extraargs + " " + args
+ else:
+ self.install["extraargs"] = args
+
+ self._tmpfiles.append(kernelfn)
+ self._tmpfiles.append(initrdfn)
+
+ # If they're installing off a local file/device, we map it
+ # through to a virtual harddisk
+ if self.location is not None and self.location.startswith("/"):
+ guest.disks.append(Guest.VirtualDisk(self.location,
+ readOnly=True,
+ transient=True))
+
+ def prepare(self, guest, need_bootdev, meter, distro = None):
+ self.cleanup()
+
+ self.install = {
+ "kernel" : "",
+ "initrd" : "",
+ "extraargs" : "",
+ }
+
+ if need_bootdev:
+ self._prepare_cdrom(guest, distro, meter)
+ else:
+ self._prepare_kernel_and_initrd(guest, distro, meter)
+
+ def _get_osblob(self, install, hvm, arch = None, loader = None):
+ osblob = ""
+ if install or hvm:
+ osblob = "<os>\n"
+
+ if hvm:
+ type = "hvm"
+ else:
+ type = "linux"
+
+ if arch:
+ osblob += " <type arch='%s'>%s</type>\n" % (arch, type)
+ else:
+ osblob += " <type>%s</type>\n" % type
+
+ if self.install["kernel"]:
+ osblob += " <kernel>%s</kernel>\n" % self.install["kernel"]
+ osblob += " <initrd>%s</initrd>\n" % self.install["initrd"]
+ osblob += " <cmdline>%s</cmdline>\n" % self.install["extraargs"]
+ else:
+ if loader:
+ osblob += " <loader>%s</loader>\n" % loader
+
+ if install:
+ osblob += " <boot dev='cdrom'/>\n"
+ else:
+ osblob += " <boot dev='hd'/>\n"
+
+ osblob += " </os>"
+ else:
+ osblob += "<bootloader>/usr/bin/pygrub</bootloader>"
+
+ return osblob
Index: virtinst--devel/virtinst/FullVirtGuest.py
===================================================================
--- virtinst--devel.orig/virtinst/FullVirtGuest.py
+++ virtinst--devel/virtinst/FullVirtGuest.py
@@ -89,8 +89,10 @@ class FullVirtGuest(Guest.XenGuest):
get_os_variant_label = staticmethod(get_os_variant_label)
- def __init__(self, type=None, arch=None, connection=None, hypervisorURI=None, emulator=None):
- Guest.Guest.__init__(self, type=type, connection=connection, hypervisorURI=hypervisorURI)
+ def __init__(self, type=None, arch=None, connection=None, hypervisorURI=None, emulator=None, installer=None):
+ if not installer:
+ installer = DistroManager.DistroInstaller(type = type)
+ Guest.Guest.__init__(self, type, connection, hypervisorURI, installer)
self.disknode = "hd"
self.features = { "acpi": None, "pae": util.is_pae_capable(), "apic": None }
self.arch = arch
@@ -148,58 +150,20 @@ class FullVirtGuest(Guest.XenGuest):
os_distro = property(get_os_distro)
def _get_features_xml(self):
- ret = ""
- for (k, v) in self.features.items():
- if v:
+ ret = "<features>\n"
+ if self.features:
+ ret += " "
+ for (k, v) in self.features.items():
ret += "<%s/>" %(k,)
- return ret
-
- def _get_loader_xml(self):
- if self.loader is None:
- return ""
-
- return """ <loader>%(loader)s</loader>""" % { "loader": self.loader }
-
- def _get_os_xml(self, bootdev, install=True):
- if self.arch is None:
- arch = ""
- else:
- arch = " arch='" + self.arch + "'"
-
- if self.kernel is None or install == False:
- return """<os>
- <type%(arch)s>hvm</type>
-%(loader)s
- <boot dev='%(bootdev)s'/>
- </os>
- <features>
- %(features)s
- </features>""" % \
- { "bootdev": bootdev, \
- "arch": arch, \
- "loader": self._get_loader_xml(), \
- "features": self._get_features_xml() }
- else:
- return """<os>
- <type%(arch)s>hvm</type>
- <kernel>%(kernel)s</kernel>
- <initrd>%(initrd)s</initrd>
- <cmdline>%(extra)s</cmdline>
- </os>
- <features>
- %(features)s
- </features>""" % \
- { "kernel": self.kernel, \
- "initrd": self.initrd, \
- "extra": self.extraargs, \
- "arch": arch, \
- "features": self._get_features_xml() }
+ ret += "\n"
+ return ret + " </features>"
- def _get_install_xml(self):
- return self._get_os_xml("cdrom", True)
+ def _get_osblob(self, install):
+ osblob = self.installer._get_osblob(install, True, self.arch, self.loader)
+ if osblob is None:
+ return None
- def _get_runtime_xml(self):
- return self._get_os_xml("hd", False)
+ return "%s\n %s" % (osblob, self._get_features_xml())
def _get_device_xml(self, install = True):
if self.emulator is None:
@@ -217,42 +181,18 @@ class FullVirtGuest(Guest.XenGuest):
self.set_os_type_parameters(self.os_type, self.os_variant)
Guest.Guest.validate_parms(self)
- def _prepare_install_location(self, meter):
- cdrom = None
- tmpfiles = []
- self.kernel = None
- self.initrd = None
- if self.location.startswith("/"):
- # Huzzah, a local file/device
- cdrom = self.location
- else:
- # Hmm, qemu ought to be able to boot off a kernel/initrd but
- # for some reason it often fails, hence disabled here..
- if self.type == "qemuXXX":
- # QEMU can go straight off a kernel/initrd
- if self.boot is not None:
- # Got a local kernel/initrd already
- self.kernel = self.boot["kernel"]
- self.initrd = self.boot["initrd"]
- else:
- (kernelfn,initrdfn,args) = DistroManager.acquireKernel(self.location, meter, scratchdir=self.scratchdir, distro=self.os_distro)
- self.kernel = kernelfn
- self.initrd = initrdfn
- if self.extraargs is not None:
- self.extraargs = self.extraargs + " " + args
- else:
- self.extraargs = args
- tmpfiles.append(kernelfn)
- tmpfiles.append(initrdfn)
- else:
- # Xen needs a boot.iso if its a http://, ftp://, or nfs:/ url
- cdrom = DistroManager.acquireBootDisk(self.location, meter, scratchdir=self.scratchdir, distro=self.os_distro)
- tmpfiles.append(cdrom)
-
- if cdrom is not None:
- self.disks.append(Guest.VirtualDisk(cdrom, device=Guest.VirtualDisk.DEVICE_CDROM, readOnly=True, transient=True))
+ def _prepare_install(self, meter):
+ need_bootdev = True
- return tmpfiles
+ # Hmm, qemu ought to be able to boot off a kernel/initrd but
+ # for some reason it often fails, hence disabled here..
+ if self.type == "qemuXXX":
+ need_bootdev = False
+
+ self._installer.prepare(guest = self,
+ need_bootdev = need_bootdev,
+ meter = meter,
+ distro = self.os_distro)
def get_continue_inst(self):
if self.os_type is not None:
Index: virtinst--devel/virtinst/Guest.py
===================================================================
--- virtinst--devel.orig/virtinst/Guest.py
+++ virtinst--devel/virtinst/Guest.py
@@ -338,14 +338,80 @@ class SDLVirtualGraphics(XenGraphics):
class XenSDLGraphics(SDLVirtualGraphics):
pass
-class Guest(object):
- def __init__(self, type=None, connection=None, hypervisorURI=None):
+class Installer(object):
+ def __init__(self, type = "xen", location = None, boot = None, extraargs = None):
+ self._location = None
+ self._extraargs = None
+ self._boot = None
+
if type is None:
type = "xen"
- self._type = type
+ self.type = type
+
+ if not location is None:
+ self.location = location
+ if not boot is None:
+ self.boot = boot
+ if not extraargs is None:
+ self.extraargs = extraargs
+
+ self._tmpfiles = []
+
+ def cleanup(self):
+ for f in self._tmpfiles:
+ logging.debug("Removing " + f)
+ os.unlink(f)
+ self._tmpfiles = []
+
+ def get_type(self):
+ return self._type
+ def set_type(self, val):
+ self._type = val
+ type = property(get_type, set_type)
+
+ def get_scratchdir(self):
+ if self.type == "xen":
+ return "/var/lib/xen"
+ return "/var/tmp"
+ scratchdir = property(get_scratchdir)
+
+ def get_location(self):
+ return self._location
+ def set_location(self, val):
+ self._location = val
+ location = property(get_location, set_location)
+
+ # kernel + initrd pair to use for installing as opposed to using a location
+ def get_boot(self):
+ return self._boot
+ def set_boot(self, val):
+ if type(val) == tuple:
+ if len(val) != 2:
+ raise ValueError, "Must pass both a kernel and initrd"
+ (k, i) = val
+ self._boot = {"kernel": k, "initrd": i}
+ elif type(val) == dict:
+ if not val.has_key("kernel") or not val.has_key("initrd"):
+ raise ValueError, "Must pass both a kernel and initrd"
+ self._boot = val
+ elif type(val) == list:
+ if len(val) != 2:
+ raise ValueError, "Must pass both a kernel and initrd"
+ self._boot = {"kernel": val[0], "initrd": val[1]}
+ boot = property(get_boot, set_boot)
+
+ # extra arguments to pass to the guest installer
+ def get_extraargs(self):
+ return self._extraargs
+ def set_extraargs(self, val):
+ self._extraargs = val
+ extraargs = property(get_extraargs, set_extraargs)
+
+class Guest(object):
+ def __init__(self, type=None, connection=None, hypervisorURI=None, installer=None):
+ self._installer = installer
self.disks = []
self.nics = []
- self._location = None
self._name = None
self._uuid = None
self._memory = None
@@ -362,24 +428,19 @@ class Guest(object):
raise RuntimeError, "Unable to connect to hypervisor, aborting installation!"
self.disknode = None # this needs to be set in the subclass
- self._boot = None
- self._extraargs = ""
+
+ def get_installer(self):
+ return self._installer
+ installer = property(get_installer)
+
def get_type(self):
- return self._type
+ return self._installer.type
def set_type(self, val):
- self._type = type
+ self._installer.type = type
type = property(get_type, set_type)
- def get_scratchdir(self):
- if self.type == "xen":
- return "/var/lib/xen"
- return "/var/tmp"
- scratchdir = property(get_scratchdir)
-
-
-
# Domain name of the guest
def get_name(self):
return self._name
@@ -439,61 +500,6 @@ class Guest(object):
vcpus = property(get_vcpus, set_vcpus)
- # kernel + initrd pair to use for installing as opposed to using a location
- def get_boot(self):
- return self._boot
- def set_boot(self, val):
- if type(val) == tuple:
- if len(val) != 2:
- raise ValueError, "Must pass both a kernel and initrd"
- (k, i) = val
- self._boot = {"kernel": k, "initrd": i}
- elif type(val) == dict:
- if not val.has_key("kernel") or not val.has_key("initrd"):
- raise ValueError, "Must pass both a kernel and initrd"
- self._boot = val
- elif type(val) == list:
- if len(val) != 2:
- raise ValueError, "Must pass both a kernel and initrd"
- self._boot = {"kernel": val[0], "initrd": val[1]}
- boot = property(get_boot, set_boot)
-
- # extra arguments to pass to the guest installer
- def get_extra_args(self):
- return self._extraargs
- def set_extra_args(self, val):
- self._extraargs = val
- extraargs = property(get_extra_args, set_extra_args)
-
-
- # install location for the PV guest
- # this is a string pointing to an NFS, HTTP or FTP install source
- def get_install_location(self):
- return self._location
- def set_install_location(self, val):
- if not (val.startswith("http://";) or val.startswith("ftp://";) or
- val.startswith("nfs:") or val.startswith("/")):
- raise ValueError, "Install location must be an NFS, HTTP or FTP network install source, or local file/device"
- if os.geteuid() != 0 and val.startswith("nfs:"):
- raise ValueError, "NFS installations are only supported as root"
- self._location = val
- location = property(get_install_location, set_install_location)
-
-
- # Legacy, deprecated
- def get_cdrom(self):
- if self._location is not None and self._location.startswith("/"):
- return self._location
- return None
- def set_cdrom(self, val):
- val = os.path.abspath(val)
- if not os.path.exists(val):
- raise ValueError, "CD device must exist!"
- self.set_install_location(val)
- cdrom = property(get_cdrom, set_cdrom)
-
-
-
# graphics setup
def get_graphics(self):
return self._graphics
@@ -549,6 +555,38 @@ class Guest(object):
graphics = property(get_graphics, set_graphics)
+ # Legacy, deprecated properties
+ def get_scratchdir(self):
+ return self._installer.scratchdir
+ scratchdir = property(get_scratchdir)
+
+ def get_boot(self):
+ return self._installer.boot
+ def set_boot(self, val):
+ self._installer.boot = val
+ boot = property(get_boot, set_boot)
+
+ def get_location(self):
+ return self._installer.location
+ def set_location(self, val):
+ self._installer.location = val
+ location = property(get_location, set_location)
+
+ def get_extraargs(self):
+ return self._installer.extraargs
+ def set_extraargs(self, val):
+ self._installer.extraargs = val
+ extraargs = property(get_extraargs, set_extraargs)
+
+ def get_cdrom(self):
+ if self._installer.location is not None and self._installer.location.startswith("/"):
+ return self._installer.location
+ return None
+ def set_cdrom(self, val):
+ self._installer.location = os.path.abspath(val)
+ cdrom = property(get_cdrom, set_cdrom)
+
+
def _create_devices(self,progresscb):
"""Ensure that devices are setup"""
for disk in self.disks:
@@ -580,15 +618,17 @@ class Guest(object):
def get_config_xml(self, install = True, disk_boot = False):
if install:
- if disk_boot:
- osblob = self._get_runtime_xml()
- else:
- osblob = self._get_install_xml()
action = "destroy"
else:
- osblob = self._get_runtime_xml()
action = "restart"
+ if disk_boot:
+ install = False
+
+ osblob = self._get_osblob(install)
+ if not osblob:
+ return None
+
return """<domain type='%(type)s'>
<name>%(name)s</name>
<currentMemory>%(ramkb)s</currentMemory>
@@ -622,13 +662,11 @@ class Guest(object):
# BaseMeter does nothing, but saves a lot of null checking
meter = progress.BaseMeter()
- tmpfiles = self._prepare_install_location(meter)
+ self._prepare_install(meter)
try:
return self._do_install(consolecb, meter)
finally:
- for file in tmpfiles:
- logging.debug("Removing " + file)
- os.unlink(file)
+ self._installer.cleanup()
def _do_install(self, consolecb, meter):
try:
Index: virtinst--devel/virtinst/ParaVirtGuest.py
===================================================================
--- virtinst--devel.orig/virtinst/ParaVirtGuest.py
+++ virtinst--devel/virtinst/ParaVirtGuest.py
@@ -18,24 +18,14 @@ import Guest
import DistroManager
class ParaVirtGuest(Guest.XenGuest):
- def __init__(self, type=None, connection=None, hypervisorURI=None):
- Guest.Guest.__init__(self, type=type, connection=connection, hypervisorURI=hypervisorURI)
+ def __init__(self, type=None, connection=None, hypervisorURI=None, installer=None):
+ if not installer:
+ installer = DistroManager.DistroInstaller(type = type)
+ Guest.Guest.__init__(self, type, connection, hypervisorURI, installer)
self.disknode = "xvd"
- def _get_install_xml(self):
- return """<os>
- <type>linux</type>
- <kernel>%(kernel)s</kernel>
- <initrd>%(initrd)s</initrd>
- <cmdline>%(extra)s</cmdline>
- </os>""" % \
- { "kernel": self.kernel, \
- "initrd": self.initrd, \
- "extra": self.extraargs }
-
-
- def _get_runtime_xml(self):
- return """<bootloader>/usr/bin/pygrub</bootloader>"""
+ def _get_osblob(self, install):
+ return self.installer._get_osblob(install, hvm = False)
def _connectSerialConsole(self):
# *sigh* would be nice to have a python version of xmconsole
@@ -53,31 +43,8 @@ class ParaVirtGuest(Guest.XenGuest):
raise RuntimeError, "A location must be specified to install from"
Guest.Guest.validate_parms(self)
- def _prepare_install_location(self, meter):
- tmpfiles = []
- if self.boot is not None:
- # Got a local kernel/initrd already
- self.kernel = self.boot["kernel"]
- self.initrd = self.boot["initrd"]
- else:
- # Need to fetch the kernel & initrd from a remote site, or
- # out of a loopback mounted disk image/device
- (kernelfn,initrdfn,args) = DistroManager.acquireKernel(self.location, meter, scratchdir=self.scratchdir, type=self.type)
- self.kernel = kernelfn
- self.initrd = initrdfn
- if self.extraargs is not None:
- self.extraargs = self.extraargs + " " + args
- else:
- self.extraargs = args
- tmpfiles.append(kernelfn)
- tmpfiles.append(initrdfn)
-
- # If they're installing off a local file/device, we map it
- # through to a virtual harddisk
- if self.location is not None and self.location.startswith("/"):
- self.disks.append(Guest.VirtualDisk(self.location, readOnly=True, transient=True))
-
- return tmpfiles
+ def _prepare_install(self, meter):
+ self._installer.prepare(guest = self, need_bootdev = False, meter = meter)
def _get_disk_xml(self, install = True):
"""Get the disk config in the libvirt XML format"""
Index: virtinst--devel/virtinst/__init__.py
===================================================================
--- virtinst--devel.orig/virtinst/__init__.py
+++ virtinst--devel/virtinst/__init__.py
@@ -2,3 +2,4 @@ import util
from Guest import Guest, VirtualDisk, VirtualNetworkInterface, XenGuest, XenDisk, XenNetworkInterface
from FullVirtGuest import FullVirtGuest
from ParaVirtGuest import ParaVirtGuest
+from DistroManager import DistroInstaller
Index: virtinst--devel/tests/xmlconfig.py
===================================================================
--- virtinst--devel.orig/tests/xmlconfig.py
+++ virtinst--devel/tests/xmlconfig.py
@@ -11,7 +11,7 @@ class TestXMLConfig(unittest.TestCase):
expectXML = string.join(f.readlines(), "")
f.close()
- tmpfiles = xenguest._prepare_install_location(progress.BaseMeter())
+ xenguest._prepare_install(progress.BaseMeter())
try:
actualXML = xenguest.get_config_xml(install=install)
@@ -21,8 +21,7 @@ class TestXMLConfig(unittest.TestCase):
self.assertEqual(actualXML, expectXML)
finally:
- for file in tmpfiles:
- os.unlink(file)
+ xenguest._installer.cleanup()
def _get_basic_paravirt_guest(self):
g = virtinst.ParaVirtGuest(hypervisorURI="test:///default", type="xen")
