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,18 @@ class FullVirtGuest(Guest.XenGuest): os_distro = property(get_os_distro) def _get_features_xml(self): - ret = "" + ret = "<features>\n" for (k, v) in self.features.items(): if v: - 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 += " <%s/>\n" %(k,) + 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 +179,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 --