domain: Make device XML editing similar to guest XML editing

virtManager/addhardware.py

@@ -1391,17 +1391,17 @@ class vmmAddHardware(vmmGObjectUI):
 
     @staticmethod
     def change_config_helper(define_func, define_args, vm, err,
-                              devobj=None,
-                              hotplug_args=None):
+            devobj=None, hotplug_args=None):
         hotplug_args = hotplug_args or {}
 
         # Persistent config change
         try:
             if devobj:
-                define_func(devobj, False, **define_args)
+                # Device XML editing
+                define_func(devobj=devobj, do_hotplug=False, **define_args)
             else:
+                # Guest XML editing
                 define_func(**define_args)
-            vm.redefine_cached()
         except Exception, e:
             err.show_err((_("Error changing VM configuration: %s") %
                               str(e)))
@@ -1409,20 +1409,27 @@ class vmmAddHardware(vmmGObjectUI):
             vm.refresh_xml()
             return False
 
+        if not vm.is_active():
+            return True
+
         # Hotplug change
         hotplug_err = None
-        if vm.is_active():
+        did_hotplug = False
         try:
             if devobj:
-                    hotplug_args["device"] = define_func(
-                        devobj, True, **define_args)
-                if hotplug_args:
+                define_func(devobj=devobj, do_hotplug=True, **define_args)
+                did_hotplug = True
+            elif hotplug_args:
+                did_hotplug = True
                 vm.hotplug(**hotplug_args)
         except Exception, e:
+            did_hotplug = True
             logging.debug("Hotplug failed: %s", str(e))
             hotplug_err = ((str(e), "".join(traceback.format_exc())))
 
-        if (hotplug_err or (vm.is_active() and not hotplug_args)):
+        if did_hotplug and not hotplug_err:
+            return True
+
         if len(define_args) > 1:
             msg = _("Some changes may require a guest shutdown "
                     "to take effect.")
@@ -1473,7 +1480,8 @@ class vmmAddHardware(vmmGObjectUI):
             kwargs["model"] = self._selected_model
 
             self.change_config_helper(self.vm.define_controller,
-                    kwargs, self.vm, self.err, self._remove_usb_controller)
+                    kwargs, self.vm, self.err,
+                    devobj=self._remove_usb_controller)
 
             self._remove_usb_controller = None
             self._selected_model = None

virtManager/details.py

@@ -2126,12 +2126,9 @@ class vmmDetails(vmmGObjectUI):
     # <device> defining
     def change_storage_media(self, devobj, newpath):
         kwargs = {"path": newpath}
-        hotplug_args = {"storage_path": True}
-
         return vmmAddHardware.change_config_helper(self.vm.define_disk,
                                           kwargs, self.vm, self.err,
-                                          devobj=devobj,
-                                          hotplug_args=hotplug_args)
+                                          devobj=devobj)
 
     def config_disk_apply(self, devobj):
         kwargs = {}

virtManager/domain.py

@@ -536,11 +536,11 @@ class vmmDomain(vmmLibvirtObject):
         self._status_reason = None
         self._has_managed_save = None
 
-    def _lookup_device_to_define(self, origdev, guest=None):
-        if guest is None:
-            guest = self._get_xmlobj_to_define()
+    def _lookup_device_to_define(self, xmlobj, origdev, for_hotplug):
+        if for_hotplug:
+            return origdev
 
-        dev = _find_device(guest, origdev)
+        dev = _find_device(xmlobj, origdev)
         if dev:
             return dev
 
@@ -554,15 +554,6 @@ class vmmDomain(vmmLibvirtObject):
         raise RuntimeError(_("Could not find specified device in the "
                              "inactive VM configuration: %s") % repr(origdev))
 
-    def _redefine_device(self, cb, origdev, use_live_device):
-        if not use_live_device:
-            dev = self._lookup_device_to_define(origdev)
-        else:
-            dev = origdev
-        if dev:
-            cb(dev)
-        return dev
-
 
     ##############################
     # Persistent XML change APIs #
@@ -588,17 +579,17 @@ class vmmDomain(vmmLibvirtObject):
             con = getattr(devobj, "virtmanager_console_dup")
 
         xmlobj = self._get_xmlobj_to_define()
-        def rmdev(editdev):
+        editdev = self._lookup_device_to_define(xmlobj, devobj, False)
+        if not editdev:
+            return
+
         if con:
             rmcon = _find_device(xmlobj, con)
             if rmcon:
                 xmlobj.remove_device(rmcon)
-
         xmlobj.remove_device(editdev)
 
-        ret = self._redefine_device(rmdev, devobj, False)
         self.redefine_cached()
-        return ret
 
     def define_cpu(self, vcpus=_SENTINEL, maxvcpus=_SENTINEL,
         cpuset=_SENTINEL, model=_SENTINEL, sockets=_SENTINEL,
@@ -694,7 +685,7 @@ class vmmDomain(vmmLibvirtObject):
 
             count = 1
             for origdev in boot_dev_order:
-                dev = self._lookup_device_to_define(origdev, guest=guest)
+                dev = self._lookup_device_to_define(guest, origdev, False)
                 if not dev:
                     continue
                 dev.boot.order = count
@@ -723,14 +714,24 @@ class vmmDomain(vmmLibvirtObject):
 
         self.redefine_cached()
 
-    def define_disk(self, devobj, use_live_device,
+
+    ######################
+    # Device XML editing #
+    ######################
+
+    def define_disk(self, devobj, do_hotplug,
         path=_SENTINEL, readonly=_SENTINEL, serial=_SENTINEL,
         shareable=_SENTINEL, removable=_SENTINEL, cache=_SENTINEL,
         io=_SENTINEL, driver_type=_SENTINEL, bus=_SENTINEL, addrstr=_SENTINEL,
         iotune_rbs=_SENTINEL, iotune_ris=_SENTINEL, iotune_tbs=_SENTINEL,
         iotune_tis=_SENTINEL, iotune_wbs=_SENTINEL, iotune_wis=_SENTINEL,
         sgio=_SENTINEL):
-        def _change_bus(editdev):
+        xmlobj = self._get_xmlobj_to_define()
+        editdev = self._lookup_device_to_define(xmlobj, devobj, do_hotplug)
+        if not editdev:
+            return
+
+        def _change_bus():
             oldprefix = editdev.get_target_prefix()[0]
             oldbus = editdev.bus
             editdev.bus = bus
@@ -756,10 +757,9 @@ class vmmDomain(vmmLibvirtObject):
             editdev.target = None
             editdev.generate_target(used)
 
-        def change(editdev):
         if path != _SENTINEL:
             editdev.path = path
-                if not use_live_device:
+            if not do_hotplug:
                 editdev.sync_path_props()
 
         if readonly != _SENTINEL:
@@ -794,18 +794,27 @@ class vmmDomain(vmmLibvirtObject):
             editdev.sgio = sgio or None
 
         if bus != _SENTINEL:
-                _change_bus(editdev)
+            _change_bus()
 
-        return self._redefine_device(change, devobj, use_live_device)
+        if do_hotplug:
+            hotplug_kwargs = {"device": editdev}
+            if path != _SENTINEL:
+                hotplug_kwargs["storage_path"] = True
+            self.hotplug(**hotplug_kwargs)
+        else:
+            self.redefine_cached()
 
-    def define_network(self, devobj, use_live_device,
+    def define_network(self, devobj, do_hotplug,
         ntype=_SENTINEL, source=_SENTINEL,
         mode=_SENTINEL, model=_SENTINEL, addrstr=_SENTINEL,
         vtype=_SENTINEL, managerid=_SENTINEL, typeid=_SENTINEL,
         typeidversion=_SENTINEL, instanceid=_SENTINEL,
         portgroup=_SENTINEL, macaddr=_SENTINEL):
+        xmlobj = self._get_xmlobj_to_define()
+        editdev = self._lookup_device_to_define(xmlobj, devobj, do_hotplug)
+        if not editdev:
+            return
 
-        def change(editdev):
         if ntype != _SENTINEL:
             editdev.source = None
 
@@ -829,12 +838,20 @@ class vmmDomain(vmmLibvirtObject):
 
         if macaddr != _SENTINEL:
             editdev.macaddr = macaddr
-        return self._redefine_device(change, devobj, use_live_device)
 
-    def define_graphics(self, devobj, use_live_device,
+        if do_hotplug:
+            self.hotplug(device=editdev)
+        else:
+            self.redefine_cached()
+
+    def define_graphics(self, devobj, do_hotplug,
         listen=_SENTINEL, port=_SENTINEL, tlsport=_SENTINEL,
         passwd=_SENTINEL, keymap=_SENTINEL, gtype=_SENTINEL):
-        def change(editdev):
+        xmlobj = self._get_xmlobj_to_define()
+        editdev = self._lookup_device_to_define(xmlobj, devobj, do_hotplug)
+        if not editdev:
+            return
+
         if listen != _SENTINEL:
             editdev.listen = listen
         if port != _SENTINEL:
@@ -847,21 +864,35 @@ class vmmDomain(vmmLibvirtObject):
             editdev.keymap = keymap
         if gtype != _SENTINEL:
             editdev.type = gtype
-        return self._redefine_device(change, devobj, use_live_device)
 
-    def define_sound(self, devobj, use_live_device, model=_SENTINEL):
-        def change(editdev):
+        if do_hotplug:
+            self.hotplug(device=editdev)
+        else:
+            self.redefine_cached()
+
+    def define_sound(self, devobj, do_hotplug, model=_SENTINEL):
+        xmlobj = self._get_xmlobj_to_define()
+        editdev = self._lookup_device_to_define(xmlobj, devobj, do_hotplug)
+        if not editdev:
+            return
+
         if model != _SENTINEL:
             if editdev.model != model:
                 editdev.address.clear()
             editdev.model = model
-        return self._redefine_device(change, devobj, use_live_device)
 
-    def define_video(self, devobj, use_live_device, model=_SENTINEL):
-        def change(editdev):
-            if model == _SENTINEL or model == editdev.model:
+        if do_hotplug:
+            self.hotplug(device=editdev)
+        else:
+            self.redefine_cached()
+
+    def define_video(self, devobj, do_hotplug, model=_SENTINEL):
+        xmlobj = self._get_xmlobj_to_define()
+        editdev = self._lookup_device_to_define(xmlobj, devobj, do_hotplug)
+        if not editdev:
             return
 
+        if model != _SENTINEL and model != editdev.model:
             editdev.model = model
             editdev.address.clear()
 
@@ -873,11 +904,18 @@ class vmmDomain(vmmLibvirtObject):
             editdev.ram = None
             editdev.vgamem = None
 
-        return self._redefine_device(change, devobj, use_live_device)
+        if do_hotplug:
+            self.hotplug(device=editdev)
+        else:
+            self.redefine_cached()
 
-    def define_watchdog(self, devobj, use_live_device,
+    def define_watchdog(self, devobj, do_hotplug,
         model=_SENTINEL, action=_SENTINEL):
-        def change(editdev):
+        xmlobj = self._get_xmlobj_to_define()
+        editdev = self._lookup_device_to_define(xmlobj, devobj, do_hotplug)
+        if not editdev:
+            return
+
         if model != _SENTINEL:
             if editdev.model != model:
                 editdev.address.clear()
@@ -885,17 +923,34 @@ class vmmDomain(vmmLibvirtObject):
 
         if action != _SENTINEL:
             editdev.action = action
-        return self._redefine_device(change, devobj, use_live_device)
 
-    def define_smartcard(self, devobj, use_live_device, model=_SENTINEL):
-        def change(editdev):
+        if do_hotplug:
+            self.hotplug(device=editdev)
+        else:
+            self.redefine_cached()
+
+    def define_smartcard(self, devobj, do_hotplug, model=_SENTINEL):
+        xmlobj = self._get_xmlobj_to_define()
+        editdev = self._lookup_device_to_define(xmlobj, devobj, do_hotplug)
+        if not editdev:
+            return
+
         if model != _SENTINEL:
             editdev.mode = model
             editdev.type = editdev.TYPE_DEFAULT
-        return self._redefine_device(change, devobj, use_live_device)
 
-    def define_controller(self, devobj, use_live_device, model=_SENTINEL):
-        def _change_model(editdev):
+        if do_hotplug:
+            self.hotplug(device=editdev)
+        else:
+            self.redefine_cached()
+
+    def define_controller(self, devobj, do_hotplug, model=_SENTINEL):
+        xmlobj = self._get_xmlobj_to_define()
+        editdev = self._lookup_device_to_define(xmlobj, devobj, do_hotplug)
+        if not editdev:
+            return
+
+        def _change_model():
             if editdev.type == "usb":
                 guest = self._get_xmlobj_to_define()
                 ctrls = guest.get_devices("controller")
@@ -922,17 +977,21 @@ class vmmDomain(vmmLibvirtObject):
                     editdev.model = model
                 self.hotplug(device=editdev)
 
-        def change(editdev):
         if model != _SENTINEL:
-                _change_model(editdev)
+            _change_model()
 
-        return self._redefine_device(change, devobj, use_live_device)
+        if do_hotplug:
+            self.hotplug(device=editdev)
+        else:
+            self.redefine_cached()
 
-    def define_filesystem(self, devobj, use_live_device, newdev=_SENTINEL):
-        def change(editdev):
-            if newdev == _SENTINEL:
+    def define_filesystem(self, devobj, do_hotplug, newdev=_SENTINEL):
+        xmlobj = self._get_xmlobj_to_define()
+        editdev = self._lookup_device_to_define(xmlobj, devobj, do_hotplug)
+        if not editdev:
             return
 
+        if newdev != _SENTINEL:
             # pylint: disable=maybe-no-member
             editdev.type = newdev.type
             editdev.mode = newdev.mode
@@ -944,14 +1003,25 @@ class vmmDomain(vmmLibvirtObject):
             editdev.source = newdev.source
             editdev.target = newdev.target
 
-        return self._redefine_device(change, devobj, use_live_device)
+        if do_hotplug:
+            self.redefine_cached()
+        else:
+            self.hotplug(device=editdev)
 
 
-    def define_hostdev(self, devobj, use_live_device, rom_bar=_SENTINEL):
-        def change(editdev):
+    def define_hostdev(self, devobj, do_hotplug, rom_bar=_SENTINEL):
+        xmlobj = self._get_xmlobj_to_define()
+        editdev = self._lookup_device_to_define(xmlobj, devobj, do_hotplug)
+        if not editdev:
+            return
+
         if rom_bar != _SENTINEL:
             editdev.rom_bar = rom_bar
-        return self._redefine_device(change, devobj, use_live_device)
+
+        if do_hotplug:
+            self.hotplug(device=editdev)
+        else:
+            self.redefine_cached()
 
 
     ####################
@@ -966,6 +1036,7 @@ class vmmDomain(vmmLibvirtObject):
             return
 
         devxml = devobj.get_xml_config()
+        logging.debug("attach_device with xml=\n%s", devxml)
         self._backend.attachDevice(devxml)
 
     def detach_device(self, devobj):
@@ -975,15 +1046,16 @@ class vmmDomain(vmmLibvirtObject):
         if not self.is_active():
             return
 
-        xml = devobj.get_xml_config()
-        self._backend.detachDevice(xml)
+        devxml = devobj.get_xml_config()
+        logging.debug("detach_device with xml=\n%s", devxml)
+        self._backend.detachDevice(devxml)
 
     def _update_device(self, devobj, flags=None):
         if flags is None:
             flags = getattr(libvirt, "VIR_DOMAIN_DEVICE_MODIFY_LIVE", 1)
 
         xml = devobj.get_xml_config()
-        logging.debug("Calling update_device with xml=\n%s", xml)
+        logging.debug("update_device with xml=\n%s", xml)
         self._backend.updateDeviceFlags(xml, flags)
 
     def hotplug(self, vcpus=_SENTINEL, memory=_SENTINEL, maxmem=_SENTINEL,