[libvirt] [PATCH 2/2] qemu: Avoid deadlock in autodestroy
Michal Privoznik
mprivozn at redhat.com
Tue Feb 19 09:21:18 UTC 2013
On 18.02.2013 17:07, Jiri Denemark wrote:
> Since closeCallbacks were turned into virObjectLockable, we can no
> longer call virQEMUCloseCallbacks APIs from within a registered close
> callback.
> ---
> src/qemu/qemu_conf.c | 19 ++++---------------
> src/qemu/qemu_conf.h | 4 ++++
> src/qemu/qemu_domain.h | 1 +
> src/qemu/qemu_process.c | 10 +++++++++-
> 4 files changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index edadf36..fc8e152 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -825,8 +825,6 @@ virQEMUCloseCallbacksRunOne(void *payload,
> {
> struct virQEMUCloseCallbacksData *data = opaque;
> qemuDriverCloseDefPtr closeDef = payload;
> - unsigned char uuid[VIR_UUID_BUFLEN];
> - char uuidstr[VIR_UUID_STRING_BUFLEN];
> virDomainObjPtr dom;
>
> VIR_DEBUG("conn=%p, thisconn=%p, uuid=%s, cb=%p",
> @@ -835,18 +833,9 @@ virQEMUCloseCallbacksRunOne(void *payload,
> if (data->conn != closeDef->conn || !closeDef->cb)
> return;
>
> - if (virUUIDParse(name, uuid) < 0) {
> - VIR_WARN("Failed to parse %s", (const char *)name);
> - return;
> - }
> - /* We need to reformat uuidstr, because closeDef->cb
> - * might cause the current hash entry to be removed,
> - * which means 'name' will have been free()d
> - */
> - virUUIDFormat(uuid, uuidstr);
> -
> - if (!(dom = virDomainObjListFindByUUID(data->driver->domains, uuid))) {
> - VIR_DEBUG("No domain object with UUID %s", uuidstr);
> + if (!(dom = virDomainObjListFindByUUID(data->driver->domains, name))) {
> + VIR_DEBUG("No domain object with UUID %s",
> + (const char *) name);
No. Please s/name/uuid/ as storing UUID into variable named @name is
insane. Or do we want to confuse an enemy? :) I know you haven't
introduced it, but since you are touching the body anyway, it's worth
changing the parameter name as well.
> return;
> }
>
> @@ -854,7 +843,7 @@ virQEMUCloseCallbacksRunOne(void *payload,
> if (dom)
> virObjectUnlock(dom);
>
> - virHashRemoveEntry(data->list, uuidstr);
> + virHashRemoveEntry(data->list, name);
> }
>
ACK if you change the variable name.
Michal
More information about the libvir-list
mailing list