qemu: Pass correct qemuCaps to virDomainDefCopy

Since qemuDomainDefPostParse callback requires qemuCaps, we need to make
sure it gets the capabilities stored in the domain's private data if the
domain is running. Passing NULL may cause QEMU capabilities probing to
be triggered in case QEMU binary changed in the meantime. When this
happens while a running domain object is locked, QMP event delivered to
the domain before QEMU capabilities probing finishes will deadlock the
event loop.

Several general functions from domain_conf.c were lazily passing NULL as
the parseOpaque pointer instead of letting their callers pass the right
data. This patch fixes all paths leading to virDomainDefCopy to do the
right thing.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0456369d55ac399ce819f642177bd86b6debd50b..c44ceba7d5581a5ffa94cee2dc651dcb42f05c9a 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3674,7 +3674,8 @@ virDomainObjWaitUntil(virDomainObjPtr vm,
 int
 virDomainObjSetDefTransient(virCapsPtr caps,
                             virDomainXMLOptionPtr xmlopt,
-                            virDomainObjPtr domain)
+                            virDomainObjPtr domain,
+                            void *parseOpaque)
 {
     int ret = -1;
 
@@ -3684,7 +3685,8 @@ virDomainObjSetDefTransient(virCapsPtr caps,
     if (domain->newDef)
         return 0;
 
-    if (!(domain->newDef = virDomainDefCopy(domain->def, caps, xmlopt, NULL, false)))
+    if (!(domain->newDef = virDomainDefCopy(domain->def, caps, xmlopt,
+                                            parseOpaque, false)))
         goto out;
 
     ret = 0;
@@ -3723,10 +3725,11 @@ virDomainObjRemoveTransientDef(virDomainObjPtr domain)
 virDomainDefPtr
 virDomainObjGetPersistentDef(virCapsPtr caps,
                              virDomainXMLOptionPtr xmlopt,
-                             virDomainObjPtr domain)
+                             virDomainObjPtr domain,
+                             void *parseOpaque)
 {
     if (virDomainObjIsActive(domain) &&
-        virDomainObjSetDefTransient(caps, xmlopt, domain) < 0)
+        virDomainObjSetDefTransient(caps, xmlopt, domain, parseOpaque) < 0)
         return NULL;
 
     if (domain->newDef)
@@ -29341,12 +29344,13 @@ virDomainDefCopy(virDomainDefPtr src,
 virDomainDefPtr
 virDomainObjCopyPersistentDef(virDomainObjPtr dom,
                               virCapsPtr caps,
-                              virDomainXMLOptionPtr xmlopt)
+                              virDomainXMLOptionPtr xmlopt,
+                              void *parseOpaque)
 {
     virDomainDefPtr cur;
 
-    cur = virDomainObjGetPersistentDef(caps, xmlopt, dom);
-    return virDomainDefCopy(cur, caps, xmlopt, NULL, false);
+    cur = virDomainObjGetPersistentDef(caps, xmlopt, dom, parseOpaque);
+    return virDomainDefCopy(cur, caps, xmlopt, parseOpaque, false);
 }
 
 

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 57ca2a8ad17bf71453e92cc94aff74b8ffcf880e..7b9732fbbc445b760d05fad89976cac18ce9c6f4 100644 (file)
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2902,12 +2902,14 @@ void virDomainObjAssignDef(virDomainObjPtr domain,
                            virDomainDefPtr *oldDef);
 int virDomainObjSetDefTransient(virCapsPtr caps,
                                 virDomainXMLOptionPtr xmlopt,
-                                virDomainObjPtr domain);
+                                virDomainObjPtr domain,
+                                void *parseOpaque);
 void virDomainObjRemoveTransientDef(virDomainObjPtr domain);
 virDomainDefPtr
 virDomainObjGetPersistentDef(virCapsPtr caps,
                              virDomainXMLOptionPtr xmlopt,
-                             virDomainObjPtr domain);
+                             virDomainObjPtr domain,
+                             void *parseOpaque);
 
 int virDomainObjUpdateModificationImpact(virDomainObjPtr vm,
                                          unsigned int *flags);
@@ -2928,7 +2930,8 @@ virDomainDefPtr virDomainDefCopy(virDomainDefPtr src,
                                  bool migratable);
 virDomainDefPtr virDomainObjCopyPersistentDef(virDomainObjPtr dom,
                                               virCapsPtr caps,
-                                              virDomainXMLOptionPtr xmlopt);
+                                              virDomainXMLOptionPtr xmlopt,
+                                              void *parseOpaque);
 
 typedef enum {
     /* parse internal domain status information */

diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 2d8569e5921949df65de3efa492bfdbfbcb7d6ed..b60511a26665b8269264fa453a574f0c39c41b83 100644 (file)
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1340,7 +1340,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
         VIR_FREE(managed_save_path);
     }
 
-    if (virDomainObjSetDefTransient(cfg->caps, driver->xmlopt, vm) < 0)
+    if (virDomainObjSetDefTransient(cfg->caps, driver->xmlopt, vm, NULL) < 0)
         goto cleanup;
 
     /* Run an early hook to set-up missing devices */

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index f430522b35aadbdcba5ca9897ef32d219cef0e2f..0e1bf2f0bbb29840b75b5c854b55b83ef38c49aa 100644 (file)
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1607,7 +1607,7 @@ virDomainLiveConfigHelperMethod(virCapsPtr caps,
         return -1;
 
     if (*flags & VIR_DOMAIN_AFFECT_CONFIG) {
-        if (!(*persistentDef = virDomainObjGetPersistentDef(caps, xmlopt, dom))) {
+        if (!(*persistentDef = virDomainObjGetPersistentDef(caps, xmlopt, dom, NULL))) {
             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                            _("Get persistent config failed"));
             return -1;
@@ -2294,7 +2294,7 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
         goto endjob;
     }
 
-    if (!(def = virDomainObjGetPersistentDef(cfg->caps, driver->xmlopt, vm)))
+    if (!(def = virDomainObjGetPersistentDef(cfg->caps, driver->xmlopt, vm, NULL)))
         goto endjob;
 
     maplen = VIR_CPU_MAPLEN(nvcpus);
@@ -4126,7 +4126,7 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml,
 
         /* Make a copy for updated domain. */
         if (!(vmdef = virDomainObjCopyPersistentDef(vm, cfg->caps,
-                                                    driver->xmlopt)))
+                                                    driver->xmlopt, NULL)))
             goto endjob;
 
         if (libxlDomainAttachDeviceConfig(vmdef, dev) < 0)
@@ -4216,7 +4216,7 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
 
         /* Make a copy for updated domain. */
         if (!(vmdef = virDomainObjCopyPersistentDef(vm, cfg->caps,
-                                                    driver->xmlopt)))
+                                                    driver->xmlopt, NULL)))
             goto endjob;
 
         if (libxlDomainDetachDeviceConfig(vmdef, dev) < 0)
@@ -4303,7 +4303,7 @@ libxlDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
 
         /* Make a copy for updated domain. */
         if (!(vmdef = virDomainObjCopyPersistentDef(vm, cfg->caps,
-                                                    driver->xmlopt)))
+                                                    driver->xmlopt, NULL)))
             goto cleanup;
 
         if ((ret = libxlDomainUpdateDeviceConfig(vmdef, dev)) < 0)

diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index 76bcb66a112c30d33f2fec8239adda229548e343..a1021d499b699b8e5bf808810fecad0f8c731933 100644 (file)
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -1343,7 +1343,7 @@ libxlDomainMigrationDstFinish(virConnectPtr dconn,
 
         vm->persistent = 1;
         if (!(vmdef = virDomainObjGetPersistentDef(cfg->caps,
-                                                   driver->xmlopt, vm)))
+                                                   driver->xmlopt, vm, NULL)))
             goto cleanup;
 
         if (virDomainSaveConfig(cfg->configDir, cfg->caps, vmdef) < 0)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index d0b670310163cfd0f93f6ce8425cae2f64f25c64..a2180b197e35fec4cb6b6e48b3422f2a2017de89 100644 (file)
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1913,7 +1913,7 @@ lxcDomainSetSchedulerParametersFlags(virDomainPtr dom,
 
     if (persistentDef) {
         /* Make a copy for updated domain. */
-        persistentDefCopy = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt);
+        persistentDefCopy = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt, NULL);
         if (!persistentDefCopy)
             goto endjob;
     }
@@ -4735,7 +4735,7 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom,
 
     if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
         /* Make a copy for updated domain. */
-        vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt);
+        vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt, NULL);
         if (!vmdef)
             goto endjob;
 
@@ -4840,7 +4840,7 @@ static int lxcDomainUpdateDeviceFlags(virDomainPtr dom,
         goto endjob;
 
     /* Make a copy for updated domain. */
-    if (!(vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt)))
+    if (!(vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt, NULL)))
         goto endjob;
 
     /* virDomainDefCompatibleDevice call is delayed until we know the
@@ -4919,7 +4919,7 @@ static int lxcDomainDetachDeviceFlags(virDomainPtr dom,
 
     if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
         /* Make a copy for updated domain. */
-        vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt);
+        vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt, NULL);
         if (!vmdef)
             goto endjob;
 

diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index a1d028b2e638a0a72fdc4099d5db59b1932c8329..3d03086ea74e478d8be5a62f05c1772ddd6a9898 100644 (file)
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -1289,7 +1289,7 @@ int virLXCProcessStart(virConnectPtr conn,
      * report implicit runtime defaults in the XML, like vnc listen/socket
      */
     VIR_DEBUG("Setting current domain def as transient");
-    if (virDomainObjSetDefTransient(caps, driver->xmlopt, vm) < 0)
+    if (virDomainObjSetDefTransient(caps, driver->xmlopt, vm, NULL) < 0)
         goto cleanup;
 
     /* Run an early hook to set-up missing devices */

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3004433aa513d2f95fb800514c8663078d588716..f6e9c8672ddc53344edb0c2a0c1dc331f26095d9 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8826,6 +8826,7 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
                                     const char *xml,
                                     unsigned int flags)
 {
+    qemuDomainObjPrivatePtr priv = vm->privateData;
     virDomainDefPtr vmdef = NULL;
     virQEMUDriverConfigPtr cfg = NULL;
     virDomainDeviceDefPtr devConf = NULL;
@@ -8847,7 +8848,8 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
      * rely on the correct vm->def or vm->newDef being passed, so call the
      * device parse based on which definition is in use */
     if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
-        vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt);
+        vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt,
+                                              priv->qemuCaps);
         if (!vmdef)
             goto cleanup;
 
@@ -8965,6 +8967,7 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
 {
     virQEMUDriverPtr driver = dom->conn->privateData;
     virDomainObjPtr vm = NULL;
+    qemuDomainObjPrivatePtr priv;
     virDomainDefPtr vmdef = NULL;
     virDomainDeviceDefPtr dev = NULL, dev_copy = NULL;
     bool force = (flags & VIR_DOMAIN_DEVICE_MODIFY_FORCE) != 0;
@@ -8987,6 +8990,8 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
     if (!(vm = qemuDomObjFromDomain(dom)))
         goto cleanup;
 
+    priv = vm->privateData;
+
     if (virDomainUpdateDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
         goto cleanup;
 
@@ -9019,7 +9024,8 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
 
     if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
         /* Make a copy for updated domain. */
-        vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt);
+        vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt,
+                                              priv->qemuCaps);
         if (!vmdef)
             goto endjob;
 
@@ -9077,6 +9083,7 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver,
                                     const char *xml,
                                     unsigned int flags)
 {
+    qemuDomainObjPrivatePtr priv = vm->privateData;
     virCapsPtr caps = NULL;
     virQEMUDriverConfigPtr cfg = NULL;
     virDomainDeviceDefPtr dev = NULL, dev_copy = NULL;
@@ -9115,7 +9122,7 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver,
 
     if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
         /* Make a copy for updated domain. */
-        vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt);
+        vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt, priv->qemuCaps);
         if (!vmdef)
             goto cleanup;
 
@@ -9171,6 +9178,7 @@ qemuDomainDetachDeviceAliasLiveAndConfig(virQEMUDriverPtr driver,
                                          const char *alias,
                                          unsigned int flags)
 {
+    qemuDomainObjPrivatePtr priv = vm->privateData;
     virCapsPtr caps = NULL;
     virQEMUDriverConfigPtr cfg = NULL;
     virDomainDefPtr def = NULL;
@@ -9197,7 +9205,8 @@ qemuDomainDetachDeviceAliasLiveAndConfig(virQEMUDriverPtr driver,
     if (persistentDef) {
         virDomainDeviceDef dev;
 
-        if (!(vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt)))
+        if (!(vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt,
+                                                    priv->qemuCaps)))
             goto cleanup;
 
         if (virDomainDefFindDevice(vmdef, alias, &dev, true) < 0)
@@ -10810,7 +10819,8 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
     if (persistentDef) {
         /* Make a copy for updated domain. */
         if (!(persistentDefCopy = virDomainObjCopyPersistentDef(vm, caps,
-                                                                driver->xmlopt)))
+                                                                driver->xmlopt,
+                                                                priv->qemuCaps)))
             goto endjob;
     }
 
@@ -16477,6 +16487,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
     if (!(vm = qemuDomObjFromSnapshot(snapshot)))
         goto cleanup;
 
+    priv = vm->privateData;
     cfg = virQEMUDriverGetConfig(driver);
 
     if (virDomainRevertToSnapshotEnsureACL(snapshot->domain->conn, vm->def) < 0)
@@ -16553,7 +16564,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
      * than inactive xml?  */
     if (snap->def->dom) {
         config = virDomainDefCopy(snap->def->dom, caps,
-                                  driver->xmlopt, NULL, true);
+                                  driver->xmlopt, priv->qemuCaps, true);
         if (!config)
             goto endjob;
     }
@@ -16563,7 +16574,6 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
     switch ((virDomainSnapshotState) snapdef->state) {
     case VIR_DOMAIN_SNAPSHOT_RUNNING:
     case VIR_DOMAIN_SNAPSHOT_PAUSED:
-        priv = vm->privateData;
         start_flags |= VIR_QEMU_PROCESS_START_PAUSED;
 
         /* Transitions 2, 3, 5, 6, 8, 9 */
@@ -22016,6 +22026,7 @@ qemuDomainGetFSInfo(virDomainPtr dom,
                     unsigned int flags)
 {
     virQEMUDriverPtr driver = dom->conn->privateData;
+    qemuDomainObjPrivatePtr priv;
     virDomainObjPtr vm;
     qemuAgentPtr agent;
     virCapsPtr caps = NULL;
@@ -22027,6 +22038,8 @@ qemuDomainGetFSInfo(virDomainPtr dom,
     if (!(vm = qemuDomObjFromDomain(dom)))
         return ret;
 
+    priv = vm->privateData;
+
     if (virDomainGetFSInfoEnsureACL(dom->conn, vm->def) < 0)
         goto cleanup;
 
@@ -22042,7 +22055,7 @@ qemuDomainGetFSInfo(virDomainPtr dom,
     if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
         goto endjob;
 
-    if (!(def = virDomainDefCopy(vm->def, caps, driver->xmlopt, NULL, false)))
+    if (!(def = virDomainDefCopy(vm->def, caps, driver->xmlopt, priv->qemuCaps, false)))
         goto endjob;
 
     agent = qemuDomainObjEnterAgent(vm);

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index d07482d9f2ff53b4ae8e626206dcab281ad3786f..c2570f7303cd547dcc1e31edf7b6178e17e8211e 100644 (file)
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -4853,6 +4853,7 @@ qemuMigrationDstPersist(virQEMUDriverPtr driver,
                         bool ignoreSaveError)
 {
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+    qemuDomainObjPrivatePtr priv = vm->privateData;
     virCapsPtr caps = NULL;
     virDomainDefPtr vmdef;
     virDomainDefPtr oldDef = NULL;
@@ -4867,7 +4868,8 @@ qemuMigrationDstPersist(virQEMUDriverPtr driver,
     oldDef = vm->newDef;
     vm->newDef = qemuMigrationCookieGetPersistent(mig);
 
-    if (!(vmdef = virDomainObjGetPersistentDef(caps, driver->xmlopt, vm)))
+    if (!(vmdef = virDomainObjGetPersistentDef(caps, driver->xmlopt, vm,
+                                               priv->qemuCaps)))
         goto error;
 
     if (virDomainSaveConfig(cfg->configDir, driver->caps, vmdef) < 0 &&

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 9345ecf1beeb94902fd20a916a4f37418f1bf4b0..ffe43388542823ae77c6e12176845a74da42f2bf 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5637,7 +5637,7 @@ qemuProcessInit(virQEMUDriverPtr driver,
      * report implicit runtime defaults in the XML, like vnc listen/socket
      */
     VIR_DEBUG("Setting current domain def as transient");
-    if (virDomainObjSetDefTransient(caps, driver->xmlopt, vm) < 0)
+    if (virDomainObjSetDefTransient(caps, driver->xmlopt, vm, priv->qemuCaps) < 0)
         goto cleanup;
 
     /* Update qemu capabilities according to lists passed in via namespace */

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 106e8f027e6082c58ffc29eb5df2e1f97e43baff..043d63b12afbe320df688f1f2b2def4eb262ba79 100755 (executable)
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -713,7 +713,7 @@ testDomainStartState(testDriverPtr privconn,
 
     if (virDomainObjSetDefTransient(privconn->caps,
                                     privconn->xmlopt,
-                                    dom) < 0) {
+                                    dom, NULL) < 0) {
         goto cleanup;
     }
 

diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 354068d7481e804421d99d516515e52d727027bd..016a3ed4b12e7c9ae832b958eb4873e50f6e762b 100644 (file)
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -427,7 +427,7 @@ testQemuHotplugCpuPrepare(const char *test,
 
     /* create vm->newDef */
     data->vm->persistent = true;
-    if (virDomainObjSetDefTransient(caps, driver.xmlopt, data->vm) < 0)
+    if (virDomainObjSetDefTransient(caps, driver.xmlopt, data->vm, NULL) < 0)
         goto error;
 
     priv = data->vm->privateData;