osxml: Convert to new style XML props

tests/xmlconfig.py

@@ -88,11 +88,10 @@ class TestXMLConfig(unittest.TestCase):
         meter = None
         removeOld = None
         wait = True
-        dom = None
 
         try:
-            dom = guest.start_install(consolecb, meter, removeOld, wait)
+            guest.start_install(consolecb, meter, removeOld, wait)
-            dom.destroy()
+            guest.domain.destroy()
 
             # Replace kernel/initrd with known info
             if guest.installer._install_kernel:
@@ -114,15 +113,14 @@ class TestXMLConfig(unittest.TestCase):
                 guest.continue_install(consolecb, meter, wait)
 
         finally:
-            if dom:
+            try:
-                try:
+                guest.domain.destroy()
-                    dom.destroy()
+            except:
-                except:
+                pass
-                    pass
+            try:
-                try:
+                guest.domain.undefine()
-                    dom.undefine()
+            except:
-                except:
+                pass
-                    pass
 
 
     def testBootParavirtDiskFile(self):
@@ -322,6 +320,10 @@ class TestXMLConfig(unittest.TestCase):
 
         # Call get_xml_config sets first round of defaults w/o os_variant set
         g.get_xml_config(do_install)
+        g._prepare_install(None)
+        g.get_xml_config(do_install)
+        g._prepare_install(None)
+        g.get_xml_config(do_install)
 
         g.os_variant = "fedora11"
         self._compare(g, "install-f11", do_install)

tests/xmlparse-xml/change-guest-out.xml

@@ -66,4 +66,5 @@
   <memoryBacking>
     <hugepages/>
   </memoryBacking>
+  <bootloader>pygrub</bootloader>
 </domain>

tests/xmlparse.py

@@ -113,6 +113,8 @@ class XMLParseTest(unittest.TestCase):
                       "11111111-2222-3333-4444-555555555555")
         check("emulator", "/usr/lib/xen/bin/qemu-dm", "/usr/binnnn/fooemu")
         check("hugepage", False, True)
+        check("type", "kvm", "test")
+        check("bootloader", None, "pygrub")
 
         check = self._make_checker(guest.clock)
         check("offset", "utc", "localtime")
@@ -125,7 +127,6 @@ class XMLParseTest(unittest.TestCase):
         check("relabel", None, True)
 
         check = self._make_checker(guest.os)
-        check("type", "kvm", "test")
         check("os_type", "hvm", "xen")
         check("arch", "i686", None)
         check("machine", "foobar", "pc-0.11")

virtinst/Guest.py

@@ -417,19 +417,13 @@ class Guest(XMLBuilder):
                        doc=_("Whether we should overwrite an existing guest "
                              "with the same name."))
 
-    def get_type(self):
-        return self.os.type
-    def set_type(self, val):
-        self.os.type = val
-    type = property(get_type, set_type)
-
 
     ########################################
     # Device Add/Remove Public API methods #
     ########################################
 
     def _dev_build_list(self, devtype, devlist=None):
-        if not devlist:
+        if devlist is None:
             devlist = self._devices
 
         newlist = []
@@ -461,7 +455,12 @@ class Guest(XMLBuilder):
                     return [dev][:]
                 else:
                     return []
-            self._set_defaults(list_one_dev, self.features)
+            origdev = self._devices
+            try:
+                self._devices = [dev]
+                self._set_defaults(self.features)
+            except:
+                self._devices = origdev
 
     def _track_device(self, dev):
         self._devices.append(dev)
@@ -512,6 +511,13 @@ class Guest(XMLBuilder):
             if xpath:
                 self._remove_child_xpath(xpath)
 
+    bootloader = XMLProperty(xpath="./bootloader")
+    type = XMLProperty(xpath="./@type", default_cb=lambda s: "xen")
+
+    def _cleanup_xml(self, xml):
+        if not xml.endswith("\n"):
+            xml += "\n"
+        return xml
 
     ################################
     # Private xml building methods #
@@ -677,16 +683,34 @@ class Guest(XMLBuilder):
 
         return xml
 
+    def _get_default_init(self):
+        for fs in self.get_devices("filesystem"):
+            if fs.target == "/":
+                return "/sbin/init"
+        return "/bin/sh"
+
     def _get_osblob(self, install):
         """
         Return os, features, and clock xml (Implemented in subclass)
         """
         oscopy = self.os.copy()
         self.installer.alter_bootconfig(self, install, oscopy)
-        osxml = oscopy._get_osblob_helper(self, install, oscopy, self.os)
+
-        if not osxml:
+        if oscopy.is_container() and not oscopy.init:
-            return None
+            oscopy.init = self._get_default_init()
-        return osxml
+        if not oscopy.loader and oscopy.is_hvm() and self.type == "xen":
+            oscopy.loader = "/usr/lib/xen/boot/hvmloader"
+        if oscopy.os_type == "xen" and self.type == "xen":
+            # Use older libvirt 'linux' value for back compat
+            oscopy.os_type = "linux"
+        if oscopy.kernel or oscopy.init:
+            oscopy.bootorder = []
+
+        return oscopy.get_xml_config() or None
+
+    def _get_osblob_helper(self, guest, isinstall,
+                           bootconfig, endbootconfig):
+        return self.get_xml_config()
 
     def _get_vcpu_xml(self):
         curvcpus_supported = self.conn.check_conn_support(
@@ -736,7 +760,19 @@ class Guest(XMLBuilder):
     # Public API #
     ##############
 
-    def _get_xml_config(self, install=True, disk_boot=False):
+    def _get_xml_config(self, *args, **kwargs):
+        # We do a shallow copy of the device list here, and set the defaults.
+        # This way, default changes aren't persistent, and we don't need
+        # to worry about when to call set_defaults
+        origdevs = self._devices
+        try:
+            self._devices = [dev.copy() for dev in self._devices]
+            return self._do_get_xml_config(*args, **kwargs)
+        finally:
+            self._devices = origdevs
+
+
+    def _do_get_xml_config(self, install=True, disk_boot=False):
         """
         Return the full Guest xml configuration.
 
@@ -754,36 +790,22 @@ class Guest(XMLBuilder):
         """
         # pylint: disable=W0221
         # Argument number differs from overridden method
-
-        # We do a shallow copy of the device list here, and set the defaults.
-        # This way, default changes aren't persistent, and we don't need
-        # to worry about when to call set_defaults
-        origdevs = self.get_all_devices()
-        devs = []
-        for dev in origdevs:
-            devs.append(dev.copy())
         tmpfeat = self.features.copy()
+        self._set_defaults(tmpfeat)
 
-        def get_transient_devices(devtype):
+        action = install and "destroy" or "restart"
-            return self._dev_build_list(devtype, devs)
+        osblob_install = install and not disk_boot
 
-        # Set device defaults so we can validly generate XML
-        self._set_defaults(get_transient_devices,
-                           tmpfeat)
-
-        if install:
-            action = "destroy"
-        else:
-            action = "restart"
-
-        osblob_install = install
-        if disk_boot:
-            osblob_install = False
-
-        osblob = self._get_osblob(osblob_install)
         if osblob_install and not self.installer.has_install_phase():
             return None
 
+        if (not install and
+            self.os.is_xenpv() and
+            not self.os.kernel):
+            osblob = "  <bootloader>/usr/bin/pygrub</bootloader>"
+        else:
+            osblob = self._get_osblob(osblob_install)
+
         desc_xml = ""
         if self.description is not None:
             desc = str(self.description)
@@ -811,8 +833,7 @@ class Guest(XMLBuilder):
         # <cputune>
         xml = add(self.numatune.get_xml_config())
         # <sysinfo>
-        # XXX: <bootloader> goes here, not in installer XML
+        xml = add(osblob)
-        xml = add("  %s" % osblob)
         xml = add(self._get_features_xml(tmpfeat))
         xml = add(self._get_cpu_xml())
         xml = add(self._get_clock_xml())
@@ -820,7 +841,7 @@ class Guest(XMLBuilder):
         xml = add("  <on_reboot>%s</on_reboot>" % action)
         xml = add("  <on_crash>%s</on_crash>" % action)
         xml = add("  <devices>")
-        xml = add(self._get_device_xml(devs, install))
+        xml = add(self._get_device_xml(self.get_all_devices(), install))
         xml = add("  </devices>")
         xml = add(self._get_seclabel_xml())
         xml = add("</domain>\n")
@@ -1125,20 +1146,20 @@ class Guest(XMLBuilder):
         The install process will call a non-persistent version, so calling
         this manually isn't required.
         """
-        self._set_defaults(self.get_devices, self.features)
+        self._set_defaults(self.features)
 
-    def _set_hvm_defaults(self, devlist_func, features):
+    def _set_hvm_defaults(self, features):
         disktype = VirtualDevice.VIRTUAL_DEV_DISK
         nettype = VirtualDevice.VIRTUAL_DEV_NET
         disk_bus  = self._lookup_device_param(disktype, "bus")
         net_model = self._lookup_device_param(nettype, "model")
 
         # Only overwrite params if they weren't already specified
-        for net in devlist_func(nettype):
+        for net in self.get_devices(nettype):
             if net_model and not net.model:
                 net.model = net_model
 
-        for disk in devlist_func(disktype):
+        for disk in self.get_devices(disktype):
             if (disk_bus and not disk.bus and
                 disk.device == VirtualDisk.DEVICE_DISK):
                 disk.bus = disk_bus
@@ -1157,15 +1178,15 @@ class Guest(XMLBuilder):
             self.conn.caps.host.arch == "ppc64"):
             self.os.machine = "pseries"
 
-    def _set_pv_defaults(self, devlist_func):
+    def _set_pv_defaults(self):
         # Default file backed PV guests to tap driver
-        for d in devlist_func(VirtualDevice.VIRTUAL_DEV_DISK):
+        for d in self.get_devices(VirtualDevice.VIRTUAL_DEV_DISK):
             if (d.type == VirtualDisk.TYPE_FILE
                 and d.driver_name is None
                 and util.is_blktap_capable(self.conn)):
                 d.driver_name = VirtualDisk.DRIVER_TAP
 
-        for d in devlist_func(VirtualDevice.VIRTUAL_DEV_INPUT):
+        for d in self.get_devices(VirtualDevice.VIRTUAL_DEV_INPUT):
             if d.type == d.TYPE_DEFAULT:
                 d.type = d.TYPE_MOUSE
             if d.bus == d.BUS_DEFAULT:
@@ -1195,14 +1216,14 @@ class Guest(XMLBuilder):
         ctrl.master_startport = 4
         self.add_device(ctrl)
 
-    def _set_defaults(self, devlist_func, features):
+    def _set_defaults(self, features):
-        for dev in devlist_func("all"):
+        for dev in self.get_devices("all"):
             dev.set_defaults()
 
         if self.os.is_hvm():
-            self._set_hvm_defaults(devlist_func, features)
+            self._set_hvm_defaults(features)
         if self.os.is_xenpv():
-            self._set_pv_defaults(devlist_func)
+            self._set_pv_defaults()
 
         soundtype = VirtualDevice.VIRTUAL_DEV_AUDIO
         videotype = VirtualDevice.VIRTUAL_DEV_VIDEO
@@ -1213,7 +1234,7 @@ class Guest(XMLBuilder):
         # Set default input values
         input_type = self._lookup_device_param(inputtype, "type")
         input_bus = self._lookup_device_param(inputtype, "bus")
-        for inp in devlist_func(inputtype):
+        for inp in self.get_devices(inputtype):
             if (inp.type == inp.TYPE_DEFAULT and
                 inp.bus  == inp.BUS_DEFAULT):
                 inp.type = input_type
@@ -1221,13 +1242,13 @@ class Guest(XMLBuilder):
 
         # Generate disk targets, and set preferred disk bus
         used_targets = []
-        for disk in devlist_func(VirtualDevice.VIRTUAL_DEV_DISK):
+        for disk in self.get_devices(VirtualDevice.VIRTUAL_DEV_DISK):
             if not disk.bus:
                 if disk.device == disk.DEVICE_FLOPPY:
                     disk.bus = "fdc"
                 else:
                     if self.os.is_hvm():
-                        if (self.os.type == "kvm" and
+                        if (self.type == "kvm" and
                             self.os.machine == "pseries"):
                             disk.bus = "scsi"
                         else:
@@ -1241,14 +1262,14 @@ class Guest(XMLBuilder):
 
         # Set sound device model
         sound_model  = self._lookup_device_param(soundtype, "model")
-        for sound in devlist_func(soundtype):
+        for sound in self.get_devices(soundtype):
             if sound.model == sound.MODEL_DEFAULT:
                 sound.model = sound_model
 
         # Set video device model
         # QXL device (only if we use spice) - safe even if guest is VGA only
         def has_spice():
-            for gfx in devlist_func(gfxtype):
+            for gfx in self.get_devices(gfxtype):
                 if gfx.type == gfx.TYPE_SPICE:
                     return True
         if has_spice():
@@ -1256,13 +1277,13 @@ class Guest(XMLBuilder):
         else:
             video_model  = self._lookup_device_param(videotype, "model")
 
-        for video in devlist_func(videotype):
+        for video in self.get_devices(videotype):
             if video.model == video.MODEL_DEFAULT:
                 video.model = video_model
 
         # Spice agent channel (only if we use spice)
         def has_spice_agent():
-            for chn in devlist_func(channeltype):
+            for chn in self.get_devices(channeltype):
                 if chn.type == chn.TYPE_SPICEVMC:
                     return True
 

virtinst/osxml.py

@@ -17,7 +17,6 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
 # MA 02110-1301 USA.
 
-from virtinst import util
 from virtinst.xmlbuilder import XMLBuilder, XMLProperty
 
 
@@ -33,30 +32,6 @@ class OSXML(XMLBuilder):
     boot_devices = [BOOT_DEVICE_HARDDISK, BOOT_DEVICE_CDROM,
                     BOOT_DEVICE_FLOPPY, BOOT_DEVICE_NETWORK]
 
-    _dumpxml_xpath = "/domain/os"
-    def __init__(self, conn, parsexml=None, parsexmlnode=None):
-        XMLBuilder.__init__(self, conn, parsexml,
-                                                   parsexmlnode)
-
-        self._bootorder = []
-        self._enable_bootmenu = None
-        self._kernel = None
-        self._initrd = None
-        self._kernel_args = None
-        self._type = None
-        self._arch = None
-        self._machine = None
-        self._loader = None
-        self._init = None
-        self._os_type = None
-
-        if self._is_parse():
-            return
-
-        self._arch = self.conn.caps.host.arch
-        self._type = "xen"
-        self._os_type = "xen"
-
     def is_hvm(self):
         return self.os_type == "hvm"
     def is_xenpv(self):
@@ -64,161 +39,26 @@ class OSXML(XMLBuilder):
     def is_container(self):
         return self.os_type == "exe"
 
-    def _get_enable_bootmenu(self):
+    _dumpxml_xpath = "/domain/os"
-        return self._enable_bootmenu
+    _XML_ROOT_NAME = "os"
-    def _set_enable_bootmenu(self, val):
+    _XML_INDENT = 2
-        self._enable_bootmenu = val
+    _XML_XPATH_RELATIVE = True
-    enable_bootmenu = XMLProperty(_get_enable_bootmenu, _set_enable_bootmenu,
+    _XML_PROP_ORDER = ["arch", "os_type", "loader",
-                            xpath="./os/bootmenu/@enable", is_yesno=True)
+                       "kernel", "initrd", "kernel_args",
+                       "bootorder"]
 
-    def _get_bootorder(self):
+    type = property(lambda s: s.snarf)
-        return self._bootorder
-    def _set_bootorder(self, val):
-        self._bootorder = val
-    bootorder = XMLProperty(_get_bootorder, _set_bootorder,
-                              is_multi=True,
-                              xpath="./os/boot/@dev")
 
-    def _get_kernel(self):
+    enable_bootmenu = XMLProperty(xpath="./os/bootmenu/@enable", is_yesno=True)
-        return self._kernel
+    bootorder = XMLProperty(xpath="./os/boot/@dev", is_multi=True)
-    def _set_kernel(self, val):
-        self._kernel = val
-    kernel = XMLProperty(_get_kernel, _set_kernel,
-                           xpath="./os/kernel")
 
-    def _get_initrd(self):
+    kernel = XMLProperty(xpath="./os/kernel")
-        return self._initrd
+    initrd = XMLProperty(xpath="./os/initrd")
-    def _set_initrd(self, val):
+    kernel_args = XMLProperty(xpath="./os/cmdline")
-        self._initrd = val
-    initrd = XMLProperty(_get_initrd, _set_initrd,
-                           xpath="./os/initrd")
 
-    def _get_kernel_args(self):
+    init = XMLProperty(xpath="./os/init")
-        return self._kernel_args
+    loader = XMLProperty(xpath="./os/loader")
-    def _set_kernel_args(self, val):
+    arch = XMLProperty(xpath="./os/type/@arch",
-        self._kernel_args = val
+                       default_cb=lambda s: s.conn.caps.host.arch)
-    kernel_args = XMLProperty(_get_kernel_args, _set_kernel_args,
+    machine = XMLProperty(xpath="./os/type/@machine")
-                                xpath="./os/cmdline")
+    os_type = XMLProperty(xpath="./os/type", default_cb=lambda s: "xen")
-
-    def _get_default_init(self, guest):
-        if not self.is_container():
-            return
-
-        for fs in guest.get_devices("filesystem"):
-            if fs.target == "/":
-                return "/sbin/init"
-        return "/bin/sh"
-    def _get_init(self):
-        return self._init
-    def _set_init(self, val):
-        self._init = val
-    init = XMLProperty(_get_init, _set_init,
-                         xpath="./os/init")
-
-    def _get_loader(self):
-        return self._loader
-    def _set_loader(self, val):
-        self._loader = val
-    loader = XMLProperty(_get_loader, _set_loader,
-                           xpath="./os/loader")
-
-    def get_arch(self):
-        return self._arch
-    def set_arch(self, val):
-        self._arch = val
-    arch = XMLProperty(get_arch, set_arch,
-                         xpath="./os/type/@arch")
-
-    def _get_machine(self):
-        return self._machine
-    def _set_machine(self, val):
-        self._machine = val
-    machine = XMLProperty(_get_machine, _set_machine,
-                            xpath="./os/type/@machine")
-
-    def get_ostype(self):
-        return self._os_type
-    def set_ostype(self, val):
-        self._os_type = val
-    os_type = XMLProperty(get_ostype, set_ostype, xpath="./os/type")
-
-    def get_type(self):
-        return self._type
-    def set_type(self, val):
-        self._type = val
-    type = XMLProperty(get_type, set_type, xpath="./@type")
-
-
-    def _get_xml_config(self):
-        xml = ""
-
-        if self.kernel:
-            xml = util.xml_append(xml, "    <kernel>%s</kernel>" %
-                                   util.xml_escape(self.kernel))
-            if self.initrd:
-                xml = util.xml_append(xml, "    <initrd>%s</initrd>" %
-                                       util.xml_escape(self.initrd))
-            if self.kernel_args:
-                xml = util.xml_append(xml, "    <cmdline>%s</cmdline>" %
-                                       util.xml_escape(self.kernel_args))
-
-        else:
-            for dev in self.bootorder:
-                xml = util.xml_append(xml, "    <boot dev='%s'/>" % dev)
-
-            if self.enable_bootmenu in [True, False]:
-                val = self.enable_bootmenu and "yes" or "no"
-                xml = util.xml_append(xml,
-                                       "    <bootmenu enable='%s'/>" % val)
-
-        return xml
-
-    def _get_osblob_helper(self, guest, isinstall,
-                           bootconfig, endbootconfig):
-        arch = self.arch
-        machine = self.machine
-        hvtype = self.type
-        loader = self.loader
-        os_type = self.os_type
-        init = self.init or self._get_default_init(guest)
-
-        hvxen = (hvtype == "xen")
-
-        if not loader and self.is_hvm() and hvxen:
-            loader = "/usr/lib/xen/boot/hvmloader"
-
-        # Use older libvirt 'linux' value for back compat
-        if os_type == "xen" and hvxen:
-            os_type = "linux"
-
-        if (not isinstall and
-            self.is_xenpv() and
-            not endbootconfig.kernel):
-            # This really should be provided by capabilites xml
-            return "<bootloader>/usr/bin/pygrub</bootloader>"
-
-        osblob = "<os>"
-
-        typexml = "    <type"
-        if arch:
-            typexml += " arch='%s'" % arch
-        if machine:
-            typexml += " machine='%s'" % machine
-        typexml += ">%s</type>" % os_type
-
-        osblob = util.xml_append(osblob, typexml)
-
-        if init:
-            osblob = util.xml_append(osblob,
-                                      "    <init>%s</init>" %
-                                      util.xml_escape(init))
-        if loader:
-            osblob = util.xml_append(osblob,
-                                      "    <loader>%s</loader>" %
-                                      util.xml_escape(loader))
-
-        if not self.is_container():
-            osblob = util.xml_append(osblob, bootconfig.get_xml_config())
-        osblob = util.xml_append(osblob, "  </os>")
-
-        return osblob

virtinst/xmlbuilder.py

@@ -700,6 +700,8 @@ class XMLBuilder(object):
                 self._xml_fixup_relative_xpath = self._XML_XPATH_RELATIVE
                 xmlstub = self._make_xml_stub(fail=False)
                 ret = self._get_xml_config(*args, **kwargs)
+                if ret is None:
+                    return None
                 ret = self._add_parse_bits(ret)
 
                 for propname in self._XML_SUB_ELEMENTS:
@@ -822,8 +824,9 @@ class XMLBuilder(object):
             indent += 1
         xml = "\n".join([l[indent:] for l in xml.splitlines()])
 
-        # Parse the XML into our internal state
+        # Parse the XML into our internal state. Use the raw
-        self._parsexml(xml, None)
+        # _parsexml so we don't hit Guest parsing into its internal state
+        XMLBuilder._parsexml(self, xml, None)
 
         # Set up preferred XML ordering
         do_order = self._proporder[:]