qemu: Pass correct qemuCaps to virDomainDeviceDefPostParse

Since qemuDomainDeviceDefPostParse 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.

QEMU capabilities lookup (via domainPostParseDataAlloc callback) is
hidden inside virDomainDeviceDefPostParseOne with no way to pass
qemuCaps to virDomainDeviceDef* functions. This patch fixes all
remaining paths leading to virDomainDeviceDefPostParse.

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 c44ceba7d5581a5ffa94cee2dc651dcb42f05c9a..a3b3e5938d9215cd6b7192bb8bb3b83702d6ae93 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5454,22 +5454,24 @@ virDomainDeviceDefPostParseOne(virDomainDeviceDefPtr dev,
                                const virDomainDef *def,
                                virCapsPtr caps,
                                unsigned int flags,
-                               virDomainXMLOptionPtr xmlopt)
+                               virDomainXMLOptionPtr xmlopt,
+                               void *parseOpaque)
 {
-    void *parseOpaque = NULL;
+    void *data = NULL;
     int ret;
 
-    if (xmlopt->config.domainPostParseDataAlloc) {
+    if (!parseOpaque && xmlopt->config.domainPostParseDataAlloc) {
         if (xmlopt->config.domainPostParseDataAlloc(def, caps, flags,
                                                     xmlopt->config.priv,
-                                                    &parseOpaque) < 0)
+                                                    &data) < 0)
             return -1;
+        parseOpaque = data;
     }
 
     ret = virDomainDeviceDefPostParse(dev, def, caps, flags, xmlopt, parseOpaque);
 
-    if (parseOpaque && xmlopt->config.domainPostParseDataFree)
-        xmlopt->config.domainPostParseDataFree(parseOpaque);
+    if (data && xmlopt->config.domainPostParseDataFree)
+        xmlopt->config.domainPostParseDataFree(data);
 
     return ret;
 }
@@ -16279,6 +16281,7 @@ virDomainDeviceDefParse(const char *xmlStr,
                         const virDomainDef *def,
                         virCapsPtr caps,
                         virDomainXMLOptionPtr xmlopt,
+                        void *parseOpaque,
                         unsigned int flags)
 {
     xmlDocPtr xml;
@@ -16441,7 +16444,8 @@ virDomainDeviceDefParse(const char *xmlStr,
     }
 
     /* callback to fill driver specific device aspects */
-    if (virDomainDeviceDefPostParseOne(dev, def, caps, flags, xmlopt) < 0)
+    if (virDomainDeviceDefPostParseOne(dev, def, caps, flags,
+                                       xmlopt, parseOpaque) < 0)
         goto error;
 
     /* validate the configuration */
@@ -29799,7 +29803,8 @@ virDomainDeviceDefPtr
 virDomainDeviceDefCopy(virDomainDeviceDefPtr src,
                        const virDomainDef *def,
                        virCapsPtr caps,
-                       virDomainXMLOptionPtr xmlopt)
+                       virDomainXMLOptionPtr xmlopt,
+                       void *parseOpaque)
 {
     VIR_AUTOCLEAN(virBuffer) buf = VIR_BUFFER_INITIALIZER;
     int flags = VIR_DOMAIN_DEF_FORMAT_INACTIVE | VIR_DOMAIN_DEF_FORMAT_SECURE;
@@ -29887,7 +29892,7 @@ virDomainDeviceDefCopy(virDomainDeviceDefPtr src,
         return NULL;
 
     xmlStr = virBufferContentAndReset(&buf);
-    return virDomainDeviceDefParse(xmlStr, def, caps, xmlopt,
+    return virDomainDeviceDefParse(xmlStr, def, caps, xmlopt, parseOpaque,
                                    VIR_DOMAIN_DEF_PARSE_INACTIVE |
                                    VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE);
 }

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 7b9732fbbc445b760d05fad89976cac18ce9c6f4..0be9f01b19d7eed2a78bec233ee01e51950ed2bf 100644 (file)
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2866,7 +2866,8 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def);
 virDomainDeviceDefPtr virDomainDeviceDefCopy(virDomainDeviceDefPtr src,
                                              const virDomainDef *def,
                                              virCapsPtr caps,
-                                             virDomainXMLOptionPtr xmlopt);
+                                             virDomainXMLOptionPtr xmlopt,
+                                             void *parseOpaque);
 virDomainDeviceInfoPtr virDomainDeviceGetInfo(virDomainDeviceDefPtr device);
 void virDomainDeviceSetData(virDomainDeviceDefPtr device,
                             void *devicedata);
@@ -2998,6 +2999,7 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(const char *xmlStr,
                                               const virDomainDef *def,
                                               virCapsPtr caps,
                                               virDomainXMLOptionPtr xmlopt,
+                                              void *parseOpaque,
                                               unsigned int flags);
 virDomainDiskDefPtr virDomainDiskDefParse(const char *xmlStr,
                                           const virDomainDef *def,

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 0e1bf2f0bbb29840b75b5c854b55b83ef38c49aa..b7a34352c5f690246ec92ffd032426a104d8eb4d 100644 (file)
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -4120,7 +4120,7 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml,
 
     if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
         if (!(dev = virDomainDeviceDefParse(xml, vm->def,
-                                            cfg->caps, driver->xmlopt,
+                                            cfg->caps, driver->xmlopt, NULL,
                                             VIR_DOMAIN_DEF_PARSE_INACTIVE)))
             goto endjob;
 
@@ -4137,7 +4137,7 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml,
         /* If dev exists it was created to modify the domain config. Free it. */
         virDomainDeviceDefFree(dev);
         if (!(dev = virDomainDeviceDefParse(xml, vm->def,
-                                            cfg->caps, driver->xmlopt,
+                                            cfg->caps, driver->xmlopt, NULL,
                                             VIR_DOMAIN_DEF_PARSE_INACTIVE)))
             goto endjob;
 
@@ -4209,7 +4209,7 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
 
     if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
         if (!(dev = virDomainDeviceDefParse(xml, vm->def,
-                                            cfg->caps, driver->xmlopt,
+                                            cfg->caps, driver->xmlopt, NULL,
                                             VIR_DOMAIN_DEF_PARSE_INACTIVE |
                                             VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)))
             goto endjob;
@@ -4227,7 +4227,7 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
         /* If dev exists it was created to modify the domain config. Free it. */
         virDomainDeviceDefFree(dev);
         if (!(dev = virDomainDeviceDefParse(xml, vm->def,
-                                            cfg->caps, driver->xmlopt,
+                                            cfg->caps, driver->xmlopt, NULL,
                                             VIR_DOMAIN_DEF_PARSE_INACTIVE |
                                             VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)))
             goto endjob;
@@ -4297,7 +4297,7 @@ libxlDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
 
     if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
         if (!(dev = virDomainDeviceDefParse(xml, vm->def,
-                                            cfg->caps, driver->xmlopt,
+                                            cfg->caps, driver->xmlopt, NULL,
                                             VIR_DOMAIN_DEF_PARSE_INACTIVE)))
             goto cleanup;
 
@@ -4316,7 +4316,7 @@ libxlDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
         /* If dev exists it was created to modify the domain config. Free it. */
         virDomainDeviceDefFree(dev);
         if (!(dev = virDomainDeviceDefParse(xml, vm->def,
-                                            cfg->caps, driver->xmlopt,
+                                            cfg->caps, driver->xmlopt, NULL,
                                             VIR_DOMAIN_DEF_PARSE_INACTIVE)))
             goto cleanup;
 

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index a2180b197e35fec4cb6b6e48b3422f2a2017de89..d8ae03c77a91b3dece3468d12faefe8a7f5047c3 100644 (file)
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -4716,7 +4716,7 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom,
         goto endjob;
 
     dev = dev_copy = virDomainDeviceDefParse(xml, vm->def,
-                                             caps, driver->xmlopt,
+                                             caps, driver->xmlopt, NULL,
                                              VIR_DOMAIN_DEF_PARSE_INACTIVE);
     if (dev == NULL)
         goto endjob;
@@ -4728,7 +4728,7 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom,
          * to CONFIG takes one instance.
          */
         dev_copy = virDomainDeviceDefCopy(dev, vm->def,
-                                          caps, driver->xmlopt);
+                                          caps, driver->xmlopt, NULL);
         if (!dev_copy)
             goto endjob;
     }
@@ -4835,7 +4835,7 @@ static int lxcDomainUpdateDeviceFlags(virDomainPtr dom,
     if (!(caps = virLXCDriverGetCapabilities(driver, false)))
         goto endjob;
 
-    if (!(dev = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt,
+    if (!(dev = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt, NULL,
                                         VIR_DOMAIN_DEF_PARSE_INACTIVE)))
         goto endjob;
 
@@ -4899,7 +4899,7 @@ static int lxcDomainDetachDeviceFlags(virDomainPtr dom,
         goto endjob;
 
     dev = dev_copy = virDomainDeviceDefParse(xml, vm->def,
-                                             caps, driver->xmlopt,
+                                             caps, driver->xmlopt, NULL,
                                              VIR_DOMAIN_DEF_PARSE_INACTIVE |
                                              VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE);
     if (dev == NULL)
@@ -4912,7 +4912,7 @@ static int lxcDomainDetachDeviceFlags(virDomainPtr dom,
          * to CONFIG takes one instance.
          */
         dev_copy = virDomainDeviceDefCopy(dev, vm->def,
-                                          caps, driver->xmlopt);
+                                          caps, driver->xmlopt, NULL);
         if (!dev_copy)
             goto endjob;
     }

diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index 39eeb2c12e7ebdd802962f41ca963f222c7f2c75..98db7fceece4afb3827aa2881f92efc977ac0eff 100644 (file)
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -1962,7 +1962,7 @@ openvzDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
     if (!(def = virDomainObjGetOneDef(vm, flags)))
         goto cleanup;
 
-    dev = virDomainDeviceDefParse(xml, def, driver->caps, driver->xmlopt,
+    dev = virDomainDeviceDefParse(xml, def, driver->caps, driver->xmlopt, NULL,
                                   VIR_DOMAIN_DEF_PARSE_INACTIVE);
     if (!dev)
         goto cleanup;

diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index a4b440a24f3c4a5a2c9ead0096b1f1e04d73979a..1638d527fde8eb8df35142fd13d894aef74b52b7 100644 (file)
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -1691,7 +1691,7 @@ phypDomainAttachDeviceFlags(virDomainPtr domain,
 
     def->os.type = VIR_DOMAIN_OSTYPE_LINUX;
 
-    dev = virDomainDeviceDefParse(xml, def, phyp_driver->caps, NULL,
+    dev = virDomainDeviceDefParse(xml, def, phyp_driver->caps, NULL, NULL,
                                   VIR_DOMAIN_DEF_PARSE_INACTIVE);
     if (!dev)
         goto cleanup;

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index db6b00852fd46a3b63ad67b6096b8b005a7e21b6..a60231a658bec6ef80c99b799e52a835d2713542 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8866,7 +8866,8 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
             goto cleanup;
 
         if (!(devConf = virDomainDeviceDefParse(xml, vmdef, caps,
-                                                driver->xmlopt, parse_flags)))
+                                                driver->xmlopt, priv->qemuCaps,
+                                                parse_flags)))
             goto cleanup;
 
         if (virDomainDeviceValidateAliasForHotplug(vm, devConf,
@@ -8886,7 +8887,8 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
 
     if (flags & VIR_DOMAIN_AFFECT_LIVE) {
         if (!(devLive = virDomainDeviceDefParse(xml, vm->def, caps,
-                                                driver->xmlopt, parse_flags)))
+                                                driver->xmlopt, priv->qemuCaps,
+                                                parse_flags)))
             goto cleanup;
 
         if (virDomainDeviceValidateAliasForHotplug(vm, devLive,
@@ -9017,8 +9019,8 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
         !(flags & VIR_DOMAIN_AFFECT_LIVE))
         parse_flags |= VIR_DOMAIN_DEF_PARSE_INACTIVE;
 
-    dev = dev_copy = virDomainDeviceDefParse(xml, vm->def,
-                                             caps, driver->xmlopt,
+    dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, caps,
+                                             driver->xmlopt, priv->qemuCaps,
                                              parse_flags);
     if (dev == NULL)
         goto endjob;
@@ -9029,7 +9031,8 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
          * create a deep copy of device as adding
          * to CONFIG takes one instance.
          */
-        dev_copy = virDomainDeviceDefCopy(dev, vm->def, caps, driver->xmlopt);
+        dev_copy = virDomainDeviceDefCopy(dev, vm->def, caps,
+                                          driver->xmlopt, priv->qemuCaps);
         if (!dev_copy)
             goto endjob;
     }
@@ -9115,8 +9118,8 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver,
         !(flags & VIR_DOMAIN_AFFECT_LIVE))
         parse_flags |= VIR_DOMAIN_DEF_PARSE_INACTIVE;
 
-    dev = dev_copy = virDomainDeviceDefParse(xml, vm->def,
-                                             caps, driver->xmlopt,
+    dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, caps,
+                                             driver->xmlopt, priv->qemuCaps,
                                              parse_flags);
     if (dev == NULL)
         goto cleanup;
@@ -9127,7 +9130,8 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver,
          * create a deep copy of device as adding
          * to CONFIG takes one instance.
          */
-        dev_copy = virDomainDeviceDefCopy(dev, vm->def, caps, driver->xmlopt);
+        dev_copy = virDomainDeviceDefCopy(dev, vm->def, caps,
+                                          driver->xmlopt, priv->qemuCaps);
         if (!dev_copy)
             goto cleanup;
     }

diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index db3d940f8532da2a858ab9f618a323201fb86fb3..49e657cdb70df1ae3dc5e8b39939f9649a0e434f 100644 (file)
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -4313,7 +4313,7 @@ static int vboxDomainAttachDeviceImpl(virDomainPtr dom,
 
     def->os.type = VIR_DOMAIN_OSTYPE_HVM;
 
-    dev = virDomainDeviceDefParse(xml, def, data->caps, data->xmlopt,
+    dev = virDomainDeviceDefParse(xml, def, data->caps, data->xmlopt, NULL,
                                   VIR_DOMAIN_DEF_PARSE_INACTIVE);
     if (dev == NULL)
         goto cleanup;
@@ -4432,7 +4432,7 @@ static int vboxDomainDetachDevice(virDomainPtr dom, const char *xml)
 
     def->os.type = VIR_DOMAIN_OSTYPE_HVM;
 
-    dev = virDomainDeviceDefParse(xml, def, data->caps, data->xmlopt,
+    dev = virDomainDeviceDefParse(xml, def, data->caps, data->xmlopt, NULL,
                                   VIR_DOMAIN_DEF_PARSE_INACTIVE |
                                   VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE);
     if (dev == NULL)

diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 016a3ed4b12e7c9ae832b958eb4873e50f6e762b..6ad67c89021f713dd2d56235b33a3d0f3f1b160b 100644 (file)
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -274,7 +274,7 @@ testQemuHotplug(const void *data)
         device_parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE;
 
     if (!(dev = virDomainDeviceDefParse(device_xml, vm->def,
-                                        caps, driver.xmlopt,
+                                        caps, driver.xmlopt, NULL,
                                         device_parse_flags)))
         goto cleanup;