On Tue, Nov 03, 2009 at 02:50:03PM -0500, Daniel P. Berrange wrote:
> Add reference counting on the virDomainObjPtr objects. With the
> forthcoming asynchronous QEMU monitor, it will be neccessary to
> release the lock on virDomainObjPtr while waiting for a monitor
> command response. It is neccessary to ensure one thread can't
> delete a virDomainObjPtr while another is waiting. By introducing
> reference counting threads can make sure objects they are using
> are not accidentally deleted while unlocked.
>
> * src/conf/domain_conf.h, src/conf/domain_conf.c: Add
> virDomainObjRef/Unref APIs, remove virDomainObjFree
> * src/openvz/openvz_conf.c: replace call to virDomainObjFree
> with virDomainObjUnref
> ---
> src/conf/domain_conf.c | 32 ++++++++++++++++++++++++++++----
> src/conf/domain_conf.h | 5 ++++-
> src/libvirt_private.syms | 3 ++-
> src/openvz/openvz_conf.c | 6 +++++-
> 4 files changed, 39 insertions(+), 7 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index a9c8573..79bb274 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -219,7 +219,9 @@ int virDomainObjListInit(virDomainObjListPtr doms)
> static void virDomainObjListDeallocator(void *payload, const char *name ATTRIBUTE_UNUSED)
> {
> virDomainObjPtr obj = payload;
> - virDomainObjFree(obj);
> + virDomainObjLock(obj);
> + if (!virDomainObjUnref(obj))
> + virDomainObjUnlock(obj);
> }
>
> void virDomainObjListDeinit(virDomainObjListPtr doms)
> @@ -582,11 +584,12 @@ void virDomainDefFree(virDomainDefPtr def)
>
> #ifndef PROXY
>
> -void virDomainObjFree(virDomainObjPtr dom)
> +static void virDomainObjFree(virDomainObjPtr dom)
> {
> if (!dom)
> return;
>
> + VIR_DEBUG("obj=%p", dom);
> virDomainDefFree(dom->def);
> virDomainDefFree(dom->newDef);
>
> @@ -602,6 +605,25 @@ void virDomainObjFree(virDomainObjPtr dom)
> VIR_FREE(dom);
> }
>
> +void virDomainObjRef(virDomainObjPtr dom)
> +{
> + dom->refs++;
> + VIR_DEBUG("obj=%p refs=%d", dom, dom->refs);
> +}
> +
> +
> +int virDomainObjUnref(virDomainObjPtr dom)
> +{
> + dom->refs--;
> + VIR_DEBUG("obj=%p refs=%d", dom, dom->refs);
> + if (dom->refs == 0) {
> + virDomainObjUnlock(dom);
> + virDomainObjFree(dom);
> + return 1;
> + }
> + return 0;
> +}
> +
> static virDomainObjPtr virDomainObjNew(virConnectPtr conn,
> virCapsPtr caps)
> {
> @@ -633,7 +655,9 @@ static virDomainObjPtr virDomainObjNew(virConnectPtr conn,
> domain->state = VIR_DOMAIN_SHUTOFF;
> domain->monitorWatch = -1;
> domain->monitor = -1;
> + domain->refs = 1;
>
> + VIR_DEBUG("obj=%p", domain);
> return domain;
> }
>
> @@ -3305,7 +3329,7 @@ static virDomainObjPtr virDomainObjParseXML(virConnectPtr conn,
> error:
> VIR_FREE(nodes);
> virDomainChrDefFree(obj->monitor_chr);
> - virDomainObjFree(obj);
> + virDomainObjUnref(obj);
> return NULL;
> }
>
> @@ -4846,7 +4870,7 @@ static virDomainObjPtr virDomainLoadStatus(virConnectPtr conn,
> return obj;
>
> error:
> - virDomainObjFree(obj);
> + virDomainObjUnref(obj);
> VIR_FREE(statusFile);
> return NULL;
> }
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 8599ee7..675a49b 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -621,6 +621,7 @@ typedef struct _virDomainObj virDomainObj;
> typedef virDomainObj *virDomainObjPtr;
> struct _virDomainObj {
> virMutex lock;
> + int refs;
>
> int monitor;
> virDomainChrDefPtr monitor_chr;
> @@ -678,7 +679,9 @@ void virDomainVideoDefFree(virDomainVideoDefPtr def);
> void virDomainHostdevDefFree(virDomainHostdevDefPtr def);
> void virDomainDeviceDefFree(virDomainDeviceDefPtr def);
> void virDomainDefFree(virDomainDefPtr vm);
> -void virDomainObjFree(virDomainObjPtr vm);
> +void virDomainObjRef(virDomainObjPtr vm);
> +/* Returns 1 if the object was freed, 0 if more refs exist */
> +int virDomainObjUnref(virDomainObjPtr vm);
>
> virDomainObjPtr virDomainAssignDef(virConnectPtr conn,
> virCapsPtr caps,
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 600dfee..52f1a05 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -123,7 +123,6 @@ virDomainLifecycleTypeToString;
> virDomainLoadAllConfigs;
> virDomainNetDefFree;
> virDomainNetTypeToString;
> -virDomainObjFree;
> virDomainRemoveInactive;
> virDomainSaveXML;
> virDomainSaveConfig;
> @@ -151,6 +150,8 @@ virDomainObjListGetActiveIDs;
> virDomainObjListNumOfDomains;
> virDomainObjListInit;
> virDomainObjListDeinit;
> +virDomainObjRef;
> +virDomainObjUnref;
>
>
> # domain_event.h
> diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
> index c928afb..c2a39d0 100644
> --- a/src/openvz/openvz_conf.c
> +++ b/src/openvz/openvz_conf.c
> @@ -463,6 +463,8 @@ int openvzLoadDomains(struct openvz_driver *driver) {
> goto cleanup;
> }
>
> + virDomainObjLock(dom);
> +
> if (VIR_ALLOC(dom->def) < 0)
> goto no_memory;
>
> @@ -471,6 +473,7 @@ int openvzLoadDomains(struct openvz_driver *driver) {
> else
> dom->state = VIR_DOMAIN_RUNNING;
>
> + dom->refs = 1;
> dom->pid = veid;
> dom->def->id = dom->state == VIR_DOMAIN_SHUTOFF ? -1 : veid;
>
> @@ -513,6 +516,7 @@ int openvzLoadDomains(struct openvz_driver *driver) {
> if (virHashAddEntry(driver->domains.objs, uuidstr, dom) < 0)
> goto no_memory;
>
> + virDomainObjUnlock(dom);
> dom = NULL;
> }
>
> @@ -525,7 +529,7 @@ int openvzLoadDomains(struct openvz_driver *driver) {
>
> cleanup:
> fclose(fp);
> - virDomainObjFree(dom);
> + virDomainObjUnref(dom);
> return -1;
> }
>
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list