qemu: Make IOThread changing more robust

There are three APIs that allow changing IOThreads:

  virDomainAddIOThread()
  virDomainDelIOThread()
  virDomainSetIOThreadParams()

In case of QEMU driver these are handled by
qemuDomainChgIOThread() which attempts to be versatile enough to
work on both inactive and live domain definitions at the same
time. However, it's a bit clumsy - when a change to live
definition succeeds but fails in inactive definition then there's
no rollback. And somewhat rightfully so - changes to live
definition are in general harder to roll back. Therefore, do what
we do elsewhere (qemuDomainAttachDeviceLiveAndConfig(),
qemuDomainDetachDeviceAliasLiveAndConfig(), ...):

  1) do the change to inactive XML first,
  2) in fact, do the change to a copy of inactive XML,
  3) swap inactive XML and its copy only after everything
     succeeded.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c4418df1edf33d18779eafa5fc68af3b99855134..e30b1b8d84319aa0644f89c516e622c07a9ca542 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5595,6 +5595,7 @@ qemuDomainChgIOThread(virQEMUDriver *driver,
 {
     g_autoptr(virQEMUDriverConfig) cfg = NULL;
     qemuDomainObjPrivate *priv;
+    g_autoptr(virDomainDef) defcopy = NULL;
     virDomainDef *def;
     virDomainDef *persistentDef;
     virDomainIOThreadIDDef *iothreaddef = NULL;
@@ -5610,34 +5611,34 @@ qemuDomainChgIOThread(virQEMUDriver *driver,
     if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0)
         goto endjob;
 
-    if (def) {
-        if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                           _("IOThreads not supported with this binary"));
-            goto endjob;
-        }
+    if (persistentDef) {
+        /* Make a copy of persistent definition and do all the changes there.
+         * Swap the definitions only after changes to live definition
+         * succeeded. */
+        if (!(defcopy = virDomainObjCopyPersistentDef(vm, driver->xmlopt,
+                                                      priv->qemuCaps)))
+            return -1;
 
         switch (action) {
         case VIR_DOMAIN_IOTHREAD_ACTION_ADD:
-            if (virDomainDriverAddIOThreadCheck(def, iothread.iothread_id) < 0)
+            if (virDomainDriverAddIOThreadCheck(defcopy, iothread.iothread_id) < 0)
                 goto endjob;
 
-            if (qemuDomainHotplugAddIOThread(driver, vm, iothread.iothread_id) < 0)
+            if (!virDomainIOThreadIDAdd(defcopy, iothread.iothread_id))
                 goto endjob;
 
             break;
 
         case VIR_DOMAIN_IOTHREAD_ACTION_DEL:
-            if (virDomainDriverDelIOThreadCheck(def, iothread.iothread_id) < 0)
+            if (virDomainDriverDelIOThreadCheck(defcopy, iothread.iothread_id) < 0)
                 goto endjob;
 
-            if (qemuDomainHotplugDelIOThread(driver, vm, iothread.iothread_id) < 0)
-                goto endjob;
+            virDomainIOThreadIDDel(defcopy, iothread.iothread_id);
 
             break;
 
         case VIR_DOMAIN_IOTHREAD_ACTION_MOD:
-            iothreaddef = virDomainIOThreadIDFind(def, iothread.iothread_id);
+            iothreaddef = virDomainIOThreadIDFind(defcopy, iothread.iothread_id);
 
             if (!iothreaddef) {
                 virReportError(VIR_ERR_INVALID_ARG,
@@ -5646,41 +5647,47 @@ qemuDomainChgIOThread(virQEMUDriver *driver,
                 goto endjob;
             }
 
-            if (qemuDomainIOThreadValidate(iothreaddef, iothread, true) < 0)
+            if (qemuDomainIOThreadValidate(iothreaddef, iothread, false) < 0)
                 goto endjob;
 
-            if (qemuDomainHotplugModIOThread(driver, vm, iothread) < 0)
+            if (qemuDomainHotplugModIOThreadIDDef(iothreaddef, iothread) < 0) {
+                virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                               _("configuring persistent polling values is not supported"));
                 goto endjob;
+            }
 
-            qemuDomainHotplugModIOThreadIDDef(iothreaddef, iothread);
             break;
-
         }
-
-        qemuDomainSaveStatus(vm);
     }
 
-    if (persistentDef) {
+    if (def) {
+        if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("IOThreads not supported with this binary"));
+            goto endjob;
+        }
+
         switch (action) {
         case VIR_DOMAIN_IOTHREAD_ACTION_ADD:
-            if (virDomainDriverAddIOThreadCheck(persistentDef, iothread.iothread_id) < 0)
+            if (virDomainDriverAddIOThreadCheck(def, iothread.iothread_id) < 0)
                 goto endjob;
 
-            if (!virDomainIOThreadIDAdd(persistentDef, iothread.iothread_id))
+            if (qemuDomainHotplugAddIOThread(driver, vm, iothread.iothread_id) < 0)
                 goto endjob;
 
             break;
 
         case VIR_DOMAIN_IOTHREAD_ACTION_DEL:
-            if (virDomainDriverDelIOThreadCheck(persistentDef, iothread.iothread_id) < 0)
+            if (virDomainDriverDelIOThreadCheck(def, iothread.iothread_id) < 0)
                 goto endjob;
 
-            virDomainIOThreadIDDel(persistentDef, iothread.iothread_id);
+            if (qemuDomainHotplugDelIOThread(driver, vm, iothread.iothread_id) < 0)
+                goto endjob;
 
             break;
 
         case VIR_DOMAIN_IOTHREAD_ACTION_MOD:
-            iothreaddef = virDomainIOThreadIDFind(persistentDef, iothread.iothread_id);
+            iothreaddef = virDomainIOThreadIDFind(def, iothread.iothread_id);
 
             if (!iothreaddef) {
                 virReportError(VIR_ERR_INVALID_ARG,
@@ -5689,21 +5696,26 @@ qemuDomainChgIOThread(virQEMUDriver *driver,
                 goto endjob;
             }
 
-            if (qemuDomainIOThreadValidate(iothreaddef, iothread, false) < 0)
+            if (qemuDomainIOThreadValidate(iothreaddef, iothread, true) < 0)
                 goto endjob;
 
-            if (qemuDomainHotplugModIOThreadIDDef(iothreaddef, iothread) < 0) {
-                virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
-                               _("configuring persistent polling values is not supported"));
+            if (qemuDomainHotplugModIOThread(driver, vm, iothread) < 0)
                 goto endjob;
-            }
 
+            qemuDomainHotplugModIOThreadIDDef(iothreaddef, iothread);
             break;
+
         }
 
-        if (virDomainDefSave(persistentDef, driver->xmlopt,
-                             cfg->configDir) < 0)
+        qemuDomainSaveStatus(vm);
+    }
+
+    /* Finally, if no error until here, we can save config. */
+    if (defcopy) {
+        if (virDomainDefSave(defcopy, driver->xmlopt, cfg->configDir) < 0)
             goto endjob;
+
+        virDomainObjAssignDef(vm, &defcopy, false, NULL);
     }
 
     ret = 0;