[libvirt] [PATCH 07/10] qemu: Pass correct qemuCaps to virDomainDefCopy
Jiri Denemark
jdenemar at redhat.com
Thu Aug 8 14:26:41 UTC 2019
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 at redhat.com>
---
src/conf/domain_conf.c | 18 +++++++++++-------
src/conf/domain_conf.h | 9 ++++++---
src/libxl/libxl_domain.c | 2 +-
src/libxl/libxl_driver.c | 10 +++++-----
src/libxl/libxl_migration.c | 2 +-
src/lxc/lxc_driver.c | 8 ++++----
src/lxc/lxc_process.c | 2 +-
src/qemu/qemu_driver.c | 29 +++++++++++++++++++++--------
src/qemu/qemu_migration.c | 4 +++-
src/qemu/qemu_process.c | 2 +-
src/test/test_driver.c | 2 +-
tests/qemuhotplugtest.c | 2 +-
12 files changed, 56 insertions(+), 34 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0456369d55..c44ceba7d5 100644
--- 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 57ca2a8ad1..7b9732fbbc 100644
--- 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 2d8569e592..b60511a266 100644
--- 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 f430522b35..0e1bf2f0bb 100644
--- 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 76bcb66a11..a1021d499b 100644
--- 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 d0b6703101..a2180b197e 100644
--- 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 a1d028b2e6..3d03086ea7 100644
--- 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 3004433aa5..f6e9c8672d 100644
--- 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 d07482d9f2..c2570f7303 100644
--- 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 9345ecf1be..ffe4338854 100644
--- 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 6f18baa265..85cfcce79f 100755
--- 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 354068d748..016a3ed4b1 100644
--- 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;
--
2.22.0
More information about the libvir-list
mailing list