9:26 a.m.
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(a)redhat.com>
---
src/conf/domain_conf.c | 23 ++++++++++++++---------
src/conf/domain_conf.h | 4 +++-
src/libxl/libxl_driver.c | 12 ++++++------
src/lxc/lxc_driver.c | 10 +++++-----
src/openvz/openvz_driver.c | 2 +-
src/phyp/phyp_driver.c | 2 +-
src/qemu/qemu_driver.c | 20 ++++++++++++--------
src/vbox/vbox_common.c | 4 ++--
tests/qemuhotplugtest.c | 2 +-
9 files changed, 45 insertions(+), 34 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c44ceba7d5..a3b3e5938d 100644
--- 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 7b9732fbbc..0be9f01b19 100644
--- 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 0e1bf2f0bb..b7a34352c5 100644
--- 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 a2180b197e..d8ae03c77a 100644
--- 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 39eeb2c12e..98db7fceec 100644
--- 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 a4b440a24f..1638d527fd 100644
--- 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 db6b00852f..a60231a658 100644
--- 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 db3d940f85..49e657cdb7 100644
--- 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 016a3ed4b1..6ad67c8902 100644
--- 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;
--
2.22.0