On 04/19/2018 09:51 AM, Daniel P. Berrangé wrote:
On Mon, Apr 02, 2018 at 09:06:13AM -0400, John Ferlan wrote:
The virDomainObjListFindByName will return a locked and reffed object. If we call virDomainObjListRemove that will unlock the object upon return, thus we need to relock the object before making the call to virDomainObjEndAPI.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/uml/uml_driver.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Matches what we do everywhere else, but ewwww if the caller owns the lock, and the 'dom' hasn't been freed entirely, it is pretty evil semantics to have the lock be lost. Why can't virDomainObjListRemove leave the lock state unchanged. It already unlocks, then locks, then unlocks again. A final lock would avoid such suprising semantics
Yep... 100% agree. I'm trying to get to that type of model, but taking "baby steps" through each of the domain object consumers. Still on list are the vmware and vz to at least change the FindByUUID semantics. Once that happens changing domain object Add and Remove are the next step. I also have a series on the list undoing the lock fire dance (Erik's term ;-)) for secret, interface, nodedev, and storage. I was hoping to follow the same model for domain Remove. Thanks for the review, John
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index ff2e7ac66b..b84585d728 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -346,8 +346,10 @@ umlInotifyEvent(int watch, event = virDomainEventLifecycleNewFromObj(dom, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN); - if (!dom->persistent) + if (!dom->persistent) { virDomainObjListRemove(driver->domains, dom); + virObjectLock(dom); + } } else if (e.mask & (IN_CREATE | IN_MODIFY)) { VIR_DEBUG("Got inotify domain startup '%s'", name); if (virDomainObjIsActive(dom)) { @@ -377,8 +379,10 @@ umlInotifyEvent(int watch, event = virDomainEventLifecycleNewFromObj(dom, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_FAILED); - if (!dom->persistent) + if (!dom->persistent) { virDomainObjListRemove(driver->domains, dom); + virObjectLock(dom); + } } else if (umlIdentifyChrPTY(driver, dom) < 0) { VIR_WARN("Could not identify character devices for new domain"); umlShutdownVMDaemon(driver, dom, @@ -387,8 +391,10 @@ umlInotifyEvent(int watch, event = virDomainEventLifecycleNewFromObj(dom, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_FAILED); - if (!dom->persistent) + if (!dom->persistent) { virDomainObjListRemove(driver->domains, dom); + virObjectLock(dom); + } } } virDomainObjEndAPI(&dom); -- 2.13.6
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Regards, Daniel