On 10.05.2016 08:18, 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.
>
> Signed-off-by: Wang Yufei <james.wangyufei@xxxxxxxxxx>
> ---
> .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
I suspect this doesn't belong here.
> 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;
> }
>
>
Otherwise looking good. ACK to the idea.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list