[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [et-mgmt-tools] [patch 1/8] Add Installer and re-factor existing code into DistroInstaller



Hey,
	Hugh points out that this patch was malformed. No idea what happened
there, but fixed version attached.

Cheers,
Mark.
Subject: Add Installer and re-factor existing code into DistroInstaller
From: Mark McLoughlin <markmc redhat com>

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 redhat com>

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

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]