domain: Use virtinst for all XML editting

Kind of a big ugly commit since there is lot's of fallout, but the end
result is unittested functionality (via virtinst) and 500 less lines :)

src/virtManager/choosecd.py

@@ -20,8 +20,6 @@
 import gtk.glade
 import gobject
 
-import virtinst
-
 import virtManager.uihelpers as uihelpers
 import virtManager.util as util
 from virtManager.mediadev import MEDIA_FLOPPY
@@ -32,16 +30,15 @@ class vmmChooseCD(gobject.GObject):
     __gsignals__ = {
         "cdrom-chosen": (gobject.SIGNAL_RUN_FIRST,
                          gobject.TYPE_NONE,
-                         # dev, source, target
+                         # dev, new path
-                         (gobject.TYPE_PYOBJECT, str, str)),
+                         (gobject.TYPE_PYOBJECT, str)),
     }
 
-    IS_FLOPPY = 1
-    IS_CDROM  = 2
-
     def __init__(self, config, dev_id_info, connection, media_type):
         self.__gobject_init__()
-        self.window = gtk.glade.XML(config.get_glade_dir() + "/vmm-choose-cd.glade", "vmm-choose-cd", domain="virt-manager")
+        self.window = gtk.glade.XML(
+                            config.get_glade_dir() + "/vmm-choose-cd.glade",
+                            "vmm-choose-cd", domain="virt-manager")
         self.topwin = self.window.get_widget("vmm-choose-cd")
         self.topwin.hide()
 
@@ -108,18 +105,14 @@ class vmmChooseCD(gobject.GObject):
                                     _("A media path must be specified."))
 
         try:
-            dev=virtinst.VirtualDisk.DEVICE_CDROM
+            self.dev_id_info.path = path
-            disk = virtinst.VirtualDisk(path=path,
-                                        device=dev,
-                                        readOnly=True,
-                                        conn=self.conn.vmm)
         except Exception, e:
             return self.err.val_err(_("Invalid Media Path"), str(e))
 
         uihelpers.check_path_search_for_qemu(self.topwin, self.config,
                                              self.conn, path)
 
-        self.emit("cdrom-chosen", self.dev_id_info, disk.path, disk.type)
+        self.emit("cdrom-chosen", self.dev_id_info, path)
         self.cancel()
 
     def media_toggled(self, ignore1=None, ignore2=None):

src/virtManager/details.py

@@ -1329,26 +1329,25 @@ class vmmDetails(gobject.GObject):
 
         if curpath:
             # Disconnect cdrom
-            self.change_storage_media(dev_id_info, None, _type=None)
+            self.change_storage_media(dev_id_info, None)
+            return
 
-        else:
+        def change_cdrom_wrapper(src, dev_id_info, newpath):
-            def change_cdrom_wrapper(src, dev_id_info, newpath,
+            return self.change_storage_media(dev_id_info, newpath)
-                                     _type=None):
-                return self.change_storage_media(dev_id_info, newpath, _type)
 
-            # Launch 'Choose CD' dialog
+        # Launch 'Choose CD' dialog
-            if self.media_choosers[devtype] is None:
+        if self.media_choosers[devtype] is None:
-                ret = vmmChooseCD(self.config,
+            ret = vmmChooseCD(self.config,
-                                  dev_id_info,
+                              dev_id_info,
-                                  self.vm.get_connection(),
+                              self.vm.get_connection(),
-                                  devtype)
+                              devtype)
 
-                ret.connect("cdrom-chosen", change_cdrom_wrapper)
+            ret.connect("cdrom-chosen", change_cdrom_wrapper)
-                self.media_choosers[devtype] = ret
+            self.media_choosers[devtype] = ret
 
-            dialog = self.media_choosers[devtype]
+        dialog = self.media_choosers[devtype]
-            dialog.dev_id_info = dev_id_info
+        dialog.dev_id_info = dev_id_info
-            dialog.show()
+        dialog.show()
 
     ##################################################
     # Details/Hardware config changes (apply button) #
@@ -1512,11 +1511,11 @@ class vmmDetails(gobject.GObject):
                                           (bootdevs,))
 
     # CDROM
-    def change_storage_media(self, dev_id_info, newpath, _type=None):
+    def change_storage_media(self, dev_id_info, newpath):
         return self._change_config_helper(self.vm.define_storage_media,
-                                          (dev_id_info, newpath, _type),
+                                          (dev_id_info, newpath),
                                           self.vm.hotplug_storage_media,
-                                          (dev_id_info, newpath, _type))
+                                          (dev_id_info, newpath))
 
     # Disk options
     def config_disk_apply(self, dev_id_info):
@@ -1579,7 +1578,7 @@ class vmmDetails(gobject.GObject):
 
         # Define the change
         try:
-            self.vm.remove_device(dev_type, dev_id_info)
+            self.vm.remove_device(dev_id_info)
         except Exception, e:
             self.err.show_err(_("Error Removing Device: %s" % str(e)),
                               "".join(traceback.format_exc()))
@@ -1589,7 +1588,7 @@ class vmmDetails(gobject.GObject):
         detach_err = False
         try:
             if self.vm.is_active():
-                self.vm.detach_device(dev_type, dev_id_info)
+                self.vm.detach_device(dev_id_info)
         except Exception, e:
             logging.debug("Device could not be hotUNplugged: %s" % str(e))
             detach_err = True
@@ -1639,15 +1638,20 @@ class vmmDetails(gobject.GObject):
                     hotplug_err = True
 
         # Persistent config change
-        for idx in range(len(define_funcs)):
+        try:
-            func = define_funcs[idx]
+            for idx in range(len(define_funcs)):
-            args = define_funcs_args[idx]
+                func = define_funcs[idx]
-            try:
+                args = define_funcs_args[idx]
                 func(*args)
-            except Exception, e:
+            if define_funcs:
-                self.err.show_err((_("Error changing VM configuration: %s") %
+                self.vm.redefine_cached()
-                                   str(e)), "".join(traceback.format_exc()))
+        except Exception, e:
-                return False
+            self.err.show_err((_("Error changing VM configuration: %s") %
+                              str(e)), "".join(traceback.format_exc()))
+            # If we fail, make sure we flush the cache
+            self.vm.refresh_xml()
+            return False
+
 
         if (hotplug_err or
             (active and not len(hotplug_funcs) == len(define_funcs))):
@@ -2045,7 +2049,7 @@ class vmmDetails(gobject.GObject):
             return
 
         char_type = chardev.virtual_device_type.capitalize()
-        target_port = chardev.index
+        target_port = chardev.target_port
         dev_type = chardev.char_type or "pty"
         src_path = chardev.source_path
         primary = hasattr(chardev, "virtmanager_console_dup")
@@ -2249,7 +2253,7 @@ class vmmDetails(gobject.GObject):
         # Populate list of char devices
         for chardev in self.vm.get_char_devices():
             devtype = chardev.virtual_device_type
-            port = chardev.index
+            port = chardev.target_port
 
             label = devtype.capitalize()
             if devtype != "console":

src/virtManager/libvirtobject.py

@@ -23,8 +23,19 @@ import gobject
 import difflib
 import logging
 
+import libxml2
+
 from virtManager import util
 
+def _sanitize_xml(xml):
+    xml = libxml2.parseDoc(xml).serialize()
+    # Strip starting <?...> line
+    if xml.startswith("<?"):
+        ignore, xml = xml.split("\n", 1)
+    if not xml.endswith("\n") and xml.count("\n"):
+        xml += "\n"
+    return xml
+
 class vmmLibvirtObject(gobject.GObject):
     __gsignals__ = {
         "config-changed": (gobject.SIGNAL_RUN_FIRST,
@@ -40,6 +51,9 @@ class vmmLibvirtObject(gobject.GObject):
         self._xml = None
         self._is_xml_valid = False
 
+        # Cached XML that accumulates changes to define
+        self._xml_to_define = None
+
         # These should be set by the child classes if necessary
         self._inactive_xml_flags = 0
         self._active_xml_flags = 0
@@ -88,15 +102,16 @@ class vmmLibvirtObject(gobject.GObject):
 
         return self._xml
 
-    def refresh_xml(self):
+    def refresh_xml(self, forcesignal=False):
         # Force an xml update. Signal 'config-changed' if domain xml has
         # changed since last refresh
 
         origxml = self._xml
+        self._invalidate_xml()
         self._xml = self._XMLDesc(self._active_xml_flags)
         self._is_xml_valid = True
 
-        if origxml != self._xml:
+        if origxml != self._xml or forcesignal:
             util.safe_idle_add(util.idle_emit, self, "config-changed")
 
     ######################################
@@ -111,24 +126,13 @@ class vmmLibvirtObject(gobject.GObject):
     # Internal API functions #
     ##########################
 
-    def _redefine(self, xml_func, *args):
+    def __xml_to_redefine(self):
-        """
+        return _sanitize_xml(self.get_xml(inactive=True))
-        Helper function for altering a redefining VM xml
 
-        @param xml_func: Function to alter the running XML. Takes the
+    def _redefine_helper(self, origxml, newxml):
-                         original XML as its first argument.
+        origxml = _sanitize_xml(origxml)
-        @param args: Extra arguments to pass to xml_func
+        newxml  = _sanitize_xml(newxml)
-        """
+        if origxml != newxml:
-        origxml = self.get_xml(inactive=True)
-        # Sanitize origxml to be similar to what we will get back
-        origxml = util.xml_parse_wrapper(origxml, lambda d, c: d.serialize())
-
-        newxml = xml_func(origxml, *args)
-
-        if origxml == newxml:
-            logging.debug("Redefinition request XML was no different,"
-                          " redefining anyways")
-        else:
             diff = "".join(difflib.unified_diff(origxml.splitlines(1),
                                                 newxml.splitlines(1),
                                                 fromfile="Original XML",
@@ -136,9 +140,14 @@ class vmmLibvirtObject(gobject.GObject):
             logging.debug("Redefining '%s' with XML diff:\n%s",
                           self.get_name(), diff)
 
-        self._define(newxml)
+            self._define(newxml)
 
-        # Invalidate cached XML
+        # Make sure we have latest XML
-        self.refresh_xml()
+        self.refresh_xml(forcesignal=True)
+        return
+
+    def _redefine_xml(self, newxml):
+        origxml = self.__xml_to_redefine()
+        return self._redefine_helper(origxml, newxml)
 
 gobject.type_register(vmmLibvirtObject)