[libvirt] [PATCH] lxc: completely rework reference counting
by Katerina Koukiou
This patch follows the pattern used in qemu driver regarding
reference counting.
It changes lxcDomObjFromDomain() to ref the domain (using
virDomainObjListFindByUUIDRef()) and adds virDomainObjEndAPI() which
should be the only function in which the return value of
virObjectUnref() is checked. This makes all reference counting
deterministic and makes the code a bit clearer.
Signed-off-by: Katerina Koukiou <k.koukiou(a)gmail.com>
---
src/lxc/lxc_domain.c | 16 +---
src/lxc/lxc_domain.h | 5 +-
src/lxc/lxc_driver.c | 216 +++++++++++++++++++--------------------------------
3 files changed, 85 insertions(+), 152 deletions(-)
diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c
index bca2bb2..6fd5423 100644
--- a/src/lxc/lxc_domain.c
+++ b/src/lxc/lxc_domain.c
@@ -80,7 +80,7 @@ virLXCDomainObjFreeJob(virLXCDomainObjPrivatePtr priv)
* in any way
*
* Upon successful return, the object will have its ref count increased,
- * successful calls must be followed by EndJob eventually
+ * Successful calls must be followed by EndJob eventually
*/
int
virLXCDomainObjBeginJob(virLXCDriverPtr driver ATTRIBUTE_UNUSED,
@@ -95,8 +95,6 @@ virLXCDomainObjBeginJob(virLXCDriverPtr driver ATTRIBUTE_UNUSED,
return -1;
then = now + LXC_JOB_WAIT_TIME;
- virObjectRef(obj);
-
while (priv->job.active) {
VIR_DEBUG("Wait normal job condition for starting job: %s",
virLXCDomainJobTypeToString(job));
@@ -126,23 +124,17 @@ virLXCDomainObjBeginJob(virLXCDriverPtr driver ATTRIBUTE_UNUSED,
else
virReportSystemError(errno,
"%s", _("cannot acquire job mutex"));
-
- virObjectUnref(obj);
return -1;
}
/*
- * obj must be locked before calling
+ * obj must be locked and have a reference before calling
*
* To be called after completing the work associated with the
* earlier virLXCDomainBeginJob() call
- *
- * Returns true if the remaining reference count on obj is
- * non-zero, false if the reference count has dropped to zero
- * and obj is disposed.
*/
-bool
+void
virLXCDomainObjEndJob(virLXCDriverPtr driver ATTRIBUTE_UNUSED,
virDomainObjPtr obj)
{
@@ -154,8 +146,6 @@ virLXCDomainObjEndJob(virLXCDriverPtr driver ATTRIBUTE_UNUSED,
virLXCDomainObjResetJob(priv);
virCondSignal(&priv->job.cond);
-
- return virObjectUnref(obj);
}
diff --git a/src/lxc/lxc_domain.h b/src/lxc/lxc_domain.h
index e82c719..5c4f31e 100644
--- a/src/lxc/lxc_domain.h
+++ b/src/lxc/lxc_domain.h
@@ -102,10 +102,9 @@ virLXCDomainObjBeginJob(virLXCDriverPtr driver,
enum virLXCDomainJob job)
ATTRIBUTE_RETURN_CHECK;
-bool
+void
virLXCDomainObjEndJob(virLXCDriverPtr driver,
- virDomainObjPtr obj)
- ATTRIBUTE_RETURN_CHECK;
+ virDomainObjPtr obj);
#endif /* __LXC_DOMAIN_H__ */
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 143089d..4aef78d 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -126,11 +126,11 @@ static virNWFilterCallbackDriver lxcCallbackDriver = {
* lxcDomObjFromDomain:
* @domain: Domain pointer that has to be looked up
*
- * This function looks up @domain and returns the appropriate
- * virDomainObjPtr.
+ * This function looks up @domain and returns the appropriate virDomainObjPtr
+ * that has to be released by calling virDomainObjEndAPI.
*
- * Returns the domain object which is locked on success, NULL
- * otherwise.
+ * Returns the domain object with incremented reference counter which is locked
+ * on success, NULL otherwise.
*/
static virDomainObjPtr
lxcDomObjFromDomain(virDomainPtr domain)
@@ -139,7 +139,7 @@ lxcDomObjFromDomain(virDomainPtr domain)
virLXCDriverPtr driver = domain->conn->privateData;
char uuidstr[VIR_UUID_STRING_BUFLEN];
- vm = virDomainObjListFindByUUID(driver->domains, domain->uuid);
+ vm = virDomainObjListFindByUUIDRef(driver->domains, domain->uuid);
if (!vm) {
virUUIDFormat(domain->uuid, uuidstr);
virReportError(VIR_ERR_NO_DOMAIN,
@@ -283,7 +283,7 @@ static virDomainPtr lxcDomainLookupByUUID(virConnectPtr conn,
virDomainObjPtr vm;
virDomainPtr dom = NULL;
- vm = virDomainObjListFindByUUID(driver->domains, uuid);
+ vm = virDomainObjListFindByUUIDRef(driver->domains, uuid);
if (!vm) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
@@ -301,8 +301,7 @@ static virDomainPtr lxcDomainLookupByUUID(virConnectPtr conn,
dom->id = vm->def->id;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return dom;
}
@@ -347,8 +346,7 @@ static int lxcDomainIsActive(virDomainPtr dom)
ret = virDomainObjIsActive(obj);
cleanup:
- if (obj)
- virObjectUnlock(obj);
+ virDomainObjEndAPI(&obj);
return ret;
}
@@ -367,8 +365,7 @@ static int lxcDomainIsPersistent(virDomainPtr dom)
ret = obj->persistent;
cleanup:
- if (obj)
- virObjectUnlock(obj);
+ virDomainObjEndAPI(&obj);
return ret;
}
@@ -386,8 +383,7 @@ static int lxcDomainIsUpdated(virDomainPtr dom)
ret = obj->updated;
cleanup:
- if (obj)
- virObjectUnlock(obj);
+ virDomainObjEndAPI(&obj);
return ret;
}
@@ -492,13 +488,14 @@ lxcDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags)
driver->xmlopt,
0, &oldDef)))
goto cleanup;
+
+ virObjectRef(vm);
def = NULL;
vm->persistent = 1;
if (virDomainSaveConfig(cfg->configDir, driver->caps,
vm->newDef ? vm->newDef : vm->def) < 0) {
virDomainObjListRemove(driver->domains, vm);
- vm = NULL;
goto cleanup;
}
@@ -515,8 +512,7 @@ lxcDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags)
cleanup:
virDomainDefFree(def);
virDomainDefFree(oldDef);
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
if (event)
virObjectEventStateQueue(driver->domainEventState, event);
virObjectUnref(caps);
@@ -566,14 +562,12 @@ static int lxcDomainUndefineFlags(virDomainPtr dom,
vm->persistent = 0;
} else {
virDomainObjListRemove(driver->domains, vm);
- vm = NULL;
}
ret = 0;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
if (event)
virObjectEventStateQueue(driver->domainEventState, event);
virObjectUnref(cfg);
@@ -628,8 +622,7 @@ static int lxcDomainGetInfo(virDomainPtr dom,
ret = 0;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -654,8 +647,7 @@ lxcDomainGetState(virDomainPtr dom,
ret = 0;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -674,8 +666,7 @@ static char *lxcDomainGetOSType(virDomainPtr dom)
goto cleanup;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -695,8 +686,7 @@ lxcDomainGetMaxMemory(virDomainPtr dom)
ret = virDomainDefGetMemoryActual(vm->def);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -790,12 +780,10 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
ret = 0;
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(caps);
virObjectUnref(cfg);
return ret;
@@ -940,12 +928,10 @@ lxcDomainSetMemoryParameters(virDomainPtr dom,
ret = 0;
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(caps);
virObjectUnref(cfg);
return ret;
@@ -1042,8 +1028,7 @@ lxcDomainGetMemoryParameters(virDomainPtr dom,
ret = 0;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(caps);
return ret;
}
@@ -1069,8 +1054,7 @@ static char *lxcDomainGetXMLDesc(virDomainPtr dom,
virDomainDefFormatConvertXMLFlags(flags));
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -1166,12 +1150,10 @@ static int lxcDomainCreateWithFiles(virDomainPtr dom,
}
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
if (event)
virObjectEventStateQueue(driver->domainEventState, event);
virObjectUnref(cfg);
@@ -1301,13 +1283,11 @@ lxcDomainCreateXMLWithFiles(virConnectPtr conn,
dom->id = vm->def->id;
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
virDomainDefFree(def);
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
if (event)
virObjectEventStateQueue(driver->domainEventState, event);
virObjectUnref(caps);
@@ -1390,8 +1370,7 @@ static int lxcDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr secla
ret = 0;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -1568,12 +1547,10 @@ lxcDomainDestroyFlags(virDomainPtr dom,
}
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
if (event)
virObjectEventStateQueue(driver->domainEventState, event);
return ret;
@@ -1907,8 +1884,7 @@ static char *lxcDomainGetSchedulerType(virDomainPtr dom,
ignore_value(VIR_STRDUP(ret, "posix"));
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -2089,13 +2065,11 @@ lxcDomainSetSchedulerParametersFlags(virDomainPtr dom,
ret = 0;
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
virDomainDefFree(vmdef);
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(caps);
virObjectUnref(cfg);
return ret;
@@ -2206,8 +2180,7 @@ lxcDomainGetSchedulerParametersFlags(virDomainPtr dom,
ret = 0;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(caps);
return ret;
}
@@ -2461,12 +2434,10 @@ lxcDomainBlockStats(virDomainPtr dom,
&stats->wr_req);
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -2594,12 +2565,10 @@ lxcDomainBlockStatsFlags(virDomainPtr dom,
*nparams = tmp;
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -2809,12 +2778,10 @@ lxcDomainSetBlkioParameters(virDomainPtr dom,
}
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(caps);
virObjectUnref(cfg);
return ret;
@@ -3209,8 +3176,7 @@ lxcDomainGetBlkioParameters(virDomainPtr dom,
ret = 0;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(caps);
return ret;
}
@@ -3258,12 +3224,10 @@ lxcDomainInterfaceStats(virDomainPtr dom,
_("Invalid path, '%s' is not a known interface"), path);
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
#else
@@ -3293,8 +3257,7 @@ static int lxcDomainGetAutostart(virDomainPtr dom,
ret = 0;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -3365,13 +3328,12 @@ static int lxcDomainSetAutostart(virDomainPtr dom,
ret = 0;
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
+
cleanup:
VIR_FREE(configFile);
VIR_FREE(autostartLink);
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(cfg);
return ret;
}
@@ -3503,13 +3465,12 @@ static int lxcDomainSuspend(virDomainPtr dom)
ret = 0;
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
+
cleanup:
if (event)
virObjectEventStateQueue(driver->domainEventState, event);
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(cfg);
return ret;
}
@@ -3559,13 +3520,12 @@ static int lxcDomainResume(virDomainPtr dom)
ret = 0;
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
+
cleanup:
if (event)
virObjectEventStateQueue(driver->domainEventState, event);
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(cfg);
return ret;
}
@@ -3630,8 +3590,7 @@ lxcDomainOpenConsole(virDomainPtr dom,
ret = 0;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -3707,12 +3666,10 @@ lxcDomainSendProcessSignal(virDomainPtr dom,
ret = 0;
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -3814,12 +3771,10 @@ lxcDomainShutdownFlags(virDomainPtr dom,
ret = 0;
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -3899,12 +3854,10 @@ lxcDomainReboot(virDomainPtr dom,
ret = 0;
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -5208,15 +5161,14 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom,
}
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
+
cleanup:
virDomainDefFree(vmdef);
if (dev != dev_copy)
virDomainDeviceDefFree(dev_copy);
virDomainDeviceDefFree(dev);
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(caps);
virObjectUnref(cfg);
return ret;
@@ -5314,15 +5266,14 @@ static int lxcDomainUpdateDeviceFlags(virDomainPtr dom,
}
}
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
+
cleanup:
virDomainDefFree(vmdef);
if (dev != dev_copy)
virDomainDeviceDefFree(dev_copy);
virDomainDeviceDefFree(dev);
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(caps);
virObjectUnref(cfg);
return ret;
@@ -5419,15 +5370,14 @@ static int lxcDomainDetachDeviceFlags(virDomainPtr dom,
}
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
+
cleanup:
virDomainDefFree(vmdef);
if (dev != dev_copy)
virDomainDeviceDefFree(dev_copy);
virDomainDeviceDefFree(dev);
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(caps);
virObjectUnref(cfg);
return ret;
@@ -5484,12 +5434,10 @@ static int lxcDomainLxcOpenNamespace(virDomainPtr dom,
ret = nfds;
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -5586,11 +5534,10 @@ lxcDomainMemoryStats(virDomainPtr dom,
}
endjob:
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
+
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -5738,12 +5685,10 @@ lxcDomainSetMetadata(virDomainPtr dom,
driver->xmlopt, cfg->stateDir,
cfg->configDir, flags);
- if (!virLXCDomainObjEndJob(driver, vm))
- vm = NULL;
+ virLXCDomainObjEndJob(driver, vm);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(caps);
virObjectUnref(cfg);
return ret;
@@ -5773,7 +5718,7 @@ lxcDomainGetMetadata(virDomainPtr dom,
ret = virDomainObjGetMetadata(vm, type, uri, caps, driver->xmlopt, flags);
cleanup:
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
virObjectUnref(caps);
return ret;
}
@@ -5820,7 +5765,7 @@ lxcDomainGetCPUStats(virDomainPtr dom,
ret = virCgroupGetPercpuStats(priv->cgroup, params,
nparams, start_cpu, ncpus, NULL);
cleanup:
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -5881,8 +5826,7 @@ lxcDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags)
ret = 0;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
--
2.7.4
8 years, 11 months
[libvirt] [PATCH] lxc: use job functions in lxcDomainLxcOpenNamespace & lxcDomainSendProcessSignal
by Katerina Koukiou
Use the recently added job functions in lxcDomainLxcOpenNamespace,
lxcDomainSendProcessSignal.
Signed-off-by: Katerina Koukiou <k.koukiou(a)gmail.com>
---
src/lxc/lxc_driver.c | 31 ++++++++++++++++++++++++-------
1 file changed, 24 insertions(+), 7 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 7cdea2c..143089d 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -3642,6 +3642,7 @@ lxcDomainSendProcessSignal(virDomainPtr dom,
unsigned int signum,
unsigned int flags)
{
+ virLXCDriverPtr driver = dom->conn->privateData;
virDomainObjPtr vm = NULL;
virLXCDomainObjPrivatePtr priv;
pid_t victim;
@@ -3664,10 +3665,13 @@ lxcDomainSendProcessSignal(virDomainPtr dom,
if (virDomainSendProcessSignalEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
+ if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0)
+ goto cleanup;
+
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("domain is not running"));
- goto cleanup;
+ goto endjob;
}
/*
@@ -3680,13 +3684,13 @@ lxcDomainSendProcessSignal(virDomainPtr dom,
if (pid_value != 1) {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
_("Only the init process may be killed"));
- goto cleanup;
+ goto endjob;
}
if (!priv->initpid) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("Init pid is not yet available"));
- goto cleanup;
+ goto endjob;
}
victim = priv->initpid;
@@ -3697,11 +3701,15 @@ lxcDomainSendProcessSignal(virDomainPtr dom,
virReportSystemError(errno,
_("Unable to send %d signal to process %d"),
signum, victim);
- goto cleanup;
+ goto endjob;
}
ret = 0;
+ endjob:
+ if (!virLXCDomainObjEndJob(driver, vm))
+ vm = NULL;
+
cleanup:
if (vm)
virObjectUnlock(vm);
@@ -5438,6 +5446,7 @@ static int lxcDomainLxcOpenNamespace(virDomainPtr dom,
int **fdlist,
unsigned int flags)
{
+ virLXCDriverPtr driver = dom->conn->privateData;
virDomainObjPtr vm;
virLXCDomainObjPrivatePtr priv;
int ret = -1;
@@ -5454,22 +5463,30 @@ static int lxcDomainLxcOpenNamespace(virDomainPtr dom,
if (virDomainLxcOpenNamespaceEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
+ if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_QUERY) < 0)
+ goto cleanup;
+
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("Domain is not running"));
- goto cleanup;
+ goto endjob;
}
if (!priv->initpid) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("Init pid is not yet available"));
- goto cleanup;
+ goto endjob;
}
if (virProcessGetNamespaces(priv->initpid, &nfds, fdlist) < 0)
- goto cleanup;
+ goto endjob;
ret = nfds;
+
+ endjob:
+ if (!virLXCDomainObjEndJob(driver, vm))
+ vm = NULL;
+
cleanup:
if (vm)
virObjectUnlock(vm);
--
2.7.4
8 years, 11 months
[libvirt] [PATCH v3 00/13] PCI Multifunction device hotplug support
by Shivaprasad G Bhat
V2 was posted here.
http://www.redhat.com/archives/libvir-list/2016-May/msg01384.html
Changes from V2:
-Squashed the original patch 1 with the patch which actually enables
multifunction hotplug as suggested.
-As suggested, introduced the virDomainDeviceDefParseMany for
parsing.
-Retained the patch which validates the function address before releasing. I
feel this should be retained as originally checks used to happen for slots.
-Changed the assign address logic to simply decrementally assign function
numbers to the devices in the list. The CONNECT_TYPE flags are yet to be
explored.
-Split the original patch 8 into multiple simpler patches for easier reading.
The original motivation was to help introduce test cases for multifunction
hotplug which is not possible for functions static in qemu_driver.c. So,
moved the functions to qemu_domain.c and qemu_hotplug.c. Hopefully, the
movements helps adding good number of test cases.
V1 was posted here.
http://www.redhat.com/archives/libvir-list/2016-May/msg01178.html
Changes from V1
Fixed couple of issues in address validation and assignment.
Added Rollback of the hotplug if anything fails in between.
Removed Patch 6 completely as it exposed way too many bugs. Changed the approach
a bit by introducing 2 new patches which take care of hostdevice preparation
and help reverting.
Todo:
1) Hardening the hotplug checks to disallow multifunction cards
hotplug as though they are single function cards.
2) Documentation update.
3) Test cases. This is easier now with the helper functions moved out of
qemu_driver.c
4) Should the events be delayed till all the functions are
hotplugged/unplugged? Something can fail and the revert may not be possible
Or the guest may refuse to free the device. If we show events
for those which got free, the rest of them may never be freed and also
that may not be usable by guest if the function 0 is not part of it. Need to
think more on this. Any suggestions ?
---
Shivaprasad G Bhat (13):
Release address in function granularity than slot
Validate address in virDomainPCIAddressReleaseAddr
Introduce PCI Multifunction device parser
Introduce virDomainPCIMultifunctionDeviceAddressAssign
Separate the hostdevice preparation and checks to a new funtion
Move the qemu[*]DomainDeviceConfig to qemu_domain.c
Move qemuDomain[*]Live functions to qemu_hotplug.c
Introduce qemuDomain*DeviceLiveInternal
Introduce qemuDomain*DeviceConfigInternal
Move the virDomainDefCompatibleDevice checks a level down
Move the detach of PCI device to the beginnging of live hotplug
Pass virDomainDeviceDefListPtr to hotplug functions
Enable PCI Multifunction hotplug/unplug
src/conf/domain_addr.c | 153 ++++++-
src/conf/domain_addr.h | 4
src/conf/domain_conf.c | 123 +++++-
src/conf/domain_conf.h | 19 +
src/libvirt_private.syms | 5
src/qemu/qemu_domain.c | 460 +++++++++++++++++++++
src/qemu/qemu_domain.h | 17 +
src/qemu/qemu_domain_address.c | 2
src/qemu/qemu_driver.c | 882 ++++++----------------------------------
src/qemu/qemu_hotplug.c | 625 ++++++++++++++++++++++++----
src/qemu/qemu_hotplug.h | 24 +
11 files changed, 1421 insertions(+), 893 deletions(-)
--
Signature
8 years, 11 months
[libvirt] [PATCH 00/10] update caps data for our tests
by Pavel Hrdina
To get replies from qemu the tests/qemucapsprobe tool was used with following
QEMUs:
qemu-1.2.2 and qemu-1.3.1 on Fedora18
qemu-1.4.2 and qemu-1.5.3 on Fedora19
qemu-1.6.0 on Fedora20
qemu-2.1.1 on Fedora21
qemu-2.4.0 and qemu-2.5.0 on Fedora23
qemu-2.6.0 on Fedora24
To get as much as possible dependencies I've used "yum-builddeb qemu",
uninstalled "xen" packages and if it was missing installed "spice" packages.
Pavel Hrdina (10):
qemucapstest: update caps for qemu-1.2.2
qemucapstest: update caps for qemu-1.3.1
qemucapstest: update caps for qemu-1.4.2
qemucapstest: update caps for qemu-1.5.3
qemucapstest: update caps for qemu-1.6.0
qemucapstest: update caps for qemu-2.1.1
qemucapstest: update caps for qemu-2.4.0
qemucapstest: update caps for qemu-2.5.0
qemucapstest: update caps for qemu-2.6.0
qemucapstest: remove data for qemu-1.6.50
tests/domaincapsschemadata/qemu_1.6.0.x86_64.xml | 78 +
tests/domaincapsschemadata/qemu_1.6.50.x86_64.xml | 78 -
tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml | 2 +-
tests/domaincapstest.c | 2 +-
.../qemucapabilitiesdata/caps_1.2.2.x86_64.replies | 3483 ++++++-------
tests/qemucapabilitiesdata/caps_1.2.2.x86_64.xml | 2 -
.../qemucapabilitiesdata/caps_1.3.1.x86_64.replies | 3943 ++++++++-------
tests/qemucapabilitiesdata/caps_1.3.1.x86_64.xml | 5 +-
.../qemucapabilitiesdata/caps_1.4.2.x86_64.replies | 4050 +++++++--------
tests/qemucapabilitiesdata/caps_1.4.2.x86_64.xml | 5 +-
.../qemucapabilitiesdata/caps_1.5.3.x86_64.replies | 5245 ++++++++++----------
tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml | 4 +
.../qemucapabilitiesdata/caps_1.6.0.x86_64.replies | 5244 +++++++++----------
tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml | 5 +-
.../caps_1.6.50.x86_64.replies | 2848 -----------
tests/qemucapabilitiesdata/caps_1.6.50.x86_64.xml | 199 -
.../qemucapabilitiesdata/caps_2.1.1.x86_64.replies | 665 +--
tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml | 2 -
.../qemucapabilitiesdata/caps_2.4.0.x86_64.replies | 411 +-
tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 1 +
.../qemucapabilitiesdata/caps_2.5.0.x86_64.replies | 1587 +++---
tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 14 +-
.../qemucapabilitiesdata/caps_2.6.0.x86_64.replies | 1877 ++++---
tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 33 +-
tests/qemucapabilitiestest.c | 1 -
25 files changed, 13934 insertions(+), 15850 deletions(-)
create mode 100644 tests/domaincapsschemadata/qemu_1.6.0.x86_64.xml
delete mode 100644 tests/domaincapsschemadata/qemu_1.6.50.x86_64.xml
delete mode 100644 tests/qemucapabilitiesdata/caps_1.6.50.x86_64.replies
delete mode 100644 tests/qemucapabilitiesdata/caps_1.6.50.x86_64.xml
--
2.8.2
8 years, 11 months
[libvirt] [PATCH] nwfilter: fix lock order deadlock
by Maxim Nestratov
Below is backtraces of two deadlocked threads:
thread #1:
virDomainConfVMNWFilterTeardown
virNWFilterTeardownFilter
lock updateMutex <------------
_virNWFilterTeardownFilter
try to lock interface <----------
thread #2:
learnIPAddressThread
lock interface <-------
virNWFilterInstantiateFilterLate
try to lock updateMutex <----------
The problem is fixed by unlocking interface before calling
virNWFilterInstantiateFilterLate to avoid updateMutex and interface ordering
deadlocks. Otherwise we are going to instantiate the filter while holding
interface lock, which will try to lock updateMutex, and if some other thread
instantiating a filter in parallel is holding updateMutex and is trying to
lock interface, both will deadlock.
Also it is safe to unlock interface before virNWFilterInstantiateFilterLate
because learnIPAddressThread stopped capturing packets and applied necessary
rules on the interface, while instantiating a new filter doesn't require a
locked interface.
Signed-off-by: Maxim Nestratov <mnestratov(a)virtuozzo.com>
---
src/nwfilter/nwfilter_learnipaddr.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c
index 1adbadb..cfd92d9 100644
--- a/src/nwfilter/nwfilter_learnipaddr.c
+++ b/src/nwfilter/nwfilter_learnipaddr.c
@@ -611,6 +611,16 @@ learnIPAddressThread(void *arg)
sa.data.inet4.sin_addr.s_addr = vmaddr;
char *inetaddr;
+ /* It is necessary to unlock interface here to avoid updateMutex and
+ * interface ordering deadlocks. Otherwise we are going to
+ * instantiate the filter, which will try to lock updateMutex, and
+ * some other thread instantiating a filter in parallel is holding
+ * updateMutex and is trying to lock interface, both will deadlock.
+ * Also it is safe to unlock interface here because we stopped
+ * capturing and applied necessary rules on the interface, while
+ * instantiating a new filter doesn't require a locked interface.*/
+ virNWFilterUnlockIface(req->ifname);
+
if ((inetaddr = virSocketAddrFormat(&sa)) != NULL) {
if (virNWFilterIPAddrMapAddIPAddr(req->ifname, inetaddr) < 0) {
VIR_ERROR(_("Failed to add IP address %s to IP address "
@@ -636,11 +646,11 @@ learnIPAddressThread(void *arg)
req->ifname, req->ifindex);
techdriver->applyDropAllRules(req->ifname);
+ virNWFilterUnlockIface(req->ifname);
}
VIR_DEBUG("pcap thread terminating for interface %s", req->ifname);
- virNWFilterUnlockIface(req->ifname);
err_no_lock:
virNWFilterDeregisterLearnReq(req->ifindex);
--
2.4.3
8 years, 11 months
[libvirt] [PATCH] maint: fix syntax-check sc_prohibit_int_ijk exclude rule
by Pavel Hrdina
This fixes the "include/" path, we need to create a regex for the whole path
because we are matching the whole line.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
I'm surprised that it even worked so far. I've noticed this after system update
few days ago.
cfg.mk | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cfg.mk b/cfg.mk
index c19f615..f688cbb 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1259,7 +1259,7 @@ exclude_file_name_regexp--sc_prohibit_include_public_headers_brackets = \
^(tools/|examples/|include/libvirt/(virterror|libvirt(-(admin|qemu|lxc))?)\.h$$)
exclude_file_name_regexp--sc_prohibit_int_ijk = \
- ^(src/remote_protocol-structs|src/remote/remote_protocol.x|cfg.mk|include/|src/admin_protocol-structs|src/admin/admin_protocol.x)$
+ ^(src/remote_protocol-structs|src/remote/remote_protocol.x|cfg.mk|include/libvirt/libvirt.+|src/admin_protocol-structs|src/admin/admin_protocol.x)$$
exclude_file_name_regexp--sc_prohibit_getenv = \
^tests/.*\.[ch]$$
--
2.8.3
8 years, 11 months
[libvirt] [PATCH 0/2] some build fixes
by Pavel Hrdina
Pavel Hrdina (2):
qemuxml2argvtest: skip test that depends on gnutls_cipher_encrypt()
makefile: fix build on systems where gnutls is not in /usr/include
src/Makefile.am | 2 +-
tests/qemuxml2argvtest.c | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
--
2.8.2
8 years, 11 months
[libvirt] [PATCH 0/2] Set PCI_PHYSICAL_FUNCTION flag more carefully
by Andrea Bolognani
If you're running libvirt master on a remote host, and connect
to it using virt-manager, the remote daemon will crash. The
same will happen if you run 'virsh nodedev-dumpxml' for a PCI
device that happens *not* to be a virtual function.
The first patch fixes the crash. The second one fixes something
that could potentially cause trouble down the line, and also
makes the code a tiny bit nicer.
Andrea Bolognani (2):
nodedev: sysfs: Set PCI_PHYSICAL_FUNCTION flag more carefully
conf: nodedev: Set PCI_PHYSICAL_FUNCTION flag more carefully
src/conf/node_device_conf.c | 4 ++--
src/node_device/node_device_linux_sysfs.c | 10 ++++++++--
2 files changed, 10 insertions(+), 4 deletions(-)
--
2.5.5
8 years, 11 months
Re: [libvirt] [PATCH V2 2/4] libxl: support hotplug USB host device
by Jim Fehlig
On 05/23/2016 12:51 AM, Chun Yan Liu wrote:
>
> Yes, I think it's OK. And another place needs to be updated, since now it's
> not only pci device, but also could be usb device, so the error message
> should be updated.
> if (virDomainHostdevFind(vmdef, hostdev, &found) >= 0) {
> - pcisrc = &hostdev->source.subsys.u.pci;
> virReportError(VIR_ERR_OPERATION_FAILED,
> - _("target pci device %.4x:%.2x:%.2x.%.1x\
> - already exists"),
> - pcisrc->addr.domain, pcisrc->addr.bus,
> - pcisrc->addr.slot, pcisrc->addr.function);
> + _("device already exists in domain configuration"));
> return -1;
> }
Nice catch. I've squashed in the below diff, changing the error code and message
to match the qemu driver.
Regards,
Jim
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 5b6f87a..4d7124d 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3293,7 +3293,6 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef,
virDomainDeviceDefPtr dev)
virDomainNetDefPtr net;
virDomainHostdevDefPtr hostdev;
virDomainHostdevDefPtr found;
- virDomainHostdevSubsysPCIPtr pcisrc;
char mac[VIR_MAC_STRING_BUFLEN];
switch (dev->type) {
@@ -3336,12 +3335,8 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef,
virDomainDeviceDefPtr dev)
}
if (virDomainHostdevFind(vmdef, hostdev, &found) >= 0) {
- pcisrc = &hostdev->source.subsys.u.pci;
- virReportError(VIR_ERR_OPERATION_FAILED,
- _("target pci device %.4x:%.2x:%.2x.%.1x\
- already exists"),
- pcisrc->addr.domain, pcisrc->addr.bus,
- pcisrc->addr.slot, pcisrc->addr.function);
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("device is already in the domain configuration"));
return -1;
}
8 years, 11 months
[libvirt] [PATCH v2 0/8] Replace VIR_ERROR with standard vir*Error in state driver init
by Jovanka Gulicoska
Replace VIR_ERROR logging macros for error reporting in driver startup routines
with vir*Error function.
Link to task: http://wiki.libvirt.org/page/BiteSizedTasks#Replace_VIR_ERROR_with_standa...
Jovanka Gulicoska (8):
uml: Replace VIR_ERROR with standard vir*Error in state driver init
xen: Replace VIR_ERROR with standard vir*Error in state driver init
bhyve: Replace VIR_ERROR with standard vir*Error in state driver init
libxl: Replace VIR_ERROR with standard vir*Error in state driver init
node_device: Replace VIR_ERROR with standard vir*Error in state driver
init
nwfilter: Replace VIR_ERROR with standard vir*Error in state driver
init
qemu: Replace VIR_ERROR with standard vir*Error in state driver init
storage: Replace VIR_ERROR with standard vir*Error in state driver
init
src/bhyve/bhyve_driver.c | 4 +--
src/libxl/libxl_driver.c | 6 ++---
src/node_device/node_device_hal.c | 24 ++++++++++-------
src/node_device/node_device_udev.c | 45 +++++++++++++++++++------------
src/nwfilter/nwfilter_driver.c | 5 ++--
src/qemu/qemu_driver.c | 55 ++++++++++++++++++++++++--------------
src/storage/storage_driver.c | 32 +++++++++++++---------
src/uml/uml_driver.c | 20 +++++++-------
src/xen/xen_driver.c | 5 ++--
9 files changed, 119 insertions(+), 77 deletions(-)
--
2.5.5
8 years, 11 months
