May 2016 - Devel - libvirt List Archives

[libvirt] [PATCH] lxc: completely rework reference counting
by Katerina Koukiou 24 May '16

24 May '16

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

2 1

[libvirt] [PATCH] lxc: use job functions in lxcDomainLxcOpenNamespace & lxcDomainSendProcessSignal
by Katerina Koukiou 24 May '16

24 May '16

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

2 1

[libvirt] [PATCH v3 00/13] PCI Multifunction device hotplug support
by Shivaprasad G Bhat 24 May '16

24 May '16

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

2 17

[libvirt] [PATCH 00/10] update caps data for our tests
by Pavel Hrdina 24 May '16

24 May '16

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

2 21

[libvirt] [PATCH] nwfilter: fix lock order deadlock
by Maxim Nestratov 24 May '16

24 May '16

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

5 10

[libvirt] [PATCH] maint: fix syntax-check sc_prohibit_int_ijk exclude rule
by Pavel Hrdina 24 May '16

24 May '16

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

2 2

[libvirt] [PATCH 0/2] some build fixes
by Pavel Hrdina 24 May '16

24 May '16

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

2 7

[libvirt] [PATCH 0/2] Set PCI_PHYSICAL_FUNCTION flag more carefully
by Andrea Bolognani 24 May '16

24 May '16

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

2 4

Re: [libvirt] [PATCH V2 2/4] libxl: support hotplug USB host device
by Jim Fehlig 24 May '16

24 May '16

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; }

2 1

[libvirt] [PATCH v2 0/8] Replace VIR_ERROR with standard vir*Error in state driver init
by Jovanka Gulicoska 23 May '16

23 May '16

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_standard… 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

2 13