On 2016/5/11 13:31, Jim Fehlig wrote: > On 05/10/2016 12:18 AM, Wang Yufei wrote: >> In libxl driver we do virObjectRef in libxlDomainObjBeginJob, >> If virCondWaitUntil failed, it goes to error, do virObjectUnref, >> There's a chance that someone undefine the vm at the same time, >> and refs unref to zero, vm is freed in libxlDomainObjBeginJob. >> But the vm outside function is not Null, we do virObjectUnlock(vm). >> That's how we overwrite the vm memory after it's freed. Because the >> coding amount is much, I fix it partly in libxlDomainCreateWithFlags. >> If my opinion is right and there're no problems, I'll fix them all >> later. > I haven't looked at this patch in detail yet, but I posted a similar patch a > while back > > https://www.redhat.com/archives/libvir-list/2015-June/msg00711.html > > Later, Martin had some good review comments > > https://www.redhat.com/archives/libvir-list/2015-July/msg00557.html > > I was working on a V2 to address his comments, but found that it introduced a > race between saving and destroying a domain. The work got interrupted and > sadly > I haven't found time to resume that. > > Regards, > Jim Thank you for your information. This is a bug I'm dealing with now. I found the newest lixl driver had the same bug, and the solution of qemu driver can be moved here to fix it. It doesn't matter who push the patch. Fixing the bug is the most important thing. If you completed your patch firstly, I'll be happy to see it. ^_^
>> Signed-off-by: Wang Yufei <[email protected]> >> --- >> .gnulib | 1 - >> src/libxl/libxl_domain.c | 6 +----- >> src/libxl/libxl_domain.h | 2 +- >> src/libxl/libxl_driver.c | 5 ++--- >> 4 files changed, 4 insertions(+), 10 deletions(-) >> delete mode 160000 .gnulib >> >> diff --git a/.gnulib b/.gnulib >> deleted file mode 160000 >> index 6cc32c6..0000000 >> --- a/.gnulib >> +++ /dev/null >> @@ -1 +0,0 @@ >> -Subproject commit 6cc32c63e80bc1a30c521b2f07f2b54909b59892 >> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c >> index 14a900c..a90ce53 100644 >> --- a/src/libxl/libxl_domain.c >> +++ b/src/libxl/libxl_domain.c >> @@ -123,7 +123,6 @@ libxlDomainObjBeginJob(libxlDriverPrivatePtr driver >> ATTRIBUTE_UNUSED, >> return -1; >> then = now + LIBXL_JOB_WAIT_TIME; >> >> - virObjectRef(obj); >> >> while (priv->job.active) { >> VIR_DEBUG("Wait normal job condition for starting job: %s", >> @@ -157,7 +156,6 @@ libxlDomainObjBeginJob(libxlDriverPrivatePtr driver >> ATTRIBUTE_UNUSED, >> virReportSystemError(errno, >> "%s", _("cannot acquire job mutex")); >> >> - virObjectUnref(obj); >> return -1; >> } >> >> @@ -171,7 +169,7 @@ libxlDomainObjBeginJob(libxlDriverPrivatePtr driver >> ATTRIBUTE_UNUSED, >> * non-zero, false if the reference count has dropped to zero >> * and obj is disposed. >> */ >> -bool >> +void >> libxlDomainObjEndJob(libxlDriverPrivatePtr driver ATTRIBUTE_UNUSED, >> virDomainObjPtr obj) >> { >> @@ -183,8 +181,6 @@ libxlDomainObjEndJob(libxlDriverPrivatePtr driver >> ATTRIBUTE_UNUSED, >> >> libxlDomainObjResetJob(priv); >> virCondSignal(&priv->job.cond); >> - >> - return virObjectUnref(obj); >> } >> >> int >> diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h >> index 1c1eba3..ce28944 100644 >> --- a/src/libxl/libxl_domain.h >> +++ b/src/libxl/libxl_domain.h >> @@ -85,7 +85,7 @@ libxlDomainObjBeginJob(libxlDriverPrivatePtr driver, >> enum libxlDomainJob job) >> ATTRIBUTE_RETURN_CHECK; >> >> -bool >> +void >> libxlDomainObjEndJob(libxlDriverPrivatePtr driver, >> virDomainObjPtr obj) >> ATTRIBUTE_RETURN_CHECK; >> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c >> index bf97c9c..a46994a 100644 >> --- a/src/libxl/libxl_driver.c >> +++ b/src/libxl/libxl_driver.c >> @@ -294,7 +294,7 @@ libxlDomObjFromDomain(virDomainPtr dom) >> libxlDriverPrivatePtr driver = dom->conn->privateData; >> char uuidstr[VIR_UUID_STRING_BUFLEN]; >> >> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); >> + vm = virDomainObjListFindByUUIDRef(driver->domains, dom->uuid); >> if (!vm) { >> virUUIDFormat(dom->uuid, uuidstr); >> virReportError(VIR_ERR_NO_DOMAIN, >> @@ -2691,8 +2691,7 @@ libxlDomainCreateWithFlags(virDomainPtr dom, >> vm = NULL; >> >> cleanup: >> - if (vm) >> - virObjectUnlock(vm); >> + virDomainObjEndAPI(&vm); >> return ret; >> } >> > > . > -- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
