[libvirt] [PATCH 0/7] Share the VNC port allocation tracking code
Both QEMU and libxl have the same code for tracking auto-allocated VNC ports. Move this code out into src/util/virportallocator.[c,h]. While doing this turn it into a virObjectLockable so that it becomes a ref-counted object with self-contained locking. This means it no longer needs to be protected by the QEMU/libxl global driver locks. As part of this series the virObject APIs are enhanced to allow arbitrarily deep inheritance (instead of fixed 1 level), and a new virObjectLockable base class is introduced which extends virObject to provide a mutex. This is a very common need so many existing classes are updated to use it.
From: "Daniel P. Berrange" <berrange@redhat.com> Currently all classes must directly inherit from virObject. This allows for arbitrarily deep hierarchy. There's not much too this aside from chaining up the 'dispose' handlers from each class & providing APIs to check types. --- src/conf/domain_conf.c | 3 +- src/datatypes.c | 3 +- src/libvirt_private.syms | 2 ++ src/lxc/lxc_monitor.c | 3 +- src/qemu/qemu_agent.c | 3 +- src/qemu/qemu_capabilities.c | 3 +- src/qemu/qemu_monitor.c | 7 ++-- src/rpc/virkeepalive.c | 3 +- src/rpc/virnetclient.c | 3 +- src/rpc/virnetclientprogram.c | 3 +- src/rpc/virnetclientstream.c | 3 +- src/rpc/virnetsaslcontext.c | 6 ++-- src/rpc/virnetserver.c | 3 +- src/rpc/virnetserverclient.c | 3 +- src/rpc/virnetserverprogram.c | 3 +- src/rpc/virnetserverservice.c | 3 +- src/rpc/virnetsocket.c | 3 +- src/rpc/virnetsshsession.c | 3 +- src/rpc/virnettlscontext.c | 6 ++-- src/util/virdnsmasq.c | 3 +- src/util/virobject.c | 84 ++++++++++++++++++++++++++++++++++++++++--- src/util/virobject.h | 9 ++++- 22 files changed, 134 insertions(+), 28 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b3a35b7..38dc334 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -695,7 +695,8 @@ static void virDomainObjDispose(void *obj); static int virDomainObjOnceInit(void) { - if (!(virDomainObjClass = virClassNew("virDomainObj", + if (!(virDomainObjClass = virClassNew(virClassForObject(), + "virDomainObj", sizeof(virDomainObj), virDomainObjDispose))) return -1; diff --git a/src/datatypes.c b/src/datatypes.c index 068233c..b04e100 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -64,7 +64,8 @@ static int virDataTypesOnceInit(void) { #define DECLARE_CLASS(basename) \ - if (!(basename ## Class = virClassNew(#basename, \ + if (!(basename ## Class = virClassNew(virClassForObject(), \ + #basename, \ sizeof(basename), \ basename ## Dispose))) \ return -1; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d079cc9..092cb59 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1767,6 +1767,8 @@ virNodeSuspendGetTargetMask; # virobject.h +virClassForObject; +virClassIsDerivedFrom; virClassName; virClassNew; virObjectFreeCallback; diff --git a/src/lxc/lxc_monitor.c b/src/lxc/lxc_monitor.c index 6971bcb..f697e09 100644 --- a/src/lxc/lxc_monitor.c +++ b/src/lxc/lxc_monitor.c @@ -50,7 +50,8 @@ static void virLXCMonitorDispose(void *obj); static int virLXCMonitorOnceInit(void) { - if (!(virLXCMonitorClass = virClassNew("virLXCMonitor", + if (!(virLXCMonitorClass = virClassNew(virClassForObject(), + "virLXCMonitor", sizeof(virLXCMonitor), virLXCMonitorDispose))) return -1; diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index bb421bd..db4a0bc 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -121,7 +121,8 @@ static void qemuAgentDispose(void *obj); static int qemuAgentOnceInit(void) { - if (!(qemuAgentClass = virClassNew("qemuAgent", + if (!(qemuAgentClass = virClassNew(virClassForObject(), + "qemuAgent", sizeof(qemuAgent), qemuAgentDispose))) return -1; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 320d8c8..29b5066 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -241,7 +241,8 @@ static void qemuCapsDispose(void *obj); static int qemuCapsOnceInit(void) { - if (!(qemuCapsClass = virClassNew("qemuCaps", + if (!(qemuCapsClass = virClassNew(virClassForObject(), + "qemuCaps", sizeof(qemuCaps), qemuCapsDispose))) return -1; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index cb5a3e2..d6176c7 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -86,9 +86,10 @@ static void qemuMonitorDispose(void *obj); static int qemuMonitorOnceInit(void) { - if (!(qemuMonitorClass = virClassNew("qemuMonitor", - sizeof(qemuMonitor), - qemuMonitorDispose))) + if (!(qemuMonitorClass = virClassNew(virClassForObject(), + "qemuMonitor", + sizeof(qemuMonitor), + qemuMonitorDispose))) return -1; return 0; diff --git a/src/rpc/virkeepalive.c b/src/rpc/virkeepalive.c index a8ceff5..04962d4 100644 --- a/src/rpc/virkeepalive.c +++ b/src/rpc/virkeepalive.c @@ -58,7 +58,8 @@ static void virKeepAliveDispose(void *obj); static int virKeepAliveOnceInit(void) { - if (!(virKeepAliveClass = virClassNew("virKeepAlive", + if (!(virKeepAliveClass = virClassNew(virClassForObject(), + "virKeepAlive", sizeof(virKeepAlive), virKeepAliveDispose))) return -1; diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index a79b79b..f281548 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -117,7 +117,8 @@ static void virNetClientDispose(void *obj); static int virNetClientOnceInit(void) { - if (!(virNetClientClass = virClassNew("virNetClient", + if (!(virNetClientClass = virClassNew(virClassForObject(), + "virNetClient", sizeof(virNetClient), virNetClientDispose))) return -1; diff --git a/src/rpc/virnetclientprogram.c b/src/rpc/virnetclientprogram.c index 9410cff..2e6e4f6 100644 --- a/src/rpc/virnetclientprogram.c +++ b/src/rpc/virnetclientprogram.c @@ -52,7 +52,8 @@ static void virNetClientProgramDispose(void *obj); static int virNetClientProgramOnceInit(void) { - if (!(virNetClientProgramClass = virClassNew("virNetClientProgram", + if (!(virNetClientProgramClass = virClassNew(virClassForObject(), + "virNetClientProgram", sizeof(virNetClientProgram), virNetClientProgramDispose))) return -1; diff --git a/src/rpc/virnetclientstream.c b/src/rpc/virnetclientstream.c index 15ed91a..e1ee30e 100644 --- a/src/rpc/virnetclientstream.c +++ b/src/rpc/virnetclientstream.c @@ -68,7 +68,8 @@ static void virNetClientStreamDispose(void *obj); static int virNetClientStreamOnceInit(void) { - if (!(virNetClientStreamClass = virClassNew("virNetClientStream", + if (!(virNetClientStreamClass = virClassNew(virClassForObject(), + "virNetClientStream", sizeof(virNetClientStream), virNetClientStreamDispose))) return -1; diff --git a/src/rpc/virnetsaslcontext.c b/src/rpc/virnetsaslcontext.c index cbf7261..41a69d1 100644 --- a/src/rpc/virnetsaslcontext.c +++ b/src/rpc/virnetsaslcontext.c @@ -55,12 +55,14 @@ static void virNetSASLSessionDispose(void *obj); static int virNetSASLContextOnceInit(void) { - if (!(virNetSASLContextClass = virClassNew("virNetSASLContext", + if (!(virNetSASLContextClass = virClassNew(virClassForObject(), + "virNetSASLContext", sizeof(virNetSASLContext), virNetSASLContextDispose))) return -1; - if (!(virNetSASLSessionClass = virClassNew("virNetSASLSession", + if (!(virNetSASLSessionClass = virClassNew(virClassForObject(), + "virNetSASLSession", sizeof(virNetSASLSession), virNetSASLSessionDispose))) return -1; diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index b9df71b..03efbb8 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -119,7 +119,8 @@ static void virNetServerDispose(void *obj); static int virNetServerOnceInit(void) { - if (!(virNetServerClass = virClassNew("virNetServer", + if (!(virNetServerClass = virClassNew(virClassForObject(), + "virNetServer", sizeof(virNetServer), virNetServerDispose))) return -1; diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index bf23d24..ce8bd6d 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -112,7 +112,8 @@ static void virNetServerClientDispose(void *obj); static int virNetServerClientOnceInit(void) { - if (!(virNetServerClientClass = virClassNew("virNetServerClient", + if (!(virNetServerClientClass = virClassNew(virClassForObject(), + "virNetServerClient", sizeof(virNetServerClient), virNetServerClientDispose))) return -1; diff --git a/src/rpc/virnetserverprogram.c b/src/rpc/virnetserverprogram.c index 06b6325..414b978 100644 --- a/src/rpc/virnetserverprogram.c +++ b/src/rpc/virnetserverprogram.c @@ -49,7 +49,8 @@ static void virNetServerProgramDispose(void *obj); static int virNetServerProgramOnceInit(void) { - if (!(virNetServerProgramClass = virClassNew("virNetServerProgram", + if (!(virNetServerProgramClass = virClassNew(virClassForObject(), + "virNetServerProgram", sizeof(virNetServerProgram), virNetServerProgramDispose))) return -1; diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c index 61dd682..05fe41b 100644 --- a/src/rpc/virnetserverservice.c +++ b/src/rpc/virnetserverservice.c @@ -55,7 +55,8 @@ static void virNetServerServiceDispose(void *obj); static int virNetServerServiceOnceInit(void) { - if (!(virNetServerServiceClass = virClassNew("virNetServerService", + if (!(virNetServerServiceClass = virClassNew(virClassForObject(), + "virNetServerService", sizeof(virNetServerService), virNetServerServiceDispose))) return -1; diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index a817999..f96b47c 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -104,7 +104,8 @@ static void virNetSocketDispose(void *obj); static int virNetSocketOnceInit(void) { - if (!(virNetSocketClass = virClassNew("virNetSocket", + if (!(virNetSocketClass = virClassNew(virClassForObject(), + "virNetSocket", sizeof(virNetSocket), virNetSocketDispose))) return -1; diff --git a/src/rpc/virnetsshsession.c b/src/rpc/virnetsshsession.c index 661860f..ca7d52e 100644 --- a/src/rpc/virnetsshsession.c +++ b/src/rpc/virnetsshsession.c @@ -161,7 +161,8 @@ static virClassPtr virNetSSHSessionClass; static int virNetSSHSessionOnceInit(void) { - if (!(virNetSSHSessionClass = virClassNew("virNetSSHSession", + if (!(virNetSSHSessionClass = virClassNew(virClassForObject(), + "virNetSSHSession", sizeof(virNetSSHSession), virNetSSHSessionDispose))) return -1; diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index 56e372b..3e194f9 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -85,12 +85,14 @@ static void virNetTLSSessionDispose(void *obj); static int virNetTLSContextOnceInit(void) { - if (!(virNetTLSContextClass = virClassNew("virNetTLSContext", + if (!(virNetTLSContextClass = virClassNew(virClassForObject(), + "virNetTLSContext", sizeof(virNetTLSContext), virNetTLSContextDispose))) return -1; - if (!(virNetTLSSessionClass = virClassNew("virNetTLSSession", + if (!(virNetTLSSessionClass = virClassNew(virClassForObject(), + "virNetTLSSession", sizeof(virNetTLSSession), virNetTLSSessionDispose))) return -1; diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c index 2d0f02c..6637a89 100644 --- a/src/util/virdnsmasq.c +++ b/src/util/virdnsmasq.c @@ -628,7 +628,8 @@ dnsmasqCapsDispose(void *obj) static int dnsmasqCapsOnceInit(void) { - if (!(dnsmasqCapsClass = virClassNew("dnsmasqCaps", + if (!(dnsmasqCapsClass = virClassNew(virClassForObject(), + "dnsmasqCaps", sizeof(dnsmasqCaps), dnsmasqCapsDispose))) { return -1; diff --git a/src/util/virobject.c b/src/util/virobject.c index f51b735..5f44ab2 100644 --- a/src/util/virobject.c +++ b/src/util/virobject.c @@ -33,6 +33,8 @@ static unsigned int magicCounter = 0xCAFE0000; struct _virClass { + virClassPtr parent; + unsigned int magic; const char *name; size_t objectSize; @@ -40,9 +42,39 @@ struct _virClass { virObjectDisposeCallback dispose; }; +static virClassPtr virObjectClass; + +static int virObjectOnceInit(void) +{ + if (!(virObjectClass = virClassNew(NULL, + "virObject", + sizeof(virObject), + NULL))) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virObject); + + +/** + * virClassForObject: + * + * Returns the class instance for the base virObject type + */ +virClassPtr virClassForObject(void) +{ + if (!virObjectInitialize() < 0) + return NULL; + + return virObjectClass; +} + /** * virClassNew: + * @parent: the parent class * @name: the class name * @objectSize: total size of the object struct * @dispose: callback to run to free object fields @@ -56,15 +88,29 @@ struct _virClass { * * Returns a new class instance */ -virClassPtr virClassNew(const char *name, +virClassPtr virClassNew(virClassPtr parent, + const char *name, size_t objectSize, virObjectDisposeCallback dispose) { virClassPtr klass; + if (parent == NULL && + STRNEQ(name, "virObject")) { + virReportInvalidNonNullArg(parent); + return NULL; + } else if (parent && + objectSize <= parent->objectSize) { + virReportInvalidArg(objectSize, + _("object size %zu of %s is smaller than parent class %zu"), + objectSize, name, parent->objectSize); + return NULL; + } + if (VIR_ALLOC(klass) < 0) goto no_memory; + klass->parent = parent; if (!(klass->name = strdup(name))) goto no_memory; klass->magic = virAtomicIntInc(&magicCounter); @@ -81,6 +127,27 @@ no_memory: /** + * virClassIsDerivedFrom: + * @klass: the klass to check + * @parent: the possible parent class + * + * Determine if @klass is derived from @parent + * + * Return true if @klass is derived from @parent, false otherwise + */ +bool virClassIsDerivedFrom(virClassPtr klass, + virClassPtr parent) +{ + while (klass) { + if (klass->magic == parent->magic) + return true; + klass = klass->parent; + } + return false; +} + + +/** * virObjectNew: * @klass: the klass of object to create * @@ -135,8 +202,14 @@ bool virObjectUnref(void *anyobj) PROBE(OBJECT_UNREF, "obj=%p", obj); if (lastRef) { PROBE(OBJECT_DISPOSE, "obj=%p", obj); - if (obj->klass->dispose) - obj->klass->dispose(obj); + virClassPtr klass = obj->klass; + while (klass) { + if (klass->dispose) + klass->dispose(obj); + klass = klass->parent; + } + + virMutexDestroy(&obj->lock); /* Clear & poison object */ memset(obj, 0, obj->klass->objectSize); @@ -184,7 +257,10 @@ bool virObjectIsClass(void *anyobj, virClassPtr klass) { virObjectPtr obj = anyobj; - return obj != NULL && (obj->magic == klass->magic) && (obj->klass == klass); + if (!obj) + return false; + + return virClassIsDerivedFrom(obj->klass, klass); } diff --git a/src/util/virobject.h b/src/util/virobject.h index b2f7612..afeb4f5 100644 --- a/src/util/virobject.h +++ b/src/util/virobject.h @@ -38,7 +38,10 @@ struct _virObject { virClassPtr klass; }; -virClassPtr virClassNew(const char *name, +virClassPtr virClassForObject(void); + +virClassPtr virClassNew(virClassPtr parent, + const char *name, size_t objectSize, virObjectDisposeCallback dispose) ATTRIBUTE_NONNULL(1); @@ -46,6 +49,10 @@ virClassPtr virClassNew(const char *name, const char *virClassName(virClassPtr klass) ATTRIBUTE_NONNULL(1); +bool virClassIsDerivedFrom(virClassPtr klass, + virClassPtr parent) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + void *virObjectNew(virClassPtr klass) ATTRIBUTE_NONNULL(1); bool virObjectUnref(void *obj); -- 1.8.0.1
On 01/11/2013 05:13 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Currently all classes must directly inherit from virObject. This allows for arbitrarily deep hierarchy. There's not much too this aside from chaining up the 'dispose' handlers from
s/too/to/
each class & providing APIs to check types. ---
-virClassPtr virClassNew(const char *name, +virClassPtr virClassNew(virClassPtr parent, + const char *name, size_t objectSize, virObjectDisposeCallback dispose) { virClassPtr klass;
+ if (parent == NULL && + STRNEQ(name, "virObject")) { + virReportInvalidNonNullArg(parent); + return NULL;
Would it have been any easier to let callers pass NULL when they want to have virObject as the default parent, instead of making all callers have to use virClassForObject()? Then again, you've already touched everyone, so don't change the bikeshed color again. ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Fri, Jan 11, 2013 at 04:17:20PM -0700, Eric Blake wrote:
On 01/11/2013 05:13 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Currently all classes must directly inherit from virObject. This allows for arbitrarily deep hierarchy. There's not much too this aside from chaining up the 'dispose' handlers from
s/too/to/
each class & providing APIs to check types. ---
-virClassPtr virClassNew(const char *name, +virClassPtr virClassNew(virClassPtr parent, + const char *name, size_t objectSize, virObjectDisposeCallback dispose) { virClassPtr klass;
+ if (parent == NULL && + STRNEQ(name, "virObject")) { + virReportInvalidNonNullArg(parent); + return NULL;
Would it have been any easier to let callers pass NULL when they want to have virObject as the default parent, instead of making all callers have to use virClassForObject()? Then again, you've already touched everyone, so don't change the bikeshed color again.
Well 'NULL' needs to mean 'no parent' so we can initialize virObject, so it felt wrong to also have it mean 'virObject as a parent' for every other callers. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
From: "Daniel P. Berrange" <berrange@redhat.com> A great many virObject instances require a mutex, so introduce a convenient class for this which provides a mutex. This avoids repeating the tedious init/destroy code --- src/libvirt_private.syms | 4 +++ src/util/virobject.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++-- src/util/virobject.h | 20 +++++++++++ 3 files changed, 109 insertions(+), 2 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 092cb59..f02aee4 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1768,13 +1768,17 @@ virNodeSuspendGetTargetMask; # virobject.h virClassForObject; +virClassForObjectLockable; virClassIsDerivedFrom; virClassName; virClassNew; virObjectFreeCallback; virObjectIsClass; +virObjectLock; +virObjectLockableNew; virObjectNew; virObjectRef; +virObjectUnlock; virObjectUnref; diff --git a/src/util/virobject.c b/src/util/virobject.c index 5f44ab2..d5bef11 100644 --- a/src/util/virobject.c +++ b/src/util/virobject.c @@ -43,6 +43,9 @@ struct _virClass { }; static virClassPtr virObjectClass; +static virClassPtr virObjectLockableClass; + +static void virObjectLockableDispose(void *anyobj); static int virObjectOnceInit(void) { @@ -52,6 +55,12 @@ static int virObjectOnceInit(void) NULL))) return -1; + if (!(virObjectLockableClass = virClassNew(virObjectClass, + "virObjectLockable", + sizeof(virObjectLockable), + virObjectLockableDispose))) + return -1; + return 0; } @@ -73,6 +82,20 @@ virClassPtr virClassForObject(void) /** + * virClassForObjectLockable: + * + * Returns the class instance for the virObjectLockable type + */ +virClassPtr virClassForObjectLockable(void) +{ + if (!virObjectInitialize() < 0) + return NULL; + + return virObjectLockableClass; +} + + +/** * virClassNew: * @parent: the parent class * @name: the class name @@ -180,6 +203,38 @@ void *virObjectNew(virClassPtr klass) } +void *virObjectLockableNew(virClassPtr klass) +{ + virObjectLockablePtr obj; + + if (!virClassIsDerivedFrom(klass, virClassForObjectLockable())) { + virReportInvalidArg(klass, + _("Class %s must derive from virObjectLockable"), + virClassName(klass)); + return NULL; + } + + if (!(obj = virObjectNew(klass))) + return NULL; + + if (virMutexInit(&obj->lock) < 0) { + virReportSystemError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to initialize mutex")); + virObjectUnref(obj); + return NULL; + } + + return obj; +} + + +static void virObjectLockableDispose(void *anyobj) +{ + virObjectLockablePtr obj = anyobj; + + virMutexDestroy(&obj->lock); +} + /** * virObjectUnref: * @anyobj: any instance of virObjectPtr @@ -209,8 +264,6 @@ bool virObjectUnref(void *anyobj) klass = klass->parent; } - virMutexDestroy(&obj->lock); - /* Clear & poison object */ memset(obj, 0, obj->klass->objectSize); obj->magic = 0xDEADBEEF; @@ -244,6 +297,36 @@ void *virObjectRef(void *anyobj) /** + * virObjectLock: + * @anyobj: any instance of virObjectLockablePtr + * + * Acquire a lock on @anyobj. The lock must be + * released by virObjectUnlock. + */ +void virObjectLock(void *anyobj) +{ + virObjectLockablePtr obj = anyobj; + + virMutexLock(&obj->lock); +} + + +/** + * virObjectUnlock: + * @anyobj: any instance of virObjectLockablePtr + * + * Release a lock on @anyobj. The lock must have been + * acquired by virObjectLock. + */ +void virObjectUnlock(void *anyobj) +{ + virObjectLockablePtr obj = anyobj; + + virMutexUnlock(&obj->lock); +} + + +/** * virObjectIsClass: * @anyobj: any instance of virObjectPtr * @klass: the class to check diff --git a/src/util/virobject.h b/src/util/virobject.h index afeb4f5..bb72a25 100644 --- a/src/util/virobject.h +++ b/src/util/virobject.h @@ -23,6 +23,7 @@ # define __VIR_OBJECT_H__ # include "internal.h" +# include "virthread.h" typedef struct _virClass virClass; typedef virClass *virClassPtr; @@ -30,6 +31,9 @@ typedef virClass *virClassPtr; typedef struct _virObject virObject; typedef virObject *virObjectPtr; +typedef struct _virObjectLockable virObjectLockable; +typedef virObjectLockable *virObjectLockablePtr; + typedef void (*virObjectDisposeCallback)(void *obj); struct _virObject { @@ -38,7 +42,14 @@ struct _virObject { virClassPtr klass; }; +struct _virObjectLockable { + virObject parent; + virMutex lock; +}; + + virClassPtr virClassForObject(void); +virClassPtr virClassForObjectLockable(void); virClassPtr virClassNew(virClassPtr parent, const char *name, @@ -64,4 +75,13 @@ bool virObjectIsClass(void *obj, void virObjectFreeCallback(void *opaque); +void *virObjectLockableNew(virClassPtr klass) + ATTRIBUTE_NONNULL(1); + +void virObjectLock(void *lockableobj) + ATTRIBUTE_NONNULL(1); +void virObjectUnlock(void *lockableobj) + ATTRIBUTE_NONNULL(1); + + #endif /* __VIR_OBJECT_H */ -- 1.8.0.1
On 01/11/2013 05:13 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
A great many virObject instances require a mutex, so introduce a convenient class for this which provides a mutex. This avoids repeating the tedious init/destroy code --- src/libvirt_private.syms | 4 +++ src/util/virobject.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++-- src/util/virobject.h | 20 +++++++++++ 3 files changed, 109 insertions(+), 2 deletions(-)
+void *virObjectLockableNew(virClassPtr klass) +{ + virObjectLockablePtr obj; + + if (!virClassIsDerivedFrom(klass, virClassForObjectLockable())) { + virReportInvalidArg(klass, + _("Class %s must derive from virObjectLockable"), + virClassName(klass)); + return NULL; + } + + if (!(obj = virObjectNew(klass))) + return NULL; + + if (virMutexInit(&obj->lock) < 0) { + virReportSystemError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to initialize mutex")); + virObjectUnref(obj); + return NULL; + }
This creates the new object locked...
+ + return obj; +} + + +static void virObjectLockableDispose(void *anyobj) +{ + virObjectLockablePtr obj = anyobj; + + virMutexDestroy(&obj->lock);
...but this doesn't unlock anything. You may want to document that the user is required to unlock the object before losing the last reference.
+ * virObjectLock: + * @anyobj: any instance of virObjectLockablePtr + * + * Acquire a lock on @anyobj. The lock must be + * released by virObjectUnlock. + */ +void virObjectLock(void *anyobj) +{ + virObjectLockablePtr obj = anyobj;
Is it worth a sanity check that anyobj is actually an object of the right class, before we blindly dereference something wrong due to a coding error?
+void virObjectUnlock(void *anyobj) +{ + virObjectLockablePtr obj = anyobj; +
And again, would a sanity check help? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Fri, Jan 11, 2013 at 04:23:23PM -0700, Eric Blake wrote:
On 01/11/2013 05:13 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
A great many virObject instances require a mutex, so introduce a convenient class for this which provides a mutex. This avoids repeating the tedious init/destroy code --- src/libvirt_private.syms | 4 +++ src/util/virobject.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++-- src/util/virobject.h | 20 +++++++++++ 3 files changed, 109 insertions(+), 2 deletions(-)
+void *virObjectLockableNew(virClassPtr klass) +{ + virObjectLockablePtr obj; + + if (!virClassIsDerivedFrom(klass, virClassForObjectLockable())) { + virReportInvalidArg(klass, + _("Class %s must derive from virObjectLockable"), + virClassName(klass)); + return NULL; + } + + if (!(obj = virObjectNew(klass))) + return NULL; + + if (virMutexInit(&obj->lock) < 0) { + virReportSystemError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to initialize mutex")); + virObjectUnref(obj); + return NULL; + }
This creates the new object locked...
No, mutexes are initially unlocked.
+ + return obj; +} + + +static void virObjectLockableDispose(void *anyobj) +{ + virObjectLockablePtr obj = anyobj; + + virMutexDestroy(&obj->lock);
...but this doesn't unlock anything. You may want to document that the user is required to unlock the object before losing the last reference.
So this isn't an issue, though we ought to document locking rules I guess.
+ * @anyobj: any instance of virObjectLockablePtr + * + * Acquire a lock on @anyobj. The lock must be + * released by virObjectUnlock. + */ +void virObjectLock(void *anyobj) +{ + virObjectLockablePtr obj = anyobj;
Is it worth a sanity check that anyobj is actually an object of the right class, before we blindly dereference something wrong due to a coding error?
+void virObjectUnlock(void *anyobj) +{ + virObjectLockablePtr obj = anyobj; +
And again, would a sanity check help?
Possibily, though what action should we take. These methods really need to be void, because it is not practical to check return values everywhere. Meanwhile we don't like to abort(). So that leaves the possibility of a VIR_WARN ? Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 01/14/2013 03:26 AM, Daniel P. Berrange wrote:
...but this doesn't unlock anything. You may want to document that the user is required to unlock the object before losing the last reference.
So this isn't an issue, though we ought to document locking rules I guess.
Thanks for correcting me - you are right that there is no issue except missing documentation.
Is it worth a sanity check that anyobj is actually an object of the right class, before we blindly dereference something wrong due to a coding error?
Possibily, though what action should we take. These methods really need to be void, because it is not practical to check return values everywhere. Meanwhile we don't like to abort(). So that leaves the possibility of a VIR_WARN ?
Yep, a VIR_WARN in the logs is about the best we can do; but thankfully the problem is most likely to hit during early development patches, when someone is more likely to be paying attention to the logs. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> The virDomainObj, qemuAgent, qemuMonitor, lxcMonitor classes all require a mutex, so can be switched to use virObjectLockable --- src/conf/domain_conf.c | 66 ++++------ src/conf/domain_conf.h | 7 +- src/libvirt_private.syms | 2 - src/libxl/libxl_driver.c | 92 ++++++------- src/lxc/lxc_driver.c | 70 +++++----- src/lxc/lxc_monitor.c | 32 +---- src/lxc/lxc_process.c | 14 +- src/nwfilter/nwfilter_gentech_driver.c | 4 +- src/openvz/openvz_conf.c | 2 +- src/openvz/openvz_driver.c | 46 +++---- src/parallels/parallels_driver.c | 28 ++-- src/qemu/THREADS.txt | 8 +- src/qemu/qemu_agent.c | 49 ++----- src/qemu/qemu_agent.h | 3 - src/qemu/qemu_capabilities.c | 4 +- src/qemu/qemu_conf.c | 2 +- src/qemu/qemu_domain.c | 32 ++--- src/qemu/qemu_driver.c | 232 ++++++++++++++++----------------- src/qemu/qemu_migration.c | 12 +- src/qemu/qemu_monitor.c | 60 +++------ src/qemu/qemu_monitor.h | 3 - src/qemu/qemu_process.c | 106 +++++++-------- src/test/test_driver.c | 74 +++++------ src/uml/uml_driver.c | 68 +++++----- src/vmware/vmware_conf.c | 2 +- src/vmware/vmware_driver.c | 38 +++--- tests/qemumonitortestutils.c | 4 +- 27 files changed, 481 insertions(+), 579 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 38dc334..494008b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -695,7 +695,7 @@ static void virDomainObjDispose(void *obj); static int virDomainObjOnceInit(void) { - if (!(virDomainObjClass = virClassNew(virClassForObject(), + if (!(virDomainObjClass = virClassNew(virClassForObjectLockable(), "virDomainObj", sizeof(virDomainObj), virDomainObjDispose))) @@ -796,11 +796,11 @@ static int virDomainObjListSearchID(const void *payload, const int *id = data; int want = 0; - virDomainObjLock(obj); + virObjectLock(obj); if (virDomainObjIsActive(obj) && obj->def->id == *id) want = 1; - virDomainObjUnlock(obj); + virObjectUnlock(obj); return want; } @@ -810,7 +810,7 @@ virDomainObjPtr virDomainFindByID(const virDomainObjListPtr doms, virDomainObjPtr obj; obj = virHashSearch(doms->objs, virDomainObjListSearchID, &id); if (obj) - virDomainObjLock(obj); + virObjectLock(obj); return obj; } @@ -825,7 +825,7 @@ virDomainObjPtr virDomainFindByUUID(const virDomainObjListPtr doms, obj = virHashLookup(doms->objs, uuidstr); if (obj) - virDomainObjLock(obj); + virObjectLock(obj); return obj; } @@ -836,10 +836,10 @@ static int virDomainObjListSearchName(const void *payload, virDomainObjPtr obj = (virDomainObjPtr)payload; int want = 0; - virDomainObjLock(obj); + virObjectLock(obj); if (STREQ(obj->def->name, (const char *)data)) want = 1; - virDomainObjUnlock(obj); + virObjectUnlock(obj); return want; } @@ -849,7 +849,7 @@ virDomainObjPtr virDomainFindByName(const virDomainObjListPtr doms, virDomainObjPtr obj; obj = virHashSearch(doms->objs, virDomainObjListSearchName, name); if (obj) - virDomainObjLock(obj); + virObjectLock(obj); return obj; } @@ -1781,8 +1781,6 @@ static void virDomainObjDispose(void *obj) if (dom->privateDataFreeFunc) (dom->privateDataFreeFunc)(dom->privateData); - virMutexDestroy(&dom->lock); - virDomainSnapshotObjListFree(dom->snapshots); } @@ -1794,16 +1792,9 @@ virDomainObjPtr virDomainObjNew(virCapsPtr caps) if (virDomainObjInitialize() < 0) return NULL; - if (!(domain = virObjectNew(virDomainObjClass))) + if (!(domain = virObjectLockableNew(virDomainObjClass))) return NULL; - if (virMutexInit(&domain->lock) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot initialize mutex")); - VIR_FREE(domain); - return NULL; - } - if (caps && caps->privateDataAllocFunc) { if (!(domain->privateData = (caps->privateDataAllocFunc)())) { @@ -1816,7 +1807,7 @@ virDomainObjPtr virDomainObjNew(virCapsPtr caps) if (!(domain->snapshots = virDomainSnapshotObjListNew())) goto error; - virDomainObjLock(domain); + virObjectLock(domain); virDomainObjSetState(domain, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_UNKNOWN); @@ -1994,7 +1985,7 @@ void virDomainRemoveInactive(virDomainObjListPtr doms, char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(dom->def->uuid, uuidstr); - virDomainObjUnlock(dom); + virObjectUnlock(dom); virHashRemoveEntry(doms->objs, uuidstr); } @@ -14894,7 +14885,7 @@ int virDomainLoadAllConfigs(virCapsPtr caps, notify, opaque); if (dom) { - virDomainObjUnlock(dom); + virObjectUnlock(dom); if (!liveStatus) dom->persistent = 1; } @@ -15061,40 +15052,29 @@ virDomainObjIsDuplicate(virDomainObjListPtr doms, ret = dupVM; cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } -void virDomainObjLock(virDomainObjPtr obj) -{ - virMutexLock(&obj->lock); -} - -void virDomainObjUnlock(virDomainObjPtr obj) -{ - virMutexUnlock(&obj->lock); -} - - static void virDomainObjListCountActive(void *payload, const void *name ATTRIBUTE_UNUSED, void *data) { virDomainObjPtr obj = payload; int *count = data; - virDomainObjLock(obj); + virObjectLock(obj); if (virDomainObjIsActive(obj)) (*count)++; - virDomainObjUnlock(obj); + virObjectUnlock(obj); } static void virDomainObjListCountInactive(void *payload, const void *name ATTRIBUTE_UNUSED, void *data) { virDomainObjPtr obj = payload; int *count = data; - virDomainObjLock(obj); + virObjectLock(obj); if (!virDomainObjIsActive(obj)) (*count)++; - virDomainObjUnlock(obj); + virObjectUnlock(obj); } int virDomainObjListNumOfDomains(virDomainObjListPtr doms, int active) @@ -15117,10 +15097,10 @@ static void virDomainObjListCopyActiveIDs(void *payload, const void *name ATTRIB { virDomainObjPtr obj = payload; struct virDomainIDData *data = opaque; - virDomainObjLock(obj); + virObjectLock(obj); if (virDomainObjIsActive(obj) && data->numids < data->maxids) data->ids[data->numids++] = obj->def->id; - virDomainObjUnlock(obj); + virObjectUnlock(obj); } int virDomainObjListGetActiveIDs(virDomainObjListPtr doms, @@ -15147,14 +15127,14 @@ static void virDomainObjListCopyInactiveNames(void *payload, const void *name AT if (data->oom) return; - virDomainObjLock(obj); + virObjectLock(obj); if (!virDomainObjIsActive(obj) && data->numnames < data->maxnames) { if (!(data->names[data->numnames] = strdup(obj->def->name))) data->oom = 1; else data->numnames++; } - virDomainObjUnlock(obj); + virObjectUnlock(obj); } @@ -15847,7 +15827,7 @@ virDomainListPopulate(void *payload, if (data->error) return; - virDomainObjLock(vm); + virObjectLock(vm); /* check if the domain matches the filter */ /* filter by active state */ @@ -15920,7 +15900,7 @@ virDomainListPopulate(void *payload, data->domains[data->ndomains++] = dom; cleanup: - virDomainObjUnlock(vm); + virObjectUnlock(vm); return; } #undef MATCH diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4284caf..e85c67e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1858,9 +1858,7 @@ struct _virDomainStateReason { typedef struct _virDomainObj virDomainObj; typedef virDomainObj *virDomainObjPtr; struct _virDomainObj { - virObject object; - - virMutex lock; + virObjectLockable parent; pid_t pid; virDomainStateReason state; @@ -2159,9 +2157,6 @@ int virDomainObjIsDuplicate(virDomainObjListPtr doms, virDomainDefPtr def, unsigned int check_active); -void virDomainObjLock(virDomainObjPtr obj); -void virDomainObjUnlock(virDomainObjPtr obj); - int virDomainObjListNumOfDomains(virDomainObjListPtr doms, int active); int virDomainObjListGetActiveIDs(virDomainObjListPtr doms, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f02aee4..cbdbb32 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -479,12 +479,10 @@ virDomainObjListGetActiveIDs; virDomainObjListGetInactiveNames; virDomainObjListInit; virDomainObjListNumOfDomains; -virDomainObjLock; virDomainObjNew; virDomainObjSetDefTransient; virDomainObjSetState; virDomainObjTaint; -virDomainObjUnlock; virDomainPausedReasonTypeFromString; virDomainPausedReasonTypeToString; virDomainPciRombarModeTypeFromString; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index a956188..8017a4a 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -299,7 +299,7 @@ libxlAutostartDomain(void *payload, const void *name ATTRIBUTE_UNUSED, virDomainObjPtr vm = payload; virErrorPtr err; - virDomainObjLock(vm); + virObjectLock(vm); virResetLastError(); if (vm->autostart && !virDomainObjIsActive(vm) && @@ -311,7 +311,7 @@ libxlAutostartDomain(void *payload, const void *name ATTRIBUTE_UNUSED, } if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); } static int @@ -528,7 +528,7 @@ static void libxlEventHandler(void *data, const libxl_event *event) virDomainEventPtr dom_event = NULL; libxlDriverLock(driver); - virDomainObjLock(vm); + virObjectLock(vm); libxlDriverUnlock(driver); if (event->type == LIBXL_EVENT_TYPE_DOMAIN_SHUTDOWN) { @@ -566,7 +566,7 @@ static void libxlEventHandler(void *data, const libxl_event *event) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (dom_event) { libxlDriverLock(driver); libxlDomainEventQueue(driver, dom_event); @@ -863,7 +863,7 @@ libxlReconnectDomain(void *payload, int len; uint8_t *data = NULL; - virDomainObjLock(vm); + virObjectLock(vm); /* Does domain still exist? */ rc = libxl_domain_info(driver->ctx, &d_info, vm->def->id); @@ -892,7 +892,7 @@ libxlReconnectDomain(void *payload, /* Recreate domain death et. al. events */ libxlCreateDomEvents(vm); - virDomainObjUnlock(vm); + virObjectUnlock(vm); return; out: @@ -900,7 +900,7 @@ out: if (!vm->persistent) virDomainRemoveInactive(&driver->domains, vm); else - virDomainObjUnlock(vm); + virObjectUnlock(vm); } static void @@ -1312,7 +1312,7 @@ libxlDomainCreateXML(virConnectPtr conn, const char *xml, cleanup: virDomainDefFree(def); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); libxlDriverUnlock(driver); return dom; } @@ -1339,7 +1339,7 @@ libxlDomainLookupByID(virConnectPtr conn, int id) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return dom; } @@ -1365,7 +1365,7 @@ libxlDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return dom; } @@ -1391,7 +1391,7 @@ libxlDomainLookupByName(virConnectPtr conn, const char *name) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return dom; } @@ -1443,7 +1443,7 @@ libxlDomainSuspend(virDomainPtr dom) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) { libxlDriverLock(driver); libxlDomainEventQueue(driver, event); @@ -1503,7 +1503,7 @@ libxlDomainResume(virDomainPtr dom) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) { libxlDriverLock(driver); libxlDomainEventQueue(driver, event); @@ -1553,7 +1553,7 @@ libxlDomainShutdownFlags(virDomainPtr dom, unsigned int flags) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); libxlDriverUnlock(driver); return ret; } @@ -1602,7 +1602,7 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); libxlDriverUnlock(driver); return ret; } @@ -1652,7 +1652,7 @@ libxlDomainDestroyFlags(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) libxlDomainEventQueue(driver, event); libxlDriverUnlock(driver); @@ -1688,7 +1688,7 @@ libxlDomainGetOSType(virDomainPtr dom) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return type; } @@ -1711,7 +1711,7 @@ libxlDomainGetMaxMemory(virDomainPtr dom) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -1825,7 +1825,7 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -1880,7 +1880,7 @@ libxlDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -1911,7 +1911,7 @@ libxlDomainGetState(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -2033,7 +2033,7 @@ libxlDomainSaveFlags(virDomainPtr dom, const char *to, const char *dxml, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); libxlDriverUnlock(driver); return ret; } @@ -2087,7 +2087,7 @@ cleanup: virReportSystemError(errno, "%s", _("cannot close file")); virDomainDefFree(def); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); libxlDriverUnlock(driver); return ret; } @@ -2183,7 +2183,7 @@ cleanup_unpause: } cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) { libxlDriverLock(driver); libxlDomainEventQueue(driver, event); @@ -2232,7 +2232,7 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int flags) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); libxlDriverUnlock(driver); VIR_FREE(name); return ret; @@ -2247,7 +2247,7 @@ libxlDomainManagedSaveLoad(void *payload, libxlDriverPrivatePtr driver = opaque; char *name; - virDomainObjLock(vm); + virObjectLock(vm); if (!(name = libxlDomainManagedSavePath(driver, vm))) goto cleanup; @@ -2255,7 +2255,7 @@ libxlDomainManagedSaveLoad(void *payload, vm->hasManagedSave = virFileExists(name); cleanup: - virDomainObjUnlock(vm); + virObjectUnlock(vm); VIR_FREE(name); } @@ -2282,7 +2282,7 @@ libxlDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); libxlDriverUnlock(driver); return ret; } @@ -2317,7 +2317,7 @@ libxlDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags) cleanup: VIR_FREE(name); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); libxlDriverUnlock(driver); return ret; } @@ -2452,7 +2452,7 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, cleanup: VIR_FREE(bitmask); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -2518,7 +2518,7 @@ libxlDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -2581,7 +2581,7 @@ libxlDomainPinVcpu(virDomainPtr dom, unsigned int vcpu, unsigned char *cpumap, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -2649,7 +2649,7 @@ libxlDomainGetVcpus(virDomainPtr dom, virVcpuInfoPtr info, int maxinfo, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -2676,7 +2676,7 @@ libxlDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -2827,7 +2827,7 @@ libxlDomainCreateWithFlags(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); libxlDriverUnlock(driver); return ret; } @@ -2882,7 +2882,7 @@ libxlDomainDefineXML(virConnectPtr conn, const char *xml) cleanup: virDomainDefFree(def); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) libxlDomainEventQueue(driver, event); libxlDriverUnlock(driver); @@ -2958,7 +2958,7 @@ libxlDomainUndefineFlags(virDomainPtr dom, cleanup: VIR_FREE(name); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) libxlDomainEventQueue(driver, event); libxlDriverUnlock(driver); @@ -3438,7 +3438,7 @@ cleanup: virDomainDefFree(vmdef); virDomainDeviceDefFree(dev); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); libxlDriverUnlock(driver); return ret; } @@ -3558,7 +3558,7 @@ libxlDomainGetAutostart(virDomainPtr dom, int *autostart) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -3626,7 +3626,7 @@ cleanup: VIR_FREE(configFile); VIR_FREE(autostartLink); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); libxlDriverUnlock(driver); return ret; } @@ -3686,7 +3686,7 @@ libxlDomainGetSchedulerType(virDomainPtr dom, int *nparams) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -3754,7 +3754,7 @@ libxlDomainGetSchedulerParametersFlags(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -3840,7 +3840,7 @@ libxlDomainSetSchedulerParametersFlags(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -3869,7 +3869,7 @@ libxlDomainIsActive(virDomainPtr dom) cleanup: if (obj) - virDomainObjUnlock(obj); + virObjectUnlock(obj); return ret; } @@ -3891,7 +3891,7 @@ libxlDomainIsPersistent(virDomainPtr dom) cleanup: if (obj) - virDomainObjUnlock(obj); + virObjectUnlock(obj); return ret; } @@ -3913,7 +3913,7 @@ libxlDomainIsUpdated(virDomainPtr dom) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 8050ce6..957a073 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -222,7 +222,7 @@ static virDomainPtr lxcDomainLookupByID(virConnectPtr conn, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return dom; } @@ -251,7 +251,7 @@ static virDomainPtr lxcDomainLookupByUUID(virConnectPtr conn, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return dom; } @@ -277,7 +277,7 @@ static virDomainPtr lxcDomainLookupByName(virConnectPtr conn, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return dom; } @@ -302,7 +302,7 @@ static int lxcDomainIsActive(virDomainPtr dom) cleanup: if (obj) - virDomainObjUnlock(obj); + virObjectUnlock(obj); return ret; } @@ -327,7 +327,7 @@ static int lxcDomainIsPersistent(virDomainPtr dom) cleanup: if (obj) - virDomainObjUnlock(obj); + virObjectUnlock(obj); return ret; } @@ -351,7 +351,7 @@ static int lxcDomainIsUpdated(virDomainPtr dom) cleanup: if (obj) - virDomainObjUnlock(obj); + virObjectUnlock(obj); return ret; } @@ -456,7 +456,7 @@ static virDomainPtr lxcDomainDefine(virConnectPtr conn, const char *xml) cleanup: virDomainDefFree(def); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) virDomainEventStateQueue(driver->domainEventState, event); lxcDriverUnlock(driver); @@ -509,7 +509,7 @@ static int lxcDomainUndefineFlags(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) virDomainEventStateQueue(driver->domainEventState, event); lxcDriverUnlock(driver); @@ -578,7 +578,7 @@ cleanup: if (cgroup) virCgroupFree(&cgroup); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -611,7 +611,7 @@ lxcDomainGetState(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -640,7 +640,7 @@ static char *lxcGetOSType(virDomainPtr dom) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -668,7 +668,7 @@ lxcDomainGetMaxMemory(virDomainPtr dom) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -700,7 +700,7 @@ static int lxcDomainSetMaxMemory(virDomainPtr dom, unsigned long newmax) { cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -755,7 +755,7 @@ static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem) { cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (cgroup) virCgroupFree(&cgroup); return ret; @@ -834,7 +834,7 @@ cleanup: if (cgroup) virCgroupFree(&cgroup); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); lxcDriverUnlock(driver); return ret; } @@ -933,7 +933,7 @@ cleanup: if (cgroup) virCgroupFree(&cgroup); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); lxcDriverUnlock(driver); return ret; } @@ -965,7 +965,7 @@ static char *lxcDomainGetXMLDesc(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -1024,7 +1024,7 @@ static int lxcDomainStartWithFlags(virDomainPtr dom, unsigned int flags) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) virDomainEventStateQueue(driver->domainEventState, event); lxcDriverUnlock(driver); @@ -1111,7 +1111,7 @@ lxcDomainCreateAndStart(virConnectPtr conn, cleanup: virDomainDefFree(def); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) virDomainEventStateQueue(driver->domainEventState, event); lxcDriverUnlock(driver); @@ -1172,7 +1172,7 @@ static int lxcDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr secla cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); lxcDriverUnlock(driver); return ret; } @@ -1342,7 +1342,7 @@ lxcDomainDestroyFlags(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) virDomainEventStateQueue(driver->domainEventState, event); lxcDriverUnlock(driver); @@ -1871,7 +1871,7 @@ cleanup: virDomainDefFree(vmdef); virCgroupFree(&group); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); lxcDriverUnlock(driver); return ret; } @@ -1990,7 +1990,7 @@ out: cleanup: virCgroupFree(&group); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); lxcDriverUnlock(driver); return ret; } @@ -2099,7 +2099,7 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, cleanup: virCgroupFree(&group); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); lxcDriverUnlock(driver); return ret; } @@ -2206,7 +2206,7 @@ cleanup: if (group) virCgroupFree(&group); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); lxcDriverUnlock(driver); return ret; } @@ -2258,7 +2258,7 @@ lxcDomainInterfaceStats(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } #else @@ -2295,7 +2295,7 @@ static int lxcDomainGetAutostart(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -2369,7 +2369,7 @@ cleanup: VIR_FREE(configFile); VIR_FREE(autostartLink); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); lxcDriverUnlock(driver); return ret; } @@ -2512,7 +2512,7 @@ cleanup: if (event) virDomainEventStateQueue(driver->domainEventState, event); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); lxcDriverUnlock(driver); return ret; } @@ -2578,7 +2578,7 @@ cleanup: if (event) virDomainEventStateQueue(driver->domainEventState, event); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); lxcDriverUnlock(driver); return ret; } @@ -2648,7 +2648,7 @@ lxcDomainOpenConsole(virDomainPtr dom, ret = 0; cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); lxcDriverUnlock(driver); return ret; } @@ -2727,7 +2727,7 @@ lxcDomainSendProcessSignal(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -2829,7 +2829,7 @@ lxcDomainShutdownFlags(virDomainPtr dom, cleanup: VIR_FREE(vroot); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -2918,7 +2918,7 @@ lxcDomainReboot(virDomainPtr dom, cleanup: VIR_FREE(vroot); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -4418,7 +4418,7 @@ cleanup: virDomainDeviceDefFree(dev_copy); virDomainDeviceDefFree(dev); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); lxcDriverUnlock(driver); return ret; } diff --git a/src/lxc/lxc_monitor.c b/src/lxc/lxc_monitor.c index f697e09..e3901f8 100644 --- a/src/lxc/lxc_monitor.c +++ b/src/lxc/lxc_monitor.c @@ -34,9 +34,7 @@ #define VIR_FROM_THIS VIR_FROM_LXC struct _virLXCMonitor { - virObject parent; - - virMutex lock; + virObjectLockable parent; virDomainObjPtr vm; virLXCMonitorCallbacks cb; @@ -50,7 +48,7 @@ static void virLXCMonitorDispose(void *obj); static int virLXCMonitorOnceInit(void) { - if (!(virLXCMonitorClass = virClassNew(virClassForObject(), + if (!(virLXCMonitorClass = virClassNew(virClassForObjectLockable(), "virLXCMonitor", sizeof(virLXCMonitor), virLXCMonitorDispose))) @@ -120,10 +118,10 @@ static void virLXCMonitorEOFNotify(virNetClientPtr client ATTRIBUTE_UNUSED, virDomainObjPtr vm; VIR_DEBUG("EOF notify mon=%p", mon); - virLXCMonitorLock(mon); + virObjectLock(mon); eofNotify = mon->cb.eofNotify; vm = mon->vm; - virLXCMonitorUnlock(mon); + virObjectUnlock(mon); if (eofNotify) { VIR_DEBUG("EOF callback mon=%p vm=%p", mon, vm); @@ -151,15 +149,8 @@ virLXCMonitorPtr virLXCMonitorNew(virDomainObjPtr vm, if (virLXCMonitorInitialize() < 0) return NULL; - if (!(mon = virObjectNew(virLXCMonitorClass))) - return NULL; - - if (virMutexInit(&mon->lock) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot initialize monitor mutex")); - VIR_FREE(mon); + if (!(mon = virObjectLockableNew(virLXCMonitorClass))) return NULL; - } if (virAsprintf(&sockpath, "%s/%s.sock", socketdir, vm->def->name) < 0) @@ -209,7 +200,6 @@ static void virLXCMonitorDispose(void *opaque) VIR_DEBUG("mon=%p", mon); if (mon->cb.destroy) (mon->cb.destroy)(mon, mon->vm); - virMutexDestroy(&mon->lock); virObjectUnref(mon->program); } @@ -229,15 +219,3 @@ void virLXCMonitorClose(virLXCMonitorPtr mon) mon->client = NULL; } } - - -void virLXCMonitorLock(virLXCMonitorPtr mon) -{ - virMutexLock(&mon->lock); -} - - -void virLXCMonitorUnlock(virLXCMonitorPtr mon) -{ - virMutexUnlock(&mon->lock); -} diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 1a89e4a..4ad7ba0 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -104,7 +104,7 @@ static void virLXCProcessAutoDestroyDom(void *payload, virDomainRemoveInactive(&data->driver->domains, dom); if (dom) - virDomainObjUnlock(dom); + virObjectUnlock(dom); if (event) virDomainEventStateQueue(data->driver->domainEventState, event); virHashRemoveEntry(data->driver->autodestroy, uuidstr); @@ -558,7 +558,7 @@ static void virLXCProcessMonitorEOFNotify(virLXCMonitorPtr mon, VIR_DEBUG("mon=%p vm=%p", mon, vm); lxcDriverLock(driver); - virDomainObjLock(vm); + virObjectLock(vm); lxcDriverUnlock(driver); priv = vm->privateData; @@ -595,7 +595,7 @@ static void virLXCProcessMonitorEOFNotify(virLXCMonitorPtr mon, } if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) { lxcDriverLock(driver); virDomainEventStateQueue(driver->domainEventState, event); @@ -1250,7 +1250,7 @@ virLXCProcessAutostartDomain(void *payload, const void *name ATTRIBUTE_UNUSED, v virDomainObjPtr vm = payload; const struct virLXCProcessAutostartData *data = opaque; - virDomainObjLock(vm); + virObjectLock(vm); if (vm->autostart && !virDomainObjIsActive(vm)) { int ret = virLXCProcessStart(data->conn, data->driver, vm, false, @@ -1270,7 +1270,7 @@ virLXCProcessAutostartDomain(void *payload, const void *name ATTRIBUTE_UNUSED, v virDomainEventStateQueue(data->driver->domainEventState, event); } } - virDomainObjUnlock(vm); + virObjectUnlock(vm); } @@ -1302,7 +1302,7 @@ virLXCProcessReconnectDomain(void *payload, const void *name ATTRIBUTE_UNUSED, v virLXCDriverPtr driver = opaque; virLXCDomainObjPrivatePtr priv; - virDomainObjLock(vm); + virObjectLock(vm); VIR_DEBUG("Reconnect id=%d pid=%d state=%d", vm->def->id, vm->pid, vm->state.state); priv = vm->privateData; @@ -1345,7 +1345,7 @@ virLXCProcessReconnectDomain(void *payload, const void *name ATTRIBUTE_UNUSED, v } cleanup: - virDomainObjUnlock(vm); + virObjectUnlock(vm); return; error: diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index 086bb13..8508f06 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -1161,7 +1161,7 @@ virNWFilterDomainFWUpdateCB(void *payload, int i, err; bool skipIface; - virDomainObjLock(obj); + virObjectLock(obj); if (virDomainObjIsActive(obj)) { for (i = 0; i < vm->nnets; i++) { @@ -1209,5 +1209,5 @@ virNWFilterDomainFWUpdateCB(void *payload, } } - virDomainObjUnlock(obj); + virObjectUnlock(obj); } diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 09518d5..8db6d6b 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -669,7 +669,7 @@ int openvzLoadDomains(struct openvz_driver *driver) { goto cleanup; } - virDomainObjUnlock(dom); + virObjectUnlock(dom); dom = NULL; } diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index c6f814e..f4f93f5 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -264,7 +264,7 @@ openvzDomainGetHostname(virDomainPtr dom, unsigned int flags) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return hostname; error: @@ -294,7 +294,7 @@ static virDomainPtr openvzDomainLookupByID(virConnectPtr conn, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return dom; } @@ -326,7 +326,7 @@ static char *openvzGetOSType(virDomainPtr dom) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -352,7 +352,7 @@ static virDomainPtr openvzDomainLookupByUUID(virConnectPtr conn, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return dom; } @@ -377,7 +377,7 @@ static virDomainPtr openvzDomainLookupByName(virConnectPtr conn, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return dom; } @@ -419,7 +419,7 @@ static int openvzDomainGetInfo(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -450,7 +450,7 @@ openvzDomainGetState(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -472,7 +472,7 @@ static int openvzDomainIsActive(virDomainPtr dom) cleanup: if (obj) - virDomainObjUnlock(obj); + virObjectUnlock(obj); return ret; } @@ -494,7 +494,7 @@ static int openvzDomainIsPersistent(virDomainPtr dom) cleanup: if (obj) - virDomainObjUnlock(obj); + virObjectUnlock(obj); return ret; } @@ -524,7 +524,7 @@ static char *openvzDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -582,7 +582,7 @@ static int openvzDomainSuspend(virDomainPtr dom) { cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -620,7 +620,7 @@ static int openvzDomainResume(virDomainPtr dom) { cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -665,7 +665,7 @@ openvzDomainShutdownFlags(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -714,7 +714,7 @@ static int openvzDomainReboot(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -1024,7 +1024,7 @@ openvzDomainDefineXML(virConnectPtr conn, const char *xml) cleanup: virDomainDefFree(vmdef); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); openvzDriverUnlock(driver); return dom; } @@ -1109,7 +1109,7 @@ openvzDomainCreateXML(virConnectPtr conn, const char *xml, cleanup: virDomainDefFree(vmdef); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); openvzDriverUnlock(driver); return dom; } @@ -1157,7 +1157,7 @@ openvzDomainCreateWithFlags(virDomainPtr dom, unsigned int flags) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -1206,7 +1206,7 @@ openvzDomainUndefineFlags(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); openvzDriverUnlock(driver); return ret; } @@ -1244,7 +1244,7 @@ openvzDomainSetAutostart(virDomainPtr dom, int autostart) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -1281,7 +1281,7 @@ cleanup: VIR_FREE(value); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -1372,7 +1372,7 @@ static int openvzDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -1985,7 +1985,7 @@ openvzDomainInterfaceStats(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -2090,7 +2090,7 @@ cleanup: openvzDriverUnlock(driver); virDomainDeviceDefFree(dev); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 6f33080..1383ff8 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -854,7 +854,7 @@ parallelsLoadDomain(parallelsConnPtr privconn, virJSONValuePtr jobj) else dom->autostart = 0; - virDomainObjUnlock(dom); + virObjectUnlock(dom); return dom; @@ -1137,7 +1137,7 @@ parallelsLookupDomainByID(virConnectPtr conn, int id) cleanup: if (dom) - virDomainObjUnlock(dom); + virObjectUnlock(dom); return ret; } @@ -1166,7 +1166,7 @@ parallelsLookupDomainByUUID(virConnectPtr conn, const unsigned char *uuid) cleanup: if (dom) - virDomainObjUnlock(dom); + virObjectUnlock(dom); return ret; } @@ -1193,7 +1193,7 @@ parallelsLookupDomainByName(virConnectPtr conn, const char *name) cleanup: if (dom) - virDomainObjUnlock(dom); + virObjectUnlock(dom); return ret; } @@ -1222,7 +1222,7 @@ parallelsGetDomainInfo(virDomainPtr domain, virDomainInfoPtr info) cleanup: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); return ret; } @@ -1246,7 +1246,7 @@ parallelsGetOSType(virDomainPtr domain) cleanup: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); parallelsDriverUnlock(privconn); return ret; } @@ -1269,7 +1269,7 @@ parallelsDomainIsPersistent(virDomainPtr domain) cleanup: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); parallelsDriverUnlock(privconn); return ret; } @@ -1297,7 +1297,7 @@ parallelsDomainGetState(virDomainPtr domain, cleanup: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); return ret; } @@ -1327,7 +1327,7 @@ parallelsDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) cleanup: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); return ret; } @@ -1352,7 +1352,7 @@ parallelsDomainGetAutostart(virDomainPtr domain, int *autostart) cleanup: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); return ret; } @@ -1395,7 +1395,7 @@ parallelsDomainChangeState(virDomainPtr domain, cleanup: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); return ret; } @@ -2360,10 +2360,10 @@ parallelsDomainDefineXML(virConnectPtr conn, const char *xml) } if (parallelsApplyChanges(conn, olddom, def) < 0) { - virDomainObjUnlock(olddom); + virObjectUnlock(olddom); goto cleanup; } - virDomainObjUnlock(olddom); + virObjectUnlock(olddom); if (!(dom = virDomainAssignDef(privconn->caps, &privconn->domains, def, false))) { @@ -2381,7 +2381,7 @@ parallelsDomainDefineXML(virConnectPtr conn, const char *xml) cleanup: virDomainDefFree(def); if (dom) - virDomainObjUnlock(dom); + virObjectUnlock(dom); parallelsDriverUnlock(privconn); return ret; } diff --git a/src/qemu/THREADS.txt b/src/qemu/THREADS.txt index f697537..c3bad21 100644 --- a/src/qemu/THREADS.txt +++ b/src/qemu/THREADS.txt @@ -125,10 +125,10 @@ To lock the driver To lock the virDomainObjPtr - virDomainObjLock() + virObjectLock() - Acquires the virDomainObjPtr lock - virDomainObjUnlock() + virObjectUnlock() - Releases the virDomainObjPtr lock @@ -419,9 +419,9 @@ Design patterns ...monitor job progress... qemuDomainObjExitMonitorWithDriver(driver, obj); - virDomainObjUnlock(obj); + virObjectUnlock(obj); sleep(aWhile); - virDomainObjLock(obj); + virObjectLock(obj); } ...do final work... diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index db4a0bc..0997784 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -82,9 +82,8 @@ struct _qemuAgentMessage { struct _qemuAgent { - virObject object; + virObjectLockable parent; - virMutex lock; /* also used to protect fd */ virCond notify; int fd; @@ -121,7 +120,7 @@ static void qemuAgentDispose(void *obj); static int qemuAgentOnceInit(void) { - if (!(qemuAgentClass = virClassNew(virClassForObject(), + if (!(qemuAgentClass = virClassNew(virClassForObjectLockable(), "qemuAgent", sizeof(qemuAgent), qemuAgentDispose))) @@ -153,17 +152,6 @@ qemuAgentEscapeNonPrintable(const char *text) } #endif -void qemuAgentLock(qemuAgentPtr mon) -{ - virMutexLock(&mon->lock); -} - - -void qemuAgentUnlock(qemuAgentPtr mon) -{ - virMutexUnlock(&mon->lock); -} - static void qemuAgentDispose(void *obj) { @@ -172,7 +160,6 @@ static void qemuAgentDispose(void *obj) if (mon->cb && mon->cb->destroy) (mon->cb->destroy)(mon, mon->vm); ignore_value(virCondDestroy(&mon->notify)); - virMutexDestroy(&mon->lock); VIR_FREE(mon->buffer); } @@ -592,7 +579,7 @@ qemuAgentIO(int watch, int fd, int events, void *opaque) { virObjectRef(mon); /* lock access to the monitor and protect fd */ - qemuAgentLock(mon); + virObjectLock(mon); #if DEBUG_IO VIR_DEBUG("Agent %p I/O on watch %d fd %d events %d", mon, watch, fd, events); #endif @@ -695,7 +682,7 @@ qemuAgentIO(int watch, int fd, int events, void *opaque) { /* Make sure anyone waiting wakes up now */ virCondSignal(&mon->notify); - qemuAgentUnlock(mon); + virObjectUnlock(mon); virObjectUnref(mon); VIR_DEBUG("Triggering EOF callback"); (eofNotify)(mon, vm); @@ -706,12 +693,12 @@ qemuAgentIO(int watch, int fd, int events, void *opaque) { /* Make sure anyone waiting wakes up now */ virCondSignal(&mon->notify); - qemuAgentUnlock(mon); + virObjectUnlock(mon); virObjectUnref(mon); VIR_DEBUG("Triggering error callback"); (errorNotify)(mon, vm); } else { - qemuAgentUnlock(mon); + virObjectUnlock(mon); virObjectUnref(mon); } } @@ -733,26 +720,18 @@ qemuAgentOpen(virDomainObjPtr vm, if (qemuAgentInitialize() < 0) return NULL; - if (!(mon = virObjectNew(qemuAgentClass))) + if (!(mon = virObjectLockableNew(qemuAgentClass))) return NULL; - if (virMutexInit(&mon->lock) < 0) { - virReportSystemError(errno, "%s", - _("cannot initialize monitor mutex")); - VIR_FREE(mon); - return NULL; - } + mon->fd = -1; if (virCondInit(&mon->notify) < 0) { virReportSystemError(errno, "%s", _("cannot initialize monitor condition")); - virMutexDestroy(&mon->lock); - VIR_FREE(mon); + virObjectUnref(mon); return NULL; } - mon->fd = -1; mon->vm = vm; mon->cb = cb; - qemuAgentLock(mon); switch (config->type) { case VIR_DOMAIN_CHR_TYPE_UNIX: @@ -791,7 +770,6 @@ qemuAgentOpen(virDomainObjPtr vm, virObjectRef(mon); VIR_DEBUG("New mon %p fd =%d watch=%d", mon, mon->fd, mon->watch); - qemuAgentUnlock(mon); return mon; @@ -802,7 +780,6 @@ cleanup: * so kill the callbacks now. */ mon->cb = NULL; - qemuAgentUnlock(mon); qemuAgentClose(mon); return NULL; } @@ -815,7 +792,7 @@ void qemuAgentClose(qemuAgentPtr mon) VIR_DEBUG("mon=%p", mon); - qemuAgentLock(mon); + virObjectLock(mon); if (mon->fd >= 0) { if (mon->watch) @@ -829,7 +806,7 @@ void qemuAgentClose(qemuAgentPtr mon) mon->msg->finished = 1; virCondSignal(&mon->notify); } - qemuAgentUnlock(mon); + virObjectUnlock(mon); virObjectUnref(mon); } @@ -883,8 +860,8 @@ static int qemuAgentSend(qemuAgentPtr mon, qemuAgentUpdateWatch(mon); while (!mon->msg->finished) { - if ((then && virCondWaitUntil(&mon->notify, &mon->lock, then) < 0) || - (!then && virCondWait(&mon->notify, &mon->lock) < 0)) { + if ((then && virCondWaitUntil(&mon->notify, &mon->parent.lock, then) < 0) || + (!then && virCondWait(&mon->notify, &mon->parent.lock) < 0)) { if (errno == ETIMEDOUT) { virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s", _("Guest agent not available for now")); diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index dad068b..ba04e61 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -47,9 +47,6 @@ qemuAgentPtr qemuAgentOpen(virDomainObjPtr vm, virDomainChrSourceDefPtr config, qemuAgentCallbacksPtr cb); -void qemuAgentLock(qemuAgentPtr mon); -void qemuAgentUnlock(qemuAgentPtr mon); - void qemuAgentClose(qemuAgentPtr mon); typedef enum { diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 29b5066..c92321b 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2380,7 +2380,7 @@ qemuCapsInitQMP(qemuCapsPtr caps, goto cleanup; } - qemuMonitorLock(mon); + virObjectLock(mon); if (qemuMonitorSetCapabilities(mon) < 0) { virErrorPtr err = virGetLastError(); @@ -2453,7 +2453,7 @@ qemuCapsInitQMP(qemuCapsPtr caps, cleanup: if (mon) - qemuMonitorUnlock(mon); + virObjectUnlock(mon); qemuMonitorClose(mon); virCommandAbort(cmd); virCommandFree(cmd); diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index a138352..680e8fb 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -536,7 +536,7 @@ qemuDriverCloseCallbackRun(void *payload, dom = closeDef->cb(data->driver, dom, data->conn); if (dom) - virDomainObjUnlock(dom); + virObjectUnlock(dom); virHashRemoveEntry(data->driver->closeCallbacks, uuidstr); } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 08774b9..1ae75e9 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -786,12 +786,12 @@ retry: } while (!nested && !qemuDomainNestedJobAllowed(priv, job)) { - if (virCondWaitUntil(&priv->job.asyncCond, &obj->lock, then) < 0) + if (virCondWaitUntil(&priv->job.asyncCond, &obj->parent.lock, then) < 0) goto error; } while (priv->job.active) { - if (virCondWaitUntil(&priv->job.cond, &obj->lock, then) < 0) + if (virCondWaitUntil(&priv->job.cond, &obj->parent.lock, then) < 0) goto error; } @@ -818,9 +818,9 @@ retry: } if (driver_locked) { - virDomainObjUnlock(obj); + virObjectUnlock(obj); qemuDriverLock(driver); - virDomainObjLock(obj); + virObjectLock(obj); } if (qemuDomainTrackJob(job)) @@ -851,9 +851,9 @@ error: "%s", _("cannot acquire job mutex")); priv->jobs_queued--; if (driver_locked) { - virDomainObjUnlock(obj); + virObjectUnlock(obj); qemuDriverLock(driver); - virDomainObjLock(obj); + virObjectLock(obj); } virObjectUnref(obj); return -1; @@ -1004,10 +1004,10 @@ qemuDomainObjEnterMonitorInternal(virQEMUDriverPtr driver, " monitor without asking for a nested job is dangerous"); } - qemuMonitorLock(priv->mon); + virObjectLock(priv->mon); virObjectRef(priv->mon); ignore_value(virTimeMillisNow(&priv->monStart)); - virDomainObjUnlock(obj); + virObjectUnlock(obj); if (driver_locked) qemuDriverUnlock(driver); @@ -1025,11 +1025,11 @@ qemuDomainObjExitMonitorInternal(virQEMUDriverPtr driver, hasRefs = virObjectUnref(priv->mon); if (hasRefs) - qemuMonitorUnlock(priv->mon); + virObjectUnlock(priv->mon); if (driver_locked) qemuDriverLock(driver); - virDomainObjLock(obj); + virObjectLock(obj); priv->monStart = 0; if (!hasRefs) @@ -1127,10 +1127,10 @@ qemuDomainObjEnterAgentInternal(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = obj->privateData; - qemuAgentLock(priv->agent); + virObjectLock(priv->agent); virObjectRef(priv->agent); ignore_value(virTimeMillisNow(&priv->agentStart)); - virDomainObjUnlock(obj); + virObjectUnlock(obj); if (driver_locked) qemuDriverUnlock(driver); @@ -1148,11 +1148,11 @@ qemuDomainObjExitAgentInternal(virQEMUDriverPtr driver, hasRefs = virObjectUnref(priv->agent); if (hasRefs) - qemuAgentUnlock(priv->agent); + virObjectUnlock(priv->agent); if (driver_locked) qemuDriverLock(driver); - virDomainObjLock(obj); + virObjectLock(obj); priv->agentStart = 0; if (!hasRefs) @@ -1214,7 +1214,7 @@ void qemuDomainObjEnterRemoteWithDriver(virQEMUDriverPtr driver, virDomainObjPtr obj) { virObjectRef(obj); - virDomainObjUnlock(obj); + virObjectUnlock(obj); qemuDriverUnlock(driver); } @@ -1222,7 +1222,7 @@ void qemuDomainObjExitRemoteWithDriver(virQEMUDriverPtr driver, virDomainObjPtr obj) { qemuDriverLock(driver); - virDomainObjLock(obj); + virObjectLock(obj); virObjectUnref(obj); } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 06b0d28..ab1fd4c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -290,7 +290,7 @@ qemuAutostartDomain(void *payload, const void *name ATTRIBUTE_UNUSED, if (data->driver->autoStartBypassCache) flags |= VIR_DOMAIN_START_BYPASS_CACHE; - virDomainObjLock(vm); + virObjectLock(vm); virResetLastError(); if (vm->autostart && !virDomainObjIsActive(vm)) { @@ -316,7 +316,7 @@ qemuAutostartDomain(void *payload, const void *name ATTRIBUTE_UNUSED, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); } @@ -501,7 +501,7 @@ qemuDomainSnapshotLoad(void *payload, VIR_DOMAIN_SNAPSHOT_PARSE_DISKS | VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL); - virDomainObjLock(vm); + virObjectLock(vm); if (virAsprintf(&snapDir, "%s/%s", baseDir, vm->def->name) < 0) { VIR_ERROR(_("Failed to allocate memory for snapshot directory for domain %s"), vm->def->name); @@ -591,7 +591,7 @@ cleanup: if (dir) closedir(dir); VIR_FREE(snapDir); - virDomainObjUnlock(vm); + virObjectUnlock(vm); } @@ -604,7 +604,7 @@ qemuDomainNetsRestart(void *payload, virDomainObjPtr vm = (virDomainObjPtr)payload; virDomainDefPtr def = vm->def; - virDomainObjLock(vm); + virObjectLock(vm); for (i = 0; i < def->nnets; i++) { virDomainNetDefPtr net = def->nets[i]; @@ -621,7 +621,7 @@ qemuDomainNetsRestart(void *payload, } } - virDomainObjUnlock(vm); + virObjectUnlock(vm); } @@ -1459,7 +1459,7 @@ static virDomainPtr qemuDomainLookupByID(virConnectPtr conn, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return dom; } @@ -1486,7 +1486,7 @@ static virDomainPtr qemuDomainLookupByUUID(virConnectPtr conn, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return dom; } @@ -1511,7 +1511,7 @@ static virDomainPtr qemuDomainLookupByName(virConnectPtr conn, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return dom; } @@ -1528,7 +1528,7 @@ static int qemuDomainIsActive(virDomainPtr dom) cleanup: if (obj) - virDomainObjUnlock(obj); + virObjectUnlock(obj); return ret; } @@ -1544,7 +1544,7 @@ static int qemuDomainIsPersistent(virDomainPtr dom) cleanup: if (obj) - virDomainObjUnlock(obj); + virObjectUnlock(obj); return ret; } @@ -1560,7 +1560,7 @@ static int qemuDomainIsUpdated(virDomainPtr dom) cleanup: if (obj) - virDomainObjUnlock(obj); + virObjectUnlock(obj); return ret; } @@ -1712,7 +1712,7 @@ static virDomainPtr qemuDomainCreate(virConnectPtr conn, const char *xml, cleanup: virDomainDefFree(def); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) { qemuDomainEventQueue(driver, event); if (event2) @@ -1798,7 +1798,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) qemuDomainEventQueue(driver, event); @@ -1862,7 +1862,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) qemuDomainEventQueue(driver, event); qemuDriverUnlock(driver); @@ -1938,7 +1938,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -2036,7 +2036,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -2076,7 +2076,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -2169,7 +2169,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) qemuDomainEventQueue(driver, event); qemuDriverUnlock(driver); @@ -2194,7 +2194,7 @@ static char *qemuDomainGetOSType(virDomainPtr dom) { cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return type; } @@ -2212,7 +2212,7 @@ qemuDomainGetMaxMemory(virDomainPtr dom) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -2301,7 +2301,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -2363,7 +2363,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -2432,7 +2432,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -2509,7 +2509,7 @@ static int qemuDomainGetInfo(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -2532,7 +2532,7 @@ qemuDomainGetState(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -2582,7 +2582,7 @@ qemuDomainGetControlInfo(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -3032,7 +3032,7 @@ cleanup: if (event) qemuDomainEventQueue(driver, event); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -3106,7 +3106,7 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return ret; @@ -3172,7 +3172,7 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); VIR_FREE(name); @@ -3188,7 +3188,7 @@ qemuDomainManagedSaveLoad(void *payload, virQEMUDriverPtr driver = opaque; char *name; - virDomainObjLock(vm); + virObjectLock(vm); if (!(name = qemuDomainManagedSavePath(driver, vm))) goto cleanup; @@ -3196,7 +3196,7 @@ qemuDomainManagedSaveLoad(void *payload, vm->hasManagedSave = virFileExists(name); cleanup: - virDomainObjUnlock(vm); + virObjectUnlock(vm); VIR_FREE(name); } @@ -3212,7 +3212,7 @@ qemuDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags) return -1; ret = vm->hasManagedSave; - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -3244,7 +3244,7 @@ qemuDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags) cleanup: VIR_FREE(name); - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -3474,7 +3474,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) qemuDomainEventQueue(driver, event); qemuDriverUnlock(driver); @@ -3564,7 +3564,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -3575,7 +3575,7 @@ static void processWatchdogEvent(void *data, void *opaque) virQEMUDriverPtr driver = opaque; qemuDriverLock(driver); - virDomainObjLock(wdEvent->vm); + virObjectLock(wdEvent->vm); switch (wdEvent->action) { case VIR_DOMAIN_WATCHDOG_ACTION_DUMP: @@ -3633,7 +3633,7 @@ endjob: ignore_value(qemuDomainObjEndAsyncJob(driver, wdEvent->vm)); unlock: - virDomainObjUnlock(wdEvent->vm); + virObjectUnlock(wdEvent->vm); virObjectUnref(wdEvent->vm); qemuDriverUnlock(driver); VIR_FREE(wdEvent); @@ -3924,7 +3924,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -4096,7 +4096,7 @@ cleanup: if (cgroup_dom) virCgroupFree(&cgroup_dom); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); virBitmapFree(pcpumap); return ret; } @@ -4186,7 +4186,7 @@ qemuDomainGetVcpuPinInfo(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -4339,7 +4339,7 @@ cleanup: virBitmapFree(pcpumap); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -4406,7 +4406,7 @@ qemuDomainGetEmulatorPinInfo(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -4495,7 +4495,7 @@ qemuDomainGetVcpus(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -4526,7 +4526,7 @@ qemuDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -4590,7 +4590,7 @@ static int qemuDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr secl cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -4663,7 +4663,7 @@ static int qemuDomainGetSecurityLabelList(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -5032,7 +5032,7 @@ cleanup: VIR_FORCE_CLOSE(fd); virFileWrapperFdFree(wrapperFd); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -5267,7 +5267,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -5582,7 +5582,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -5687,7 +5687,7 @@ static virDomainPtr qemuDomainDefine(virConnectPtr conn, const char *xml) { cleanup: virDomainDefFree(def); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) qemuDomainEventQueue(driver, event); virObjectUnref(caps); @@ -5784,7 +5784,7 @@ qemuDomainUndefineFlags(virDomainPtr dom, cleanup: VIR_FREE(name); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) qemuDomainEventQueue(driver, event); qemuDriverUnlock(driver); @@ -6570,7 +6570,7 @@ cleanup: virDomainDeviceDefFree(dev_copy); virDomainDeviceDefFree(dev); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -6620,7 +6620,7 @@ static int qemuDomainGetAutostart(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -6687,7 +6687,7 @@ cleanup: VIR_FREE(configFile); VIR_FREE(autostartLink); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -7035,7 +7035,7 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, cleanup: virCgroupFree(&group); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -7221,7 +7221,7 @@ cleanup: if (group) virCgroupFree(&group); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -7390,7 +7390,7 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, cleanup: virCgroupFree(&group); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -7550,7 +7550,7 @@ cleanup: if (group) virCgroupFree(&group); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -7699,7 +7699,7 @@ qemuDomainSetNumaParameters(virDomainPtr dom, cleanup: virCgroupFree(&group); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -7812,7 +7812,7 @@ cleanup: VIR_FREE(nodeset); virCgroupFree(&group); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -8063,7 +8063,7 @@ cleanup: virDomainDefFree(vmdef); virCgroupFree(&group); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -8322,7 +8322,7 @@ out: cleanup: virCgroupFree(&group); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -8417,7 +8417,7 @@ endjob: cleanup: VIR_FREE(device); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -8488,7 +8488,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -8658,7 +8658,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -8698,7 +8698,7 @@ qemuDomainInterfaceStats(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } #else @@ -8882,7 +8882,7 @@ cleanup: virNetDevBandwidthFree(newBandwidth); virCgroupFree(&group); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -9004,7 +9004,7 @@ cleanup: if (group) virCgroupFree(&group); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -9055,7 +9055,7 @@ qemuDomainMemoryStats(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -9113,7 +9113,7 @@ qemuDomainBlockPeek(virDomainPtr dom, cleanup: VIR_FORCE_CLOSE(fd); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -9198,7 +9198,7 @@ cleanup: unlink(tmp); VIR_FREE(tmp); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -9343,7 +9343,7 @@ cleanup: virStorageFileFreeMetadata(meta); VIR_FORCE_CLOSE(fd); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -9715,7 +9715,7 @@ qemuDomainMigrateBegin3(virDomainPtr domain, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return xml; @@ -9950,7 +9950,7 @@ qemuDomainMigrateConfirm3(virDomainPtr domain, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -10187,7 +10187,7 @@ static int qemuDomainGetJobInfo(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -10235,7 +10235,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -10283,7 +10283,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -10331,7 +10331,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -10355,7 +10355,7 @@ qemuDomainMigrateGetMaxSpeed(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -11599,7 +11599,7 @@ cleanup: } else if (snap) { virDomainSnapshotObjListRemove(vm->snapshots, snap); } - virDomainObjUnlock(vm); + virObjectUnlock(vm); } virDomainSnapshotDefFree(def); VIR_FREE(xml); @@ -11625,7 +11625,7 @@ static int qemuDomainSnapshotListNames(virDomainPtr domain, char **names, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return n; } @@ -11645,7 +11645,7 @@ static int qemuDomainSnapshotNum(virDomainPtr domain, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return n; } @@ -11666,7 +11666,7 @@ qemuDomainListAllSnapshots(virDomainPtr domain, virDomainSnapshotPtr **snaps, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return n; } @@ -11694,7 +11694,7 @@ qemuDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return n; } @@ -11719,7 +11719,7 @@ qemuDomainSnapshotNumChildren(virDomainSnapshotPtr snapshot, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return n; } @@ -11746,7 +11746,7 @@ qemuDomainSnapshotListAllChildren(virDomainSnapshotPtr snapshot, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return n; } @@ -11770,7 +11770,7 @@ static virDomainSnapshotPtr qemuDomainSnapshotLookupByName(virDomainPtr domain, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return snapshot; } @@ -11789,7 +11789,7 @@ static int qemuDomainHasCurrentSnapshot(virDomainPtr domain, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -11820,7 +11820,7 @@ qemuDomainSnapshotGetParent(virDomainSnapshotPtr snapshot, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return parent; } @@ -11845,7 +11845,7 @@ static virDomainSnapshotPtr qemuDomainSnapshotCurrent(virDomainPtr domain, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return snapshot; } @@ -11871,7 +11871,7 @@ static char *qemuDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return xml; } @@ -11896,7 +11896,7 @@ qemuDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -11924,7 +11924,7 @@ qemuDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -12267,7 +12267,7 @@ cleanup: qemuDomainEventQueue(driver, event2); } if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return ret; @@ -12422,7 +12422,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -12473,7 +12473,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -12564,7 +12564,7 @@ cleanup: virObjectUnref(caps); virDomainChrSourceDefFree(monConfig); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); VIR_FREE(pidfile); return dom; @@ -12646,7 +12646,7 @@ qemuDomainOpenConsole(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -12718,7 +12718,7 @@ qemuDomainOpenChannel(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -13051,13 +13051,13 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char *path, const char *base, if (ret <= 0) break; - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); nanosleep(&ts, NULL); qemuDriverLock(driver); - virDomainObjLock(vm); + virObjectLock(vm); if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -13078,7 +13078,7 @@ endjob: cleanup: VIR_FREE(device); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) qemuDomainEventQueue(driver, event); qemuDriverUnlock(driver); @@ -13290,7 +13290,7 @@ cleanup: virCgroupFree(&cgroup); VIR_FREE(device); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -13468,7 +13468,7 @@ endjob: cleanup: VIR_FREE(device); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -13536,7 +13536,7 @@ qemuDomainOpenGraphics(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -13708,7 +13708,7 @@ endjob: cleanup: VIR_FREE(device); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -13844,7 +13844,7 @@ endjob: cleanup: VIR_FREE(device); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -13916,7 +13916,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); virHashFree(table); if (ret < 0) { for (i = 0; i < n; i++) @@ -14010,7 +14010,7 @@ qemuDomainSetMetadata(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; no_memory: virReportOOMError(); @@ -14075,7 +14075,7 @@ qemuDomainGetMetadata(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -14346,7 +14346,7 @@ qemuDomainGetCPUStats(virDomainPtr domain, cleanup: virCgroupFree(&group); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -14448,7 +14448,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -14494,7 +14494,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -14578,7 +14578,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return result; } @@ -14645,7 +14645,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index e235677..19bf369 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1267,13 +1267,13 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver, virDomainObjPtr vm, goto cleanup; } - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); nanosleep(&ts, NULL); qemuDriverLock(driver); - virDomainObjLock(vm); + virObjectLock(vm); } cleanup: @@ -1751,7 +1751,7 @@ cleanup: VIR_FORCE_CLOSE(dataFD[1]); if (vm) { if (ret >= 0 || vm->persistent) - virDomainObjUnlock(vm); + virObjectUnlock(vm); else qemuDomainRemoveInactive(driver, vm); } @@ -3096,7 +3096,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) qemuDomainEventQueue(driver, event); return ret; @@ -3180,7 +3180,7 @@ endjob: cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) qemuDomainEventQueue(driver, event); return ret; @@ -3493,7 +3493,7 @@ endjob: cleanup: if (vm) { VIR_FREE(priv->origname); - virDomainObjUnlock(vm); + virObjectUnlock(vm); } if (event) qemuDomainEventQueue(driver, event); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index d6176c7..5d89ffd 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -48,9 +48,8 @@ #define DEBUG_RAW_IO 0 struct _qemuMonitor { - virObject object; + virObjectLockable parent; - virMutex lock; /* also used to protect fd */ virCond notify; int fd; @@ -86,7 +85,7 @@ static void qemuMonitorDispose(void *obj); static int qemuMonitorOnceInit(void) { - if (!(qemuMonitorClass = virClassNew(virClassForObject(), + if (!(qemuMonitorClass = virClassNew(virClassForObjectLockable(), "qemuMonitor", sizeof(qemuMonitor), qemuMonitorDispose))) @@ -230,17 +229,6 @@ static char * qemuMonitorEscapeNonPrintable(const char *text) } #endif -void qemuMonitorLock(qemuMonitorPtr mon) -{ - virMutexLock(&mon->lock); -} - -void qemuMonitorUnlock(qemuMonitorPtr mon) -{ - virMutexUnlock(&mon->lock); -} - - static void qemuMonitorDispose(void *obj) { qemuMonitorPtr mon = obj; @@ -250,7 +238,6 @@ static void qemuMonitorDispose(void *obj) (mon->cb->destroy)(mon, mon->vm); if (virCondDestroy(&mon->notify) < 0) {} - virMutexDestroy(&mon->lock); VIR_FREE(mon->buffer); } @@ -563,12 +550,12 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) { virObjectRef(mon); /* lock access to the monitor and protect fd */ - qemuMonitorLock(mon); + virObjectLock(mon); #if DEBUG_IO VIR_DEBUG("Monitor %p I/O on watch %d fd %d events %d", mon, watch, fd, events); #endif if (mon->fd == -1 || mon->watch == 0) { - qemuMonitorUnlock(mon); + virObjectUnlock(mon); virObjectUnref(mon); return; } @@ -666,7 +653,7 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) { /* Make sure anyone waiting wakes up now */ virCondSignal(&mon->notify); - qemuMonitorUnlock(mon); + virObjectUnlock(mon); virObjectUnref(mon); VIR_DEBUG("Triggering EOF callback"); (eofNotify)(mon, vm); @@ -677,12 +664,12 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) { /* Make sure anyone waiting wakes up now */ virCondSignal(&mon->notify); - qemuMonitorUnlock(mon); + virObjectUnlock(mon); virObjectUnref(mon); VIR_DEBUG("Triggering error callback"); (errorNotify)(mon, vm); } else { - qemuMonitorUnlock(mon); + virObjectUnlock(mon); virObjectUnref(mon); } } @@ -711,21 +698,14 @@ qemuMonitorOpenInternal(virDomainObjPtr vm, if (qemuMonitorInitialize() < 0) return NULL; - if (!(mon = virObjectNew(qemuMonitorClass))) + if (!(mon = virObjectLockableNew(qemuMonitorClass))) return NULL; - if (virMutexInit(&mon->lock) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot initialize monitor mutex")); - VIR_FREE(mon); - return NULL; - } + mon->fd = -1; if (virCondInit(&mon->notify) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot initialize monitor condition")); - virMutexDestroy(&mon->lock); - VIR_FREE(mon); - return NULL; + goto cleanup; } mon->fd = fd; mon->hasSendFD = hasSendFD; @@ -734,7 +714,7 @@ qemuMonitorOpenInternal(virDomainObjPtr vm, if (json) mon->wait_greeting = 1; mon->cb = cb; - qemuMonitorLock(mon); + virObjectLock(mon); if (virSetCloseExec(mon->fd) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -763,8 +743,8 @@ qemuMonitorOpenInternal(virDomainObjPtr vm, PROBE(QEMU_MONITOR_NEW, "mon=%p refs=%d fd=%d", - mon, mon->object.refs, mon->fd); - qemuMonitorUnlock(mon); + mon, mon->parent.parent.refs, mon->fd); + virObjectUnlock(mon); return mon; @@ -775,7 +755,7 @@ cleanup: * so kill the callbacks now. */ mon->cb = NULL; - qemuMonitorUnlock(mon); + virObjectUnlock(mon); /* The caller owns 'fd' on failure */ mon->fd = -1; if (mon->watch) @@ -834,9 +814,9 @@ void qemuMonitorClose(qemuMonitorPtr mon) if (!mon) return; - qemuMonitorLock(mon); + virObjectLock(mon); PROBE(QEMU_MONITOR_CLOSE, - "mon=%p refs=%d", mon, mon->object.refs); + "mon=%p refs=%d", mon, mon->parent.parent.refs); if (mon->fd >= 0) { if (mon->watch) { @@ -867,7 +847,7 @@ void qemuMonitorClose(qemuMonitorPtr mon) virCondSignal(&mon->notify); } - qemuMonitorUnlock(mon); + virObjectUnlock(mon); virObjectUnref(mon); } @@ -905,7 +885,7 @@ int qemuMonitorSend(qemuMonitorPtr mon, mon, mon->msg->txBuffer, mon->msg->txFD); while (!mon->msg->finished) { - if (virCondWait(&mon->notify, &mon->lock) < 0) { + if (virCondWait(&mon->notify, &mon->parent.lock) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to wait on monitor condition")); goto cleanup; @@ -960,10 +940,10 @@ cleanup: #define QEMU_MONITOR_CALLBACK(mon, ret, callback, ...) \ do { \ virObjectRef(mon); \ - qemuMonitorUnlock(mon); \ + virObjectUnlock(mon); \ if ((mon)->cb && (mon)->cb->callback) \ (ret) = ((mon)->cb->callback)(mon, __VA_ARGS__); \ - qemuMonitorLock(mon); \ + virObjectLock(mon); \ virObjectUnref(mon); \ } while (0) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 00ce813..7953b82 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -160,9 +160,6 @@ void qemuMonitorClose(qemuMonitorPtr mon); int qemuMonitorSetCapabilities(qemuMonitorPtr mon); -void qemuMonitorLock(qemuMonitorPtr mon); -void qemuMonitorUnlock(qemuMonitorPtr mon); - int qemuMonitorSetLink(qemuMonitorPtr mon, const char *name, enum virDomainNetInterfaceLinkState state) ; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 938c17e..c8c898f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -130,13 +130,13 @@ qemuProcessHandleAgentEOF(qemuAgentPtr agent, VIR_DEBUG("Received EOF from agent on %p '%s'", vm, vm->def->name); qemuDriverLock(driver); - virDomainObjLock(vm); + virObjectLock(vm); priv = vm->privateData; if (priv->agent == agent) priv->agent = NULL; - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); qemuAgentClose(agent); @@ -159,13 +159,13 @@ qemuProcessHandleAgentError(qemuAgentPtr agent ATTRIBUTE_UNUSED, VIR_DEBUG("Received error from agent on %p '%s'", vm, vm->def->name); qemuDriverLock(driver); - virDomainObjLock(vm); + virObjectLock(vm); priv = vm->privateData; priv->agentError = true; - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); } @@ -228,7 +228,7 @@ qemuConnectAgent(virQEMUDriverPtr driver, virDomainObjPtr vm) virObjectRef(vm); ignore_value(virTimeMillisNow(&priv->agentStart)); - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); agent = qemuAgentOpen(vm, @@ -236,7 +236,7 @@ qemuConnectAgent(virQEMUDriverPtr driver, virDomainObjPtr vm) &agentCallbacks); qemuDriverLock(driver); - virDomainObjLock(vm); + virObjectLock(vm); priv->agentStart = 0; if (virSecurityManagerClearSocketLabel(driver->securityManager, @@ -287,7 +287,7 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED, VIR_DEBUG("Received EOF on %p '%s'", vm, vm->def->name); qemuDriverLock(driver); - virDomainObjLock(vm); + virObjectLock(vm); priv = vm->privateData; @@ -321,7 +321,7 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED, } unlock: - virDomainObjUnlock(vm); + virObjectUnlock(vm); cleanup: if (event) @@ -346,14 +346,14 @@ qemuProcessHandleMonitorError(qemuMonitorPtr mon ATTRIBUTE_UNUSED, VIR_DEBUG("Received error on %p '%s'", vm, vm->def->name); qemuDriverLock(driver); - virDomainObjLock(vm); + virObjectLock(vm); ((qemuDomainObjPrivatePtr) vm->privateData)->monError = true; event = virDomainEventControlErrorNewFromObj(vm); if (event) qemuDomainEventQueue(driver, event); - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); } @@ -491,7 +491,7 @@ qemuProcessFindVolumeQcowPassphrase(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virDomainDiskDefPtr disk; int ret = -1; - virDomainObjLock(vm); + virObjectLock(vm); disk = qemuProcessFindDomainDiskByPath(vm, path); if (!disk) @@ -500,7 +500,7 @@ qemuProcessFindVolumeQcowPassphrase(qemuMonitorPtr mon ATTRIBUTE_UNUSED, ret = qemuProcessGetVolumeQcowPassphrase(conn, disk, secretRet, secretLen); cleanup: - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -513,14 +513,14 @@ qemuProcessHandleReset(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virDomainEventPtr event; qemuDomainObjPrivatePtr priv; - virDomainObjLock(vm); + virObjectLock(vm); event = virDomainEventRebootNewFromObj(vm); priv = vm->privateData; if (priv->agent) qemuAgentNotifyEvent(priv->agent, QEMU_AGENT_EVENT_RESET); - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) { qemuDriverLock(driver); @@ -550,7 +550,7 @@ qemuProcessFakeReboot(void *opaque) int ret = -1; VIR_DEBUG("vm=%p", vm); qemuDriverLock(driver); - virDomainObjLock(vm); + virObjectLock(vm); if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; @@ -599,7 +599,7 @@ cleanup: VIR_QEMU_PROCESS_KILL_FORCE)); } if (virObjectUnref(vm)) - virDomainObjUnlock(vm); + virObjectUnlock(vm); } if (event) qemuDomainEventQueue(driver, event); @@ -641,7 +641,7 @@ qemuProcessHandleShutdown(qemuMonitorPtr mon ATTRIBUTE_UNUSED, VIR_DEBUG("vm=%p", vm); - virDomainObjLock(vm); + virObjectLock(vm); priv = vm->privateData; if (priv->gotShutdown) { @@ -675,7 +675,7 @@ qemuProcessHandleShutdown(qemuMonitorPtr mon ATTRIBUTE_UNUSED, qemuProcessShutdownOrReboot(driver, vm); unlock: - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) { qemuDriverLock(driver); @@ -694,7 +694,7 @@ qemuProcessHandleStop(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virQEMUDriverPtr driver = qemu_driver; virDomainEventPtr event = NULL; - virDomainObjLock(vm); + virObjectLock(vm); if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { qemuDomainObjPrivatePtr priv = vm->privateData; @@ -723,7 +723,7 @@ qemuProcessHandleStop(qemuMonitorPtr mon ATTRIBUTE_UNUSED, } unlock: - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) { qemuDriverLock(driver); @@ -742,7 +742,7 @@ qemuProcessHandleResume(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virQEMUDriverPtr driver = qemu_driver; virDomainEventPtr event = NULL; - virDomainObjLock(vm); + virObjectLock(vm); if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) { qemuDomainObjPrivatePtr priv = vm->privateData; @@ -778,7 +778,7 @@ qemuProcessHandleResume(qemuMonitorPtr mon ATTRIBUTE_UNUSED, } unlock: - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) { qemuDriverLock(driver); @@ -798,7 +798,7 @@ qemuProcessHandleRTCChange(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virQEMUDriverPtr driver = qemu_driver; virDomainEventPtr event; - virDomainObjLock(vm); + virObjectLock(vm); event = virDomainEventRTCChangeNewFromObj(vm, offset); if (vm->def->clock.offset == VIR_DOMAIN_CLOCK_OFFSET_VARIABLE) @@ -807,7 +807,7 @@ qemuProcessHandleRTCChange(qemuMonitorPtr mon ATTRIBUTE_UNUSED, if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) VIR_WARN("unable to save domain status with RTC change"); - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) { qemuDriverLock(driver); @@ -828,7 +828,7 @@ qemuProcessHandleWatchdog(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virDomainEventPtr watchdogEvent = NULL; virDomainEventPtr lifecycleEvent = NULL; - virDomainObjLock(vm); + virObjectLock(vm); watchdogEvent = virDomainEventWatchdogNewFromObj(vm, action); if (action == VIR_DOMAIN_EVENT_WATCHDOG_PAUSE && @@ -872,7 +872,7 @@ qemuProcessHandleWatchdog(qemuMonitorPtr mon ATTRIBUTE_UNUSED, } if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (watchdogEvent || lifecycleEvent) { qemuDriverLock(driver); @@ -902,7 +902,7 @@ qemuProcessHandleIOError(qemuMonitorPtr mon ATTRIBUTE_UNUSED, const char *devAlias; virDomainDiskDefPtr disk; - virDomainObjLock(vm); + virObjectLock(vm); disk = qemuProcessFindDomainDiskByAlias(vm, diskAlias); if (disk) { @@ -934,7 +934,7 @@ qemuProcessHandleIOError(qemuMonitorPtr mon ATTRIBUTE_UNUSED, if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) VIR_WARN("Unable to save status on vm %s after IO error", vm->def->name); } - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (ioErrorEvent || ioErrorEvent2 || lifecycleEvent) { qemuDriverLock(driver); @@ -962,7 +962,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, const char *path; virDomainDiskDefPtr disk; - virDomainObjLock(vm); + virObjectLock(vm); disk = qemuProcessFindDomainDiskByAlias(vm, diskAlias); if (disk) { @@ -984,7 +984,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, disk->mirroring = true; } - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) { qemuDriverLock(driver); @@ -1049,9 +1049,9 @@ qemuProcessHandleGraphics(qemuMonitorPtr mon ATTRIBUTE_UNUSED, goto no_memory; } - virDomainObjLock(vm); + virObjectLock(vm); event = virDomainEventGraphicsNewFromObj(vm, phase, localAddr, remoteAddr, authScheme, subject); - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) { qemuDriverLock(driver); @@ -1102,7 +1102,7 @@ qemuProcessHandleTrayChange(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virDomainEventPtr event = NULL; virDomainDiskDefPtr disk; - virDomainObjLock(vm); + virObjectLock(vm); disk = qemuProcessFindDomainDiskByAlias(vm, devAlias); if (disk) { @@ -1121,7 +1121,7 @@ qemuProcessHandleTrayChange(qemuMonitorPtr mon ATTRIBUTE_UNUSED, } } - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) { qemuDriverLock(driver); @@ -1140,7 +1140,7 @@ qemuProcessHandlePMWakeup(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virDomainEventPtr event = NULL; virDomainEventPtr lifecycleEvent = NULL; - virDomainObjLock(vm); + virObjectLock(vm); event = virDomainEventPMWakeupNewFromObj(vm); /* Don't set domain status back to running if it wasn't paused @@ -1162,7 +1162,7 @@ qemuProcessHandlePMWakeup(qemuMonitorPtr mon ATTRIBUTE_UNUSED, } } - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event || lifecycleEvent) { qemuDriverLock(driver); @@ -1184,7 +1184,7 @@ qemuProcessHandlePMSuspend(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virDomainEventPtr event = NULL; virDomainEventPtr lifecycleEvent = NULL; - virDomainObjLock(vm); + virObjectLock(vm); event = virDomainEventPMSuspendNewFromObj(vm); if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { @@ -1208,7 +1208,7 @@ qemuProcessHandlePMSuspend(qemuMonitorPtr mon ATTRIBUTE_UNUSED, qemuAgentNotifyEvent(priv->agent, QEMU_AGENT_EVENT_SUSPEND); } - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event || lifecycleEvent) { qemuDriverLock(driver); @@ -1230,7 +1230,7 @@ qemuProcessHandleBalloonChange(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virQEMUDriverPtr driver = qemu_driver; virDomainEventPtr event; - virDomainObjLock(vm); + virObjectLock(vm); event = virDomainEventBalloonChangeNewFromObj(vm, actual); VIR_DEBUG("Updating balloon from %lld to %lld kb", @@ -1240,7 +1240,7 @@ qemuProcessHandleBalloonChange(qemuMonitorPtr mon ATTRIBUTE_UNUSED, if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) VIR_WARN("unable to save domain status with balloon change"); - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) { qemuDriverLock(driver); @@ -1259,7 +1259,7 @@ qemuProcessHandlePMSuspendDisk(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virDomainEventPtr event = NULL; virDomainEventPtr lifecycleEvent = NULL; - virDomainObjLock(vm); + virObjectLock(vm); event = virDomainEventPMSuspendDiskNewFromObj(vm); if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { @@ -1283,7 +1283,7 @@ qemuProcessHandlePMSuspendDisk(qemuMonitorPtr mon ATTRIBUTE_UNUSED, qemuAgentNotifyEvent(priv->agent, QEMU_AGENT_EVENT_SUSPEND); } - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event || lifecycleEvent) { qemuDriverLock(driver); @@ -1338,7 +1338,7 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm) virObjectRef(vm); ignore_value(virTimeMillisNow(&priv->monStart)); - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverUnlock(driver); mon = qemuMonitorOpen(vm, @@ -1347,7 +1347,7 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm) &monitorCallbacks); qemuDriverLock(driver); - virDomainObjLock(vm); + virObjectLock(vm); priv->monStart = 0; if (mon == NULL) { @@ -3209,7 +3209,7 @@ qemuProcessReconnect(void *opaque) VIR_FREE(data); qemuDriverLock(driver); - virDomainObjLock(obj); + virObjectLock(obj); VIR_DEBUG("Reconnect monitor to %p '%s'", obj, obj->def->name); @@ -3322,7 +3322,7 @@ endjob: obj = NULL; if (obj && virObjectUnref(obj)) - virDomainObjUnlock(obj); + virObjectUnlock(obj); qemuDriverUnlock(driver); @@ -3337,7 +3337,7 @@ error: if (obj) { if (!virDomainObjIsActive(obj)) { if (virObjectUnref(obj)) - virDomainObjUnlock(obj); + virObjectUnlock(obj); qemuDriverUnlock(driver); return; } @@ -3361,7 +3361,7 @@ error: if (!obj->persistent) qemuDomainRemoveInactive(driver, obj); else - virDomainObjUnlock(obj); + virObjectUnlock(obj); } } qemuDriverUnlock(driver); @@ -3410,7 +3410,7 @@ qemuProcessReconnectHelper(void *payload, * this early phase. */ - virDomainObjLock(obj); + virObjectLock(obj); qemuDomainObjRestoreJob(obj, &data->oldjob); @@ -3439,12 +3439,12 @@ qemuProcessReconnectHelper(void *payload, if (!obj->persistent) qemuDomainRemoveInactive(src->driver, obj); else - virDomainObjUnlock(obj); + virObjectUnlock(obj); } goto error; } - virDomainObjUnlock(obj); + virObjectUnlock(obj); return; @@ -4159,9 +4159,9 @@ qemuProcessKill(virQEMUDriverPtr driver, if (driver) { virObjectRef(vm); - virDomainObjUnlock(vm); + virObjectUnlock(vm); qemuDriverLock(driver); - virDomainObjLock(vm); + virObjectLock(vm); virObjectUnref(vm); } diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 3e082a4..15c1264 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -571,11 +571,11 @@ static int testOpenDefault(virConnectPtr conn) { domobj->persistent = 1; if (testDomainStartState(conn, domobj, VIR_DOMAIN_RUNNING_BOOTED) < 0) { - virDomainObjUnlock(domobj); + virObjectUnlock(domobj); goto error; } - virDomainObjUnlock(domobj); + virObjectUnlock(domobj); if (!(netdef = virNetworkDefParseString(defaultNetworkXML))) goto error; @@ -916,11 +916,11 @@ static int testOpenFromFile(virConnectPtr conn, dom->persistent = 1; if (testDomainStartState(conn, dom, VIR_DOMAIN_RUNNING_BOOTED) < 0) { - virDomainObjUnlock(dom); + virObjectUnlock(dom); goto error; } - virDomainObjUnlock(dom); + virObjectUnlock(dom); } VIR_FREE(domains); @@ -1258,7 +1258,7 @@ static int testDomainIsActive(virDomainPtr dom) cleanup: if (obj) - virDomainObjUnlock(obj); + virObjectUnlock(obj); return ret; } @@ -1279,7 +1279,7 @@ static int testDomainIsPersistent(virDomainPtr dom) cleanup: if (obj) - virDomainObjUnlock(obj); + virObjectUnlock(obj); return ret; } @@ -1329,7 +1329,7 @@ testDomainCreateXML(virConnectPtr conn, const char *xml, cleanup: if (dom) - virDomainObjUnlock(dom); + virObjectUnlock(dom); if (event) testDomainEventQueue(privconn, event); virDomainDefFree(def); @@ -1360,7 +1360,7 @@ static virDomainPtr testLookupDomainByID(virConnectPtr conn, cleanup: if (dom) - virDomainObjUnlock(dom); + virObjectUnlock(dom); return ret; } @@ -1386,7 +1386,7 @@ static virDomainPtr testLookupDomainByUUID(virConnectPtr conn, cleanup: if (dom) - virDomainObjUnlock(dom); + virObjectUnlock(dom); return ret; } @@ -1412,7 +1412,7 @@ static virDomainPtr testLookupDomainByName(virConnectPtr conn, cleanup: if (dom) - virDomainObjUnlock(dom); + virObjectUnlock(dom); return ret; } @@ -1460,7 +1460,7 @@ static int testDestroyDomain(virDomainPtr domain) ret = 0; cleanup: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); if (event) testDomainEventQueue(privconn, event); testDriverUnlock(privconn); @@ -1499,7 +1499,7 @@ static int testResumeDomain(virDomainPtr domain) cleanup: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); if (event) { testDriverLock(privconn); testDomainEventQueue(privconn, event); @@ -1541,7 +1541,7 @@ static int testPauseDomain(virDomainPtr domain) cleanup: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); if (event) { testDriverLock(privconn); @@ -1590,7 +1590,7 @@ static int testShutdownDomainFlags(virDomainPtr domain, ret = 0; cleanup: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); if (event) testDomainEventQueue(privconn, event); testDriverUnlock(privconn); @@ -1666,7 +1666,7 @@ static int testRebootDomain(virDomainPtr domain, ret = 0; cleanup: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); if (event) testDomainEventQueue(privconn, event); testDriverUnlock(privconn); @@ -1706,7 +1706,7 @@ static int testGetDomainInfo(virDomainPtr domain, cleanup: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); return ret; } @@ -1737,7 +1737,7 @@ testDomainGetState(virDomainPtr domain, cleanup: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); return ret; } @@ -1838,7 +1838,7 @@ cleanup: unlink(path); } if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); if (event) testDomainEventQueue(privconn, event); testDriverUnlock(privconn); @@ -1945,7 +1945,7 @@ cleanup: VIR_FREE(xml); VIR_FORCE_CLOSE(fd); if (dom) - virDomainObjUnlock(dom); + virObjectUnlock(dom); if (event) testDomainEventQueue(privconn, event); testDriverUnlock(privconn); @@ -2015,7 +2015,7 @@ static int testDomainCoreDump(virDomainPtr domain, cleanup: VIR_FORCE_CLOSE(fd); if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); if (event) testDomainEventQueue(privconn, event); testDriverUnlock(privconn); @@ -2048,7 +2048,7 @@ static unsigned long long testGetMaxMemory(virDomainPtr domain) { cleanup: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); return ret; } @@ -2075,7 +2075,7 @@ static int testSetMaxMemory(virDomainPtr domain, cleanup: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); return ret; } @@ -2106,7 +2106,7 @@ static int testSetMemory(virDomainPtr domain, cleanup: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); return ret; } @@ -2144,7 +2144,7 @@ testDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -2242,7 +2242,7 @@ testDomainSetVcpusFlags(virDomainPtr domain, unsigned int nrCpus, cleanup: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); return ret; } @@ -2336,7 +2336,7 @@ static int testDomainGetVcpus(virDomainPtr domain, ret = maxinfo; cleanup: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); return ret; } @@ -2393,7 +2393,7 @@ static int testDomainPinVcpu(virDomainPtr domain, ret = 0; cleanup: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); return ret; } @@ -2424,7 +2424,7 @@ static char *testDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) cleanup: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); return ret; } @@ -2493,7 +2493,7 @@ static virDomainPtr testDomainDefineXML(virConnectPtr conn, cleanup: virDomainDefFree(def); if (dom) - virDomainObjUnlock(dom); + virObjectUnlock(dom); if (event) testDomainEventQueue(privconn, event); testDriverUnlock(privconn); @@ -2562,7 +2562,7 @@ static int testDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) { cleanup: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); if (event) testDomainEventQueue(privconn, event); testDriverUnlock(privconn); @@ -2607,7 +2607,7 @@ static int testDomainUndefineFlags(virDomainPtr domain, cleanup: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); if (event) testDomainEventQueue(privconn, event); testDriverUnlock(privconn); @@ -2641,7 +2641,7 @@ static int testDomainGetAutostart(virDomainPtr domain, cleanup: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); return ret; } @@ -2668,7 +2668,7 @@ static int testDomainSetAutostart(virDomainPtr domain, cleanup: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); return ret; } @@ -2720,7 +2720,7 @@ testDomainGetSchedulerParamsFlags(virDomainPtr domain, cleanup: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); return ret; } @@ -2770,7 +2770,7 @@ testDomainSetSchedulerParamsFlags(virDomainPtr domain, cleanup: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); return ret; } @@ -2825,7 +2825,7 @@ static int testDomainBlockStats(virDomainPtr domain, ret = 0; error: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); return ret; } @@ -2883,7 +2883,7 @@ static int testDomainInterfaceStats(virDomainPtr domain, ret = 0; error: if (privdom) - virDomainObjUnlock(privdom); + virObjectUnlock(privdom); return ret; } diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 448d292..c6fef69 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -185,7 +185,7 @@ umlAutostartDomain(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaqu virDomainObjPtr vm = payload; const struct umlAutostartData *data = opaque; - virDomainObjLock(vm); + virObjectLock(vm); if (vm->autostart && !virDomainObjIsActive(vm)) { int ret; @@ -205,7 +205,7 @@ umlAutostartDomain(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaqu umlDomainEventQueue(data->driver, event); } } - virDomainObjUnlock(vm); + virObjectUnlock(vm); } static void @@ -345,7 +345,7 @@ reread: if (e->mask & IN_DELETE) { VIR_DEBUG("Got inotify domain shutdown '%s'", name); if (!virDomainObjIsActive(dom)) { - virDomainObjUnlock(dom); + virObjectUnlock(dom); continue; } @@ -362,12 +362,12 @@ reread: } else if (e->mask & (IN_CREATE | IN_MODIFY)) { VIR_DEBUG("Got inotify domain startup '%s'", name); if (virDomainObjIsActive(dom)) { - virDomainObjUnlock(dom); + virObjectUnlock(dom); continue; } if (umlReadPidFile(driver, dom) < 0) { - virDomainObjUnlock(dom); + virObjectUnlock(dom); continue; } @@ -409,7 +409,7 @@ reread: } } if (dom) - virDomainObjUnlock(dom); + virObjectUnlock(dom); } cleanup: @@ -606,12 +606,12 @@ umlShutdownOneVM(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque) virDomainObjPtr dom = payload; struct uml_driver *driver = opaque; - virDomainObjLock(dom); + virObjectLock(dom); if (virDomainObjIsActive(dom)) { umlShutdownVMDaemon(driver, dom, VIR_DOMAIN_SHUTOFF_SHUTDOWN); virDomainAuditStop(dom, "shutdown"); } - virDomainObjUnlock(dom); + virObjectUnlock(dom); } /** @@ -705,7 +705,7 @@ static void umlProcessAutoDestroyDom(void *payload, virDomainRemoveInactive(&data->driver->domains, dom); if (dom) - virDomainObjUnlock(dom); + virObjectUnlock(dom); if (event) umlDomainEventQueue(data->driver, event); virHashRemoveEntry(data->driver->autodestroy, uuidstr); @@ -1320,7 +1320,7 @@ static virDomainPtr umlDomainLookupByID(virConnectPtr conn, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return dom; } @@ -1344,7 +1344,7 @@ static virDomainPtr umlDomainLookupByUUID(virConnectPtr conn, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return dom; } @@ -1368,7 +1368,7 @@ static virDomainPtr umlDomainLookupByName(virConnectPtr conn, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return dom; } @@ -1390,7 +1390,7 @@ static int umlDomainIsActive(virDomainPtr dom) cleanup: if (obj) - virDomainObjUnlock(obj); + virObjectUnlock(obj); return ret; } @@ -1412,7 +1412,7 @@ static int umlDomainIsPersistent(virDomainPtr dom) cleanup: if (obj) - virDomainObjUnlock(obj); + virObjectUnlock(obj); return ret; } @@ -1433,7 +1433,7 @@ static int umlDomainIsUpdated(virDomainPtr dom) cleanup: if (obj) - virDomainObjUnlock(obj); + virObjectUnlock(obj); return ret; } @@ -1526,7 +1526,7 @@ static virDomainPtr umlDomainCreate(virConnectPtr conn, const char *xml, cleanup: virDomainDefFree(def); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) umlDomainEventQueue(driver, event); umlDriverUnlock(driver); @@ -1564,7 +1564,7 @@ static int umlDomainShutdownFlags(virDomainPtr dom, cleanup: VIR_FREE(info); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -1607,7 +1607,7 @@ umlDomainDestroyFlags(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) umlDomainEventQueue(driver, event); umlDriverUnlock(driver); @@ -1640,7 +1640,7 @@ static char *umlDomainGetOSType(virDomainPtr dom) { cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return type; } @@ -1668,7 +1668,7 @@ umlDomainGetMaxMemory(virDomainPtr dom) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -1701,7 +1701,7 @@ static int umlDomainSetMaxMemory(virDomainPtr dom, unsigned long newmax) { cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -1740,7 +1740,7 @@ static int umlDomainSetMemory(virDomainPtr dom, unsigned long newmem) { cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -1779,7 +1779,7 @@ static int umlDomainGetInfo(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -1811,7 +1811,7 @@ umlDomainGetState(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -1841,7 +1841,7 @@ static char *umlDomainGetXMLDesc(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -1897,7 +1897,7 @@ static int umlDomainStartWithFlags(virDomainPtr dom, unsigned int flags) { cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); if (event) umlDomainEventQueue(driver, event); umlDriverUnlock(driver); @@ -1944,7 +1944,7 @@ static virDomainPtr umlDomainDefine(virConnectPtr conn, const char *xml) { cleanup: virDomainDefFree(def); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); umlDriverUnlock(driver); return dom; } @@ -1986,7 +1986,7 @@ static int umlDomainUndefineFlags(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); umlDriverUnlock(driver); return ret; } @@ -2099,7 +2099,7 @@ cleanup: virDomainDeviceDefFree(dev); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); umlDriverUnlock(driver); return ret; } @@ -2211,7 +2211,7 @@ static int umlDomainDetachDevice(virDomainPtr dom, const char *xml) { cleanup: virDomainDeviceDefFree(dev); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); umlDriverUnlock(driver); return ret; } @@ -2254,7 +2254,7 @@ static int umlDomainGetAutostart(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); umlDriverUnlock(driver); return ret; } @@ -2320,7 +2320,7 @@ cleanup: VIR_FREE(configFile); VIR_FREE(autostartLink); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); umlDriverUnlock(driver); return ret; } @@ -2388,7 +2388,7 @@ umlDomainBlockPeek(virDomainPtr dom, cleanup: VIR_FORCE_CLOSE(fd); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -2458,7 +2458,7 @@ umlDomainOpenConsole(virDomainPtr dom, ret = 0; cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); umlDriverUnlock(driver); return ret; } diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index fd9c473..e8a66de 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -198,7 +198,7 @@ vmwareLoadDomains(struct vmware_driver *driver) VIR_DOMAIN_RUNNING_UNKNOWN); vm->persistent = 1; - virDomainObjUnlock(vm); + virObjectUnlock(vm); vmdef = NULL; vm = NULL; diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index 9c81df8..5c0e9ca 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -365,7 +365,7 @@ vmwareDomainDefineXML(virConnectPtr conn, const char *xml) VIR_FREE(fileName); VIR_FREE(vmxPath); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); vmwareDriverUnlock(driver); return dom; } @@ -410,7 +410,7 @@ vmwareDomainShutdownFlags(virDomainPtr dom, ret = 0; cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); vmwareDriverUnlock(driver); return ret; } @@ -466,7 +466,7 @@ vmwareDomainSuspend(virDomainPtr dom) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -515,7 +515,7 @@ vmwareDomainResume(virDomainPtr dom) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -563,7 +563,7 @@ vmwareDomainReboot(virDomainPtr dom, unsigned int flags) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -635,7 +635,7 @@ cleanup: VIR_FREE(vmx); VIR_FREE(vmxPath); if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); vmwareDriverUnlock(driver); return dom; } @@ -673,7 +673,7 @@ vmwareDomainCreateWithFlags(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); vmwareDriverUnlock(driver); return ret; } @@ -726,7 +726,7 @@ vmwareDomainUndefineFlags(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); vmwareDriverUnlock(driver); return ret; } @@ -759,7 +759,7 @@ vmwareDomainLookupByID(virConnectPtr conn, int id) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return dom; } @@ -784,7 +784,7 @@ vmwareGetOSType(virDomainPtr dom) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -811,7 +811,7 @@ vmwareDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return dom; } @@ -837,7 +837,7 @@ vmwareDomainLookupByName(virConnectPtr conn, const char *name) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return dom; } @@ -859,7 +859,7 @@ vmwareDomainIsActive(virDomainPtr dom) cleanup: if (obj) - virDomainObjUnlock(obj); + virObjectUnlock(obj); return ret; } @@ -882,7 +882,7 @@ vmwareDomainIsPersistent(virDomainPtr dom) cleanup: if (obj) - virDomainObjUnlock(obj); + virObjectUnlock(obj); return ret; } @@ -910,7 +910,7 @@ vmwareDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -948,9 +948,9 @@ static void vmwareDomainObjListUpdateDomain(void *payload, const void *name ATTR { struct vmware_driver *driver = data; virDomainObjPtr vm = payload; - virDomainObjLock(vm); + virObjectLock(vm); ignore_value(vmwareUpdateVMStatus(driver, vm)); - virDomainObjUnlock(vm); + virObjectUnlock(vm); } static void @@ -1045,7 +1045,7 @@ vmwareDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } @@ -1079,7 +1079,7 @@ vmwareDomainGetState(virDomainPtr dom, cleanup: if (vm) - virDomainObjUnlock(vm); + virObjectUnlock(vm); return ret; } diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index 78eab29..eb9174d 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -364,7 +364,7 @@ void qemuMonitorTestFree(qemuMonitorTestPtr test) virObjectUnref(test->server); if (test->mon) { - qemuMonitorUnlock(test->mon); + virObjectUnlock(test->mon); qemuMonitorClose(test->mon); } @@ -496,7 +496,7 @@ qemuMonitorTestPtr qemuMonitorTestNew(bool json, virCapsPtr caps) json ? 1 : 0, &qemuCallbacks))) goto error; - qemuMonitorLock(test->mon); + virObjectLock(test->mon); if (virNetSocketAccept(test->server, &test->client) < 0) goto error; -- 1.8.0.1
On 01/11/2013 05:13 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The virDomainObj, qemuAgent, qemuMonitor, lxcMonitor classes all require a mutex, so can be switched to use virObjectLockable ---
27 files changed, 481 insertions(+), 579 deletions(-)
Big, but looks mostly mechanical.
+++ b/src/conf/domain_conf.h @@ -1858,9 +1858,7 @@ struct _virDomainStateReason { typedef struct _virDomainObj virDomainObj; typedef virDomainObj *virDomainObjPtr; struct _virDomainObj { - virObject object; - - virMutex lock; + virObjectLockable parent;
A few of these hunks form the real meat of the change, with everything else being fallout of using the benefits of the new parent class.
@@ -733,26 +720,18 @@ qemuAgentOpen(virDomainObjPtr vm, if (qemuAgentInitialize() < 0) return NULL;
- if (!(mon = virObjectNew(qemuAgentClass))) + if (!(mon = virObjectLockableNew(qemuAgentClass))) return NULL;
- if (virMutexInit(&mon->lock) < 0) { - virReportSystemError(errno, "%s", - _("cannot initialize monitor mutex")); - VIR_FREE(mon); - return NULL; - } + mon->fd = -1;
Yep, I can see why you had to hoist this...
if (virCondInit(&mon->notify) < 0) { virReportSystemError(errno, "%s", _("cannot initialize monitor condition")); - virMutexDestroy(&mon->lock); - VIR_FREE(mon); + virObjectUnref(mon); return NULL;
...when you replaced ad hoc cleanup by the parent class cleanup.
} - mon->fd = -1; mon->vm = vm; mon->cb = cb; - qemuAgentLock(mon);
switch (config->type) { case VIR_DOMAIN_CHR_TYPE_UNIX: @@ -791,7 +770,6 @@ qemuAgentOpen(virDomainObjPtr vm, virObjectRef(mon);
VIR_DEBUG("New mon %p fd =%d watch=%d", mon, mon->fd, mon->watch); - qemuAgentUnlock(mon);
Question - was it really safe to remove the lock around this section of code, considering that you were previously handing 'mon' to virEventAddHandle() only inside the lock? That is, now that locks are dropped, is there any possibility that the handle you just added could fire in the window between virEventAddHandle() and virObjectRef(), such that you end up calling virObjectFreeCallback too soon and we end up calling virObjectRef on a stale object?
+++ b/src/qemu/qemu_domain.c @@ -786,12 +786,12 @@ retry: }
while (!nested && !qemuDomainNestedJobAllowed(priv, job)) { - if (virCondWaitUntil(&priv->job.asyncCond, &obj->lock, then) < 0) + if (virCondWaitUntil(&priv->job.asyncCond, &obj->parent.lock, then) < 0)
Feels a bit weird accessing the parent lock in this manner; maybe a virObjectGetLock(&obj) accessor would have been easier to read. But I'm not too concerned; this works as-is. Assuming the dropped qemuAgentLock() was safe, ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Tue, Jan 15, 2013 at 04:25:54PM -0700, Eric Blake wrote:
On 01/11/2013 05:13 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The virDomainObj, qemuAgent, qemuMonitor, lxcMonitor classes all require a mutex, so can be switched to use virObjectLockable ---
27 files changed, 481 insertions(+), 579 deletions(-)
Big, but looks mostly mechanical.
+++ b/src/conf/domain_conf.h @@ -1858,9 +1858,7 @@ struct _virDomainStateReason { typedef struct _virDomainObj virDomainObj; typedef virDomainObj *virDomainObjPtr; struct _virDomainObj { - virObject object; - - virMutex lock; + virObjectLockable parent;
A few of these hunks form the real meat of the change, with everything else being fallout of using the benefits of the new parent class.
@@ -733,26 +720,18 @@ qemuAgentOpen(virDomainObjPtr vm, if (qemuAgentInitialize() < 0) return NULL;
- if (!(mon = virObjectNew(qemuAgentClass))) + if (!(mon = virObjectLockableNew(qemuAgentClass))) return NULL;
- if (virMutexInit(&mon->lock) < 0) { - virReportSystemError(errno, "%s", - _("cannot initialize monitor mutex")); - VIR_FREE(mon); - return NULL; - } + mon->fd = -1;
Yep, I can see why you had to hoist this...
if (virCondInit(&mon->notify) < 0) { virReportSystemError(errno, "%s", _("cannot initialize monitor condition")); - virMutexDestroy(&mon->lock); - VIR_FREE(mon); + virObjectUnref(mon); return NULL;
...when you replaced ad hoc cleanup by the parent class cleanup.
} - mon->fd = -1; mon->vm = vm; mon->cb = cb; - qemuAgentLock(mon);
switch (config->type) { case VIR_DOMAIN_CHR_TYPE_UNIX: @@ -791,7 +770,6 @@ qemuAgentOpen(virDomainObjPtr vm, virObjectRef(mon);
VIR_DEBUG("New mon %p fd =%d watch=%d", mon, mon->fd, mon->watch); - qemuAgentUnlock(mon);
Question - was it really safe to remove the lock around this section of code, considering that you were previously handing 'mon' to virEventAddHandle() only inside the lock? That is, now that locks are dropped, is there any possibility that the handle you just added could fire in the window between virEventAddHandle() and virObjectRef(), such that you end up calling virObjectFreeCallback too soon and we end up calling virObjectRef on a stale object?
I believe it is safe, but for sanity I'll move the ref. Also the same code in QEMU monitor.
+++ b/src/qemu/qemu_domain.c @@ -786,12 +786,12 @@ retry: }
while (!nested && !qemuDomainNestedJobAllowed(priv, job)) { - if (virCondWaitUntil(&priv->job.asyncCond, &obj->lock, then) < 0) + if (virCondWaitUntil(&priv->job.asyncCond, &obj->parent.lock, then) < 0)
Feels a bit weird accessing the parent lock in this manner; maybe a virObjectGetLock(&obj) accessor would have been easier to read. But I'm not too concerned; this works as-is.
Might be worth adding a virObjectLockableWait() call which accepts a virCondPtr arg. Can do this as a followup
Assuming the dropped qemuAgentLock() was safe,
ACK.
Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
From: "Daniel P. Berrange" <berrange@redhat.com> --- src/rpc/virkeepalive.c | 55 +++++----------- src/rpc/virnetclient.c | 128 +++++++++++++++-------------------- src/rpc/virnetclientstream.c | 67 ++++++++----------- src/rpc/virnetsaslcontext.c | 109 ++++++++++-------------------- src/rpc/virnetserver.c | 117 ++++++++++++++------------------ src/rpc/virnetserverclient.c | 154 +++++++++++++++++++------------------------ src/rpc/virnetsocket.c | 125 ++++++++++++++++------------------- src/rpc/virnetsshsession.c | 82 +++++++++++------------ src/rpc/virnettlscontext.c | 64 +++++++----------- 9 files changed, 366 insertions(+), 535 deletions(-) diff --git a/src/rpc/virkeepalive.c b/src/rpc/virkeepalive.c index 04962d4..5fe9e8b 100644 --- a/src/rpc/virkeepalive.c +++ b/src/rpc/virkeepalive.c @@ -35,9 +35,7 @@ #define VIR_FROM_THIS VIR_FROM_RPC struct _virKeepAlive { - virObject object; - - virMutex lock; + virObjectLockable parent; int interval; unsigned int count; @@ -58,7 +56,7 @@ static void virKeepAliveDispose(void *obj); static int virKeepAliveOnceInit(void) { - if (!(virKeepAliveClass = virClassNew(virClassForObject(), + if (!(virKeepAliveClass = virClassNew(virClassForObjectLockable(), "virKeepAlive", sizeof(virKeepAlive), virKeepAliveDispose))) @@ -69,19 +67,6 @@ static int virKeepAliveOnceInit(void) VIR_ONCE_GLOBAL_INIT(virKeepAlive) -static void -virKeepAliveLock(virKeepAlivePtr ka) -{ - virMutexLock(&ka->lock); -} - -static void -virKeepAliveUnlock(virKeepAlivePtr ka) -{ - virMutexUnlock(&ka->lock); -} - - static virNetMessagePtr virKeepAliveMessage(virKeepAlivePtr ka, int proc) { @@ -174,7 +159,7 @@ virKeepAliveTimer(int timer ATTRIBUTE_UNUSED, void *opaque) bool dead; void *client; - virKeepAliveLock(ka); + virObjectLock(ka); client = ka->client; dead = virKeepAliveTimerInternal(ka, &msg); @@ -183,7 +168,7 @@ virKeepAliveTimer(int timer ATTRIBUTE_UNUSED, void *opaque) goto cleanup; virObjectRef(ka); - virKeepAliveUnlock(ka); + virObjectUnlock(ka); if (dead) { ka->deadCB(client); @@ -192,11 +177,11 @@ virKeepAliveTimer(int timer ATTRIBUTE_UNUSED, void *opaque) virNetMessageFree(msg); } - virKeepAliveLock(ka); + virObjectLock(ka); virObjectUnref(ka); cleanup: - virKeepAliveUnlock(ka); + virObjectUnlock(ka); } @@ -215,14 +200,9 @@ virKeepAliveNew(int interval, if (virKeepAliveInitialize() < 0) return NULL; - if (!(ka = virObjectNew(virKeepAliveClass))) + if (!(ka = virObjectLockableNew(virKeepAliveClass))) return NULL; - if (virMutexInit(&ka->lock) < 0) { - VIR_FREE(ka); - return NULL; - } - ka->interval = interval; ka->count = count; ka->countToDeath = count; @@ -245,7 +225,6 @@ virKeepAliveDispose(void *obj) { virKeepAlivePtr ka = obj; - virMutexDestroy(&ka->lock); ka->freeCB(ka->client); } @@ -260,7 +239,7 @@ virKeepAliveStart(virKeepAlivePtr ka, int timeout; time_t now; - virKeepAliveLock(ka); + virObjectLock(ka); if (ka->timer >= 0) { VIR_DEBUG("Keepalive messages already enabled"); @@ -306,7 +285,7 @@ virKeepAliveStart(virKeepAlivePtr ka, ret = 0; cleanup: - virKeepAliveUnlock(ka); + virObjectUnlock(ka); return ret; } @@ -314,7 +293,7 @@ cleanup: void virKeepAliveStop(virKeepAlivePtr ka) { - virKeepAliveLock(ka); + virObjectLock(ka); PROBE(RPC_KEEPALIVE_STOP, "ka=%p client=%p", @@ -325,7 +304,7 @@ virKeepAliveStop(virKeepAlivePtr ka) ka->timer = -1; } - virKeepAliveUnlock(ka); + virObjectUnlock(ka); } @@ -337,7 +316,7 @@ virKeepAliveTimeout(virKeepAlivePtr ka) if (!ka) return -1; - virKeepAliveLock(ka); + virObjectLock(ka); if (ka->interval <= 0 || ka->intervalStart == 0) { timeout = -1; @@ -347,7 +326,7 @@ virKeepAliveTimeout(virKeepAlivePtr ka) timeout = 0; } - virKeepAliveUnlock(ka); + virObjectUnlock(ka); if (timeout < 0) return -1; @@ -366,9 +345,9 @@ virKeepAliveTrigger(virKeepAlivePtr ka, if (!ka) return false; - virKeepAliveLock(ka); + virObjectLock(ka); dead = virKeepAliveTimerInternal(ka, msg); - virKeepAliveUnlock(ka); + virObjectUnlock(ka); return dead; } @@ -388,7 +367,7 @@ virKeepAliveCheckMessage(virKeepAlivePtr ka, if (!ka) return false; - virKeepAliveLock(ka); + virObjectLock(ka); ka->countToDeath = ka->count; ka->lastPacketReceived = ka->intervalStart = time(NULL); @@ -420,7 +399,7 @@ virKeepAliveCheckMessage(virKeepAlivePtr ka, if (ka->timer >= 0) virEventUpdateTimeout(ka->timer, ka->interval * 1000); - virKeepAliveUnlock(ka); + virObjectUnlock(ka); return ret; } diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index f281548..1bcd765 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -63,9 +63,7 @@ struct _virNetClientCall { struct _virNetClient { - virObject object; - - virMutex lock; + virObjectLockable parent; virNetSocketPtr sock; bool asyncIO; @@ -136,28 +134,16 @@ static void virNetClientCloseInternal(virNetClientPtr client, int reason); -static void virNetClientLock(virNetClientPtr client) -{ - virMutexLock(&client->lock); -} - - -static void virNetClientUnlock(virNetClientPtr client) -{ - virMutexUnlock(&client->lock); -} - - void virNetClientSetCloseCallback(virNetClientPtr client, virNetClientCloseFunc cb, void *opaque, virFreeCallback ff) { - virNetClientLock(client); + virObjectLock(client); client->closeCb = cb; client->closeOpaque = opaque; client->closeFf = ff; - virNetClientUnlock(client); + virObjectUnlock(client); } @@ -261,9 +247,9 @@ virNetClientKeepAliveIsSupported(virNetClientPtr client) { bool supported; - virNetClientLock(client); + virObjectLock(client); supported = !!client->keepalive; - virNetClientUnlock(client); + virObjectUnlock(client); return supported; } @@ -275,9 +261,9 @@ virNetClientKeepAliveStart(virNetClientPtr client, { int ret; - virNetClientLock(client); + virObjectLock(client); ret = virKeepAliveStart(client->keepalive, interval, count); - virNetClientUnlock(client); + virObjectUnlock(client); return ret; } @@ -285,9 +271,9 @@ virNetClientKeepAliveStart(virNetClientPtr client, void virNetClientKeepAliveStop(virNetClientPtr client) { - virNetClientLock(client); + virObjectLock(client); virKeepAliveStop(client->keepalive); - virNetClientUnlock(client); + virObjectUnlock(client); } static void @@ -323,14 +309,9 @@ static virNetClientPtr virNetClientNew(virNetSocketPtr sock, goto error; } - if (!(client = virObjectNew(virNetClientClass))) + if (!(client = virObjectLockableNew(virNetClientClass))) goto error; - if (virMutexInit(&client->lock) < 0) { - VIR_FREE(client); - goto error; - } - client->sock = sock; client->wakeupReadFD = wakeupFD[0]; client->wakeupSendFD = wakeupFD[1]; @@ -583,9 +564,9 @@ int virNetClientRegisterKeepAlive(virNetClientPtr client) int virNetClientGetFD(virNetClientPtr client) { int fd; - virNetClientLock(client); + virObjectLock(client); fd = virNetSocketGetFD(client->sock); - virNetClientUnlock(client); + virObjectUnlock(client); return fd; } @@ -593,9 +574,9 @@ int virNetClientGetFD(virNetClientPtr client) int virNetClientDupFD(virNetClientPtr client, bool cloexec) { int fd; - virNetClientLock(client); + virObjectLock(client); fd = virNetSocketDupFD(client->sock, cloexec); - virNetClientUnlock(client); + virObjectUnlock(client); return fd; } @@ -603,9 +584,9 @@ int virNetClientDupFD(virNetClientPtr client, bool cloexec) bool virNetClientHasPassFD(virNetClientPtr client) { bool hasPassFD; - virNetClientLock(client); + virObjectLock(client); hasPassFD = virNetSocketHasPassFD(client->sock); - virNetClientUnlock(client); + virObjectUnlock(client); return hasPassFD; } @@ -639,8 +620,7 @@ void virNetClientDispose(void *obj) virNetMessageClear(&client->msg); - virNetClientUnlock(client); - virMutexDestroy(&client->lock); + virObjectUnlock(client); } @@ -685,7 +665,7 @@ virNetClientCloseLocked(virNetClientPtr client) void *closeOpaque = client->closeOpaque; int closeReason = client->closeReason; virObjectRef(client); - virNetClientUnlock(client); + virObjectUnlock(client); if (ka) { virKeepAliveStop(ka); @@ -694,7 +674,7 @@ virNetClientCloseLocked(virNetClientPtr client) if (closeCb) closeCb(client, closeReason, closeOpaque); - virNetClientLock(client); + virObjectLock(client); virObjectUnref(client); } } @@ -711,7 +691,7 @@ static void virNetClientCloseInternal(virNetClientPtr client, client->wantClose) return; - virNetClientLock(client); + virObjectLock(client); virNetClientMarkClose(client, reason); @@ -730,7 +710,7 @@ static void virNetClientCloseInternal(virNetClientPtr client, virNetClientIOEventLoopPassTheBuck(client, NULL); } - virNetClientUnlock(client); + virObjectUnlock(client); } @@ -744,10 +724,10 @@ void virNetClientClose(virNetClientPtr client) void virNetClientSetSASLSession(virNetClientPtr client, virNetSASLSessionPtr sasl) { - virNetClientLock(client); + virObjectLock(client); client->sasl = virObjectRef(sasl); virNetSocketSetSASLSession(client->sock, client->sasl); - virNetClientUnlock(client); + virObjectUnlock(client); } #endif @@ -771,7 +751,7 @@ int virNetClientSetTLSSession(virNetClientPtr client, # endif sigaddset(&blockedsigs, SIGPIPE); - virNetClientLock(client); + virObjectLock(client); if (!(client->tls = virNetTLSSessionNew(tls, client->hostname))) @@ -846,13 +826,13 @@ int virNetClientSetTLSSession(virNetClientPtr client, goto error; } - virNetClientUnlock(client); + virObjectUnlock(client); return 0; error: virObjectUnref(client->tls); client->tls = NULL; - virNetClientUnlock(client); + virObjectUnlock(client); return -1; } #endif @@ -860,7 +840,7 @@ error: bool virNetClientIsEncrypted(virNetClientPtr client) { bool ret = false; - virNetClientLock(client); + virObjectLock(client); #if HAVE_GNUTLS if (client->tls) ret = true; @@ -869,7 +849,7 @@ bool virNetClientIsEncrypted(virNetClientPtr client) if (client->sasl) ret = true; #endif - virNetClientUnlock(client); + virObjectUnlock(client); return ret; } @@ -881,9 +861,9 @@ bool virNetClientIsOpen(virNetClientPtr client) if (!client) return false; - virNetClientLock(client); + virObjectLock(client); ret = client->sock && !client->wantClose; - virNetClientUnlock(client); + virObjectUnlock(client); return ret; } @@ -891,19 +871,19 @@ bool virNetClientIsOpen(virNetClientPtr client) int virNetClientAddProgram(virNetClientPtr client, virNetClientProgramPtr prog) { - virNetClientLock(client); + virObjectLock(client); if (VIR_EXPAND_N(client->programs, client->nprograms, 1) < 0) goto no_memory; client->programs[client->nprograms-1] = virObjectRef(prog); - virNetClientUnlock(client); + virObjectUnlock(client); return 0; no_memory: virReportOOMError(); - virNetClientUnlock(client); + virObjectUnlock(client); return -1; } @@ -911,19 +891,19 @@ no_memory: int virNetClientAddStream(virNetClientPtr client, virNetClientStreamPtr st) { - virNetClientLock(client); + virObjectLock(client); if (VIR_EXPAND_N(client->streams, client->nstreams, 1) < 0) goto no_memory; client->streams[client->nstreams-1] = virObjectRef(st); - virNetClientUnlock(client); + virObjectUnlock(client); return 0; no_memory: virReportOOMError(); - virNetClientUnlock(client); + virObjectUnlock(client); return -1; } @@ -931,7 +911,7 @@ no_memory: void virNetClientRemoveStream(virNetClientPtr client, virNetClientStreamPtr st) { - virNetClientLock(client); + virObjectLock(client); size_t i; for (i = 0 ; i < client->nstreams ; i++) { if (client->streams[i] == st) @@ -953,7 +933,7 @@ void virNetClientRemoveStream(virNetClientPtr client, virObjectUnref(st); cleanup: - virNetClientUnlock(client); + virObjectUnlock(client); } @@ -971,10 +951,10 @@ const char *virNetClientRemoteAddrString(virNetClientPtr client) int virNetClientGetTLSKeySize(virNetClientPtr client) { int ret = 0; - virNetClientLock(client); + virObjectLock(client); if (client->tls) ret = virNetTLSSessionGetKeySize(client->tls); - virNetClientUnlock(client); + virObjectUnlock(client); return ret; } #endif @@ -1522,7 +1502,7 @@ static int virNetClientIOEventLoop(virNetClientPtr client, /* Release lock while poll'ing so other threads * can stuff themselves on the queue */ - virNetClientUnlock(client); + virObjectUnlock(client); /* Block SIGWINCH from interrupting poll in curses programs, * then restore the original signal mask again immediately @@ -1546,7 +1526,7 @@ static int virNetClientIOEventLoop(virNetClientPtr client, ignore_value(pthread_sigmask(SIG_SETMASK, &oldmask, NULL)); - virNetClientLock(client); + virObjectLock(client); if (ret < 0) { virReportSystemError(errno, @@ -1762,7 +1742,7 @@ static int virNetClientIO(virNetClientPtr client, VIR_DEBUG("Going to sleep head=%p call=%p", client->waitDispatch, thiscall); /* Go to sleep while other thread is working... */ - if (virCondWait(&thiscall->cond, &client->lock) < 0) { + if (virCondWait(&thiscall->cond, &client->parent.lock) < 0) { virNetClientCallRemove(&client->waitDispatch, thiscall); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to wait on condition")); @@ -1834,7 +1814,7 @@ void virNetClientIncomingEvent(virNetSocketPtr sock, { virNetClientPtr client = opaque; - virNetClientLock(client); + virObjectLock(client); VIR_DEBUG("client=%p wantclose=%d", client, client ? client->wantClose : false); @@ -1876,7 +1856,7 @@ void virNetClientIncomingEvent(virNetSocketPtr sock, done: if (client->wantClose) virNetClientCloseLocked(client); - virNetClientUnlock(client); + virObjectUnlock(client); } @@ -2012,9 +1992,9 @@ int virNetClientSendWithReply(virNetClientPtr client, virNetMessagePtr msg) { int ret; - virNetClientLock(client); + virObjectLock(client); ret = virNetClientSendInternal(client, msg, true, false); - virNetClientUnlock(client); + virObjectUnlock(client); if (ret < 0) return -1; return 0; @@ -2035,9 +2015,9 @@ int virNetClientSendNoReply(virNetClientPtr client, virNetMessagePtr msg) { int ret; - virNetClientLock(client); + virObjectLock(client); ret = virNetClientSendInternal(client, msg, false, false); - virNetClientUnlock(client); + virObjectUnlock(client); if (ret < 0) return -1; return 0; @@ -2058,9 +2038,9 @@ int virNetClientSendNonBlock(virNetClientPtr client, virNetMessagePtr msg) { int ret; - virNetClientLock(client); + virObjectLock(client); ret = virNetClientSendInternal(client, msg, false, true); - virNetClientUnlock(client); + virObjectUnlock(client); return ret; } @@ -2079,18 +2059,18 @@ int virNetClientSendWithReplyStream(virNetClientPtr client, virNetClientStreamPtr st) { int ret; - virNetClientLock(client); + virObjectLock(client); /* Other thread might have already received * stream EOF so we don't want sent anything. * Server won't respond anyway. */ if (virNetClientStreamEOF(st)) { - virNetClientUnlock(client); + virObjectUnlock(client); return 0; } ret = virNetClientSendInternal(client, msg, true, false); - virNetClientUnlock(client); + virObjectUnlock(client); if (ret < 0) return -1; return 0; diff --git a/src/rpc/virnetclientstream.c b/src/rpc/virnetclientstream.c index e1ee30e..b8c457e 100644 --- a/src/rpc/virnetclientstream.c +++ b/src/rpc/virnetclientstream.c @@ -32,9 +32,7 @@ #define VIR_FROM_THIS VIR_FROM_RPC struct _virNetClientStream { - virObject object; - - virMutex lock; + virObjectLockable parent; virNetClientProgramPtr prog; int proc; @@ -68,7 +66,7 @@ static void virNetClientStreamDispose(void *obj); static int virNetClientStreamOnceInit(void) { - if (!(virNetClientStreamClass = virClassNew(virClassForObject(), + if (!(virNetClientStreamClass = virClassNew(virClassForObjectLockable(), "virNetClientStream", sizeof(virNetClientStream), virNetClientStreamDispose))) @@ -106,8 +104,7 @@ virNetClientStreamEventTimer(int timer ATTRIBUTE_UNUSED, void *opaque) virNetClientStreamPtr st = opaque; int events = 0; - - virMutexLock(&st->lock); + virObjectLock(st); if (st->cb && (st->cbEvents & VIR_STREAM_EVENT_READABLE) && @@ -124,15 +121,15 @@ virNetClientStreamEventTimer(int timer ATTRIBUTE_UNUSED, void *opaque) virFreeCallback cbFree = st->cbFree; st->cbDispatch = 1; - virMutexUnlock(&st->lock); + virObjectUnlock(st); (cb)(st, events, cbOpaque); - virMutexLock(&st->lock); + virObjectLock(st); st->cbDispatch = 0; if (!st->cb && cbFree) (cbFree)(cbOpaque); } - virMutexUnlock(&st->lock); + virObjectUnlock(st); } @@ -145,20 +142,13 @@ virNetClientStreamPtr virNetClientStreamNew(virNetClientProgramPtr prog, if (virNetClientStreamInitialize() < 0) return NULL; - if (!(st = virObjectNew(virNetClientStreamClass))) + if (!(st = virObjectLockableNew(virNetClientStreamClass))) return NULL; st->prog = prog; st->proc = proc; st->serial = serial; - if (virMutexInit(&st->lock) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot initialize mutex")); - VIR_FREE(st); - return NULL; - } - virObjectRef(prog); return st; @@ -170,7 +160,6 @@ void virNetClientStreamDispose(void *obj) virResetError(&st->err); VIR_FREE(st->incoming); - virMutexDestroy(&st->lock); virObjectUnref(st->prog); } @@ -178,21 +167,21 @@ bool virNetClientStreamMatches(virNetClientStreamPtr st, virNetMessagePtr msg) { bool match = false; - virMutexLock(&st->lock); + virObjectLock(st); if (virNetClientProgramMatches(st->prog, msg) && st->proc == msg->header.proc && st->serial == msg->header.serial) match = true; - virMutexUnlock(&st->lock); + virObjectUnlock(st); return match; } bool virNetClientStreamRaiseError(virNetClientStreamPtr st) { - virMutexLock(&st->lock); + virObjectLock(st); if (st->err.code == VIR_ERR_OK) { - virMutexUnlock(&st->lock); + virObjectUnlock(st); return false; } @@ -206,7 +195,7 @@ bool virNetClientStreamRaiseError(virNetClientStreamPtr st) st->err.int1, st->err.int2, "%s", st->err.message ? st->err.message : _("Unknown error")); - virMutexUnlock(&st->lock); + virObjectUnlock(st); return true; } @@ -217,7 +206,7 @@ int virNetClientStreamSetError(virNetClientStreamPtr st, virNetMessageError err; int ret = -1; - virMutexLock(&st->lock); + virObjectLock(st); if (st->err.code != VIR_ERR_OK) VIR_DEBUG("Overwriting existing stream error %s", NULLSTR(st->err.message)); @@ -265,7 +254,7 @@ int virNetClientStreamSetError(virNetClientStreamPtr st, cleanup: xdr_free((xdrproc_t)xdr_virNetMessageError, (void*)&err); - virMutexUnlock(&st->lock); + virObjectUnlock(st); return ret; } @@ -276,7 +265,7 @@ int virNetClientStreamQueuePacket(virNetClientStreamPtr st, int ret = -1; size_t need; - virMutexLock(&st->lock); + virObjectLock(st); need = msg->bufferLength - msg->bufferOffset; if (need) { size_t avail = st->incomingLength - st->incomingOffset; @@ -306,7 +295,7 @@ int virNetClientStreamQueuePacket(virNetClientStreamPtr st, ret = 0; cleanup: - virMutexUnlock(&st->lock); + virObjectUnlock(st); return ret; } @@ -323,7 +312,7 @@ int virNetClientStreamSendPacket(virNetClientStreamPtr st, if (!(msg = virNetMessageNew(false))) return -1; - virMutexLock(&st->lock); + virObjectLock(st); msg->header.prog = virNetClientProgramGetProgram(st->prog); msg->header.vers = virNetClientProgramGetVersion(st->prog); @@ -332,7 +321,7 @@ int virNetClientStreamSendPacket(virNetClientStreamPtr st, msg->header.serial = st->serial; msg->header.proc = st->proc; - virMutexUnlock(&st->lock); + virObjectUnlock(st); if (virNetMessageEncodeHeader(msg) < 0) goto error; @@ -373,7 +362,7 @@ int virNetClientStreamRecvPacket(virNetClientStreamPtr st, int rv = -1; VIR_DEBUG("st=%p client=%p data=%p nbytes=%zu nonblock=%d", st, client, data, nbytes, nonblock); - virMutexLock(&st->lock); + virObjectLock(st); if (!st->incomingOffset && !st->incomingEOF) { virNetMessagePtr msg; int ret; @@ -397,9 +386,9 @@ int virNetClientStreamRecvPacket(virNetClientStreamPtr st, msg->header.status = VIR_NET_CONTINUE; VIR_DEBUG("Dummy packet to wait for stream data"); - virMutexUnlock(&st->lock); + virObjectUnlock(st); ret = virNetClientSendWithReplyStream(client, msg, st); - virMutexLock(&st->lock); + virObjectLock(st); virNetMessageFree(msg); if (ret < 0) @@ -427,7 +416,7 @@ int virNetClientStreamRecvPacket(virNetClientStreamPtr st, virNetClientStreamEventTimerUpdate(st); cleanup: - virMutexUnlock(&st->lock); + virObjectUnlock(st); return rv; } @@ -440,7 +429,7 @@ int virNetClientStreamEventAddCallback(virNetClientStreamPtr st, { int ret = -1; - virMutexLock(&st->lock); + virObjectLock(st); if (st->cb) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("multiple stream callbacks not supported")); @@ -467,7 +456,7 @@ int virNetClientStreamEventAddCallback(virNetClientStreamPtr st, ret = 0; cleanup: - virMutexUnlock(&st->lock); + virObjectUnlock(st); return ret; } @@ -476,7 +465,7 @@ int virNetClientStreamEventUpdateCallback(virNetClientStreamPtr st, { int ret = -1; - virMutexLock(&st->lock); + virObjectLock(st); if (!st->cb) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("no stream callback registered")); @@ -490,7 +479,7 @@ int virNetClientStreamEventUpdateCallback(virNetClientStreamPtr st, ret = 0; cleanup: - virMutexUnlock(&st->lock); + virObjectUnlock(st); return ret; } @@ -498,7 +487,7 @@ int virNetClientStreamEventRemoveCallback(virNetClientStreamPtr st) { int ret = -1; - virMutexLock(&st->lock); + virObjectLock(st); if (!st->cb) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("no stream callback registered")); @@ -517,7 +506,7 @@ int virNetClientStreamEventRemoveCallback(virNetClientStreamPtr st) ret = 0; cleanup: - virMutexUnlock(&st->lock); + virObjectUnlock(st); return ret; } diff --git a/src/rpc/virnetsaslcontext.c b/src/rpc/virnetsaslcontext.c index 41a69d1..6943216 100644 --- a/src/rpc/virnetsaslcontext.c +++ b/src/rpc/virnetsaslcontext.c @@ -33,16 +33,14 @@ #define VIR_FROM_THIS VIR_FROM_RPC struct _virNetSASLContext { - virObject object; + virObjectLockable parent; - virMutex lock; const char *const*usernameWhitelist; }; struct _virNetSASLSession { - virObject object; + virObjectLockable parent; - virMutex lock; sasl_conn_t *conn; size_t maxbufsize; }; @@ -50,18 +48,17 @@ struct _virNetSASLSession { static virClassPtr virNetSASLContextClass; static virClassPtr virNetSASLSessionClass; -static void virNetSASLContextDispose(void *obj); static void virNetSASLSessionDispose(void *obj); static int virNetSASLContextOnceInit(void) { - if (!(virNetSASLContextClass = virClassNew(virClassForObject(), + if (!(virNetSASLContextClass = virClassNew(virClassForObjectLockable(), "virNetSASLContext", sizeof(virNetSASLContext), - virNetSASLContextDispose))) + NULL))) return -1; - if (!(virNetSASLSessionClass = virClassNew(virClassForObject(), + if (!(virNetSASLSessionClass = virClassNew(virClassForObjectLockable(), "virNetSASLSession", sizeof(virNetSASLSession), virNetSASLSessionDispose))) @@ -89,16 +86,9 @@ virNetSASLContextPtr virNetSASLContextNewClient(void) return NULL; } - if (!(ctxt = virObjectNew(virNetSASLContextClass))) + if (!(ctxt = virObjectLockableNew(virNetSASLContextClass))) return NULL; - if (virMutexInit(&ctxt->lock) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Failed to initialized mutex")); - VIR_FREE(ctxt); - return NULL; - } - return ctxt; } @@ -118,15 +108,8 @@ virNetSASLContextPtr virNetSASLContextNewServer(const char *const*usernameWhitel return NULL; } - if (!(ctxt = virObjectNew(virNetSASLContextClass))) - return NULL; - - if (virMutexInit(&ctxt->lock) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Failed to initialized mutex")); - VIR_FREE(ctxt); + if (!(ctxt = virObjectLockableNew(virNetSASLContextClass))) return NULL; - } ctxt->usernameWhitelist = usernameWhitelist; @@ -139,7 +122,7 @@ int virNetSASLContextCheckIdentity(virNetSASLContextPtr ctxt, const char *const*wildcards; int ret = -1; - virMutexLock(&ctxt->lock); + virObjectLock(ctxt); /* If the list is not set, allow any DN. */ wildcards = ctxt->usernameWhitelist; @@ -173,18 +156,11 @@ int virNetSASLContextCheckIdentity(virNetSASLContextPtr ctxt, ret = 0; cleanup: - virMutexUnlock(&ctxt->lock); + virObjectUnlock(ctxt); return ret; } -void virNetSASLContextDispose(void *obj) -{ - virNetSASLContextPtr ctxt = obj; - - virMutexDestroy(&ctxt->lock); -} - virNetSASLSessionPtr virNetSASLSessionNewClient(virNetSASLContextPtr ctxt ATTRIBUTE_UNUSED, const char *service, const char *hostname, @@ -195,15 +171,8 @@ virNetSASLSessionPtr virNetSASLSessionNewClient(virNetSASLContextPtr ctxt ATTRIB virNetSASLSessionPtr sasl = NULL; int err; - if (!(sasl = virObjectNew(virNetSASLSessionClass))) - return NULL; - - if (virMutexInit(&sasl->lock) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Failed to initialized mutex")); - VIR_FREE(sasl); + if (!(sasl = virObjectLockableNew(virNetSASLSessionClass))) return NULL; - } /* Arbitrary size for amount of data we can encode in a single block */ sasl->maxbufsize = 1 << 16; @@ -237,15 +206,8 @@ virNetSASLSessionPtr virNetSASLSessionNewServer(virNetSASLContextPtr ctxt ATTRIB virNetSASLSessionPtr sasl = NULL; int err; - if (!(sasl = virObjectNew(virNetSASLSessionClass))) - return NULL; - - if (virMutexInit(&sasl->lock) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Failed to initialized mutex")); - VIR_FREE(sasl); + if (!(sasl = virObjectLockableNew(virNetSASLSessionClass))) return NULL; - } /* Arbitrary size for amount of data we can encode in a single block */ sasl->maxbufsize = 1 << 16; @@ -277,7 +239,7 @@ int virNetSASLSessionExtKeySize(virNetSASLSessionPtr sasl, { int err; int ret = -1; - virMutexLock(&sasl->lock); + virObjectLock(sasl); err = sasl_setprop(sasl->conn, SASL_SSF_EXTERNAL, &ssf); if (err != SASL_OK) { @@ -290,7 +252,7 @@ int virNetSASLSessionExtKeySize(virNetSASLSessionPtr sasl, ret = 0; cleanup: - virMutexUnlock(&sasl->lock); + virObjectUnlock(sasl); return ret; } @@ -298,7 +260,7 @@ const char *virNetSASLSessionGetIdentity(virNetSASLSessionPtr sasl) { const void *val = NULL; int err; - virMutexLock(&sasl->lock); + virObjectLock(sasl); err = sasl_getprop(sasl->conn, SASL_USERNAME, &val); if (err != SASL_OK) { @@ -316,7 +278,7 @@ const char *virNetSASLSessionGetIdentity(virNetSASLSessionPtr sasl) VIR_DEBUG("SASL client username %s", (const char *)val); cleanup: - virMutexUnlock(&sasl->lock); + virObjectUnlock(sasl); return (const char*)val; } @@ -327,7 +289,7 @@ int virNetSASLSessionGetKeySize(virNetSASLSessionPtr sasl) int ssf; const void *val; - virMutexLock(&sasl->lock); + virObjectLock(sasl); err = sasl_getprop(sasl->conn, SASL_SSF, &val); if (err != SASL_OK) { virReportError(VIR_ERR_AUTH_FAILED, @@ -339,7 +301,7 @@ int virNetSASLSessionGetKeySize(virNetSASLSessionPtr sasl) ssf = *(const int *)val; cleanup: - virMutexUnlock(&sasl->lock); + virObjectUnlock(sasl); return ssf; } @@ -355,7 +317,7 @@ int virNetSASLSessionSecProps(virNetSASLSessionPtr sasl, VIR_DEBUG("minSSF=%d maxSSF=%d allowAnonymous=%d maxbufsize=%zu", minSSF, maxSSF, allowAnonymous, sasl->maxbufsize); - virMutexLock(&sasl->lock); + virObjectLock(sasl); memset(&secprops, 0, sizeof(secprops)); secprops.min_ssf = minSSF; @@ -375,7 +337,7 @@ int virNetSASLSessionSecProps(virNetSASLSessionPtr sasl, ret = 0; cleanup: - virMutexUnlock(&sasl->lock); + virObjectUnlock(sasl); return ret; } @@ -408,7 +370,7 @@ char *virNetSASLSessionListMechanisms(virNetSASLSessionPtr sasl) char *ret = NULL; int err; - virMutexLock(&sasl->lock); + virObjectLock(sasl); err = sasl_listmech(sasl->conn, NULL, /* Don't need to set user */ "", /* Prefix */ @@ -429,7 +391,7 @@ char *virNetSASLSessionListMechanisms(virNetSASLSessionPtr sasl) } cleanup: - virMutexUnlock(&sasl->lock); + virObjectUnlock(sasl); return ret; } @@ -448,7 +410,7 @@ int virNetSASLSessionClientStart(virNetSASLSessionPtr sasl, VIR_DEBUG("sasl=%p mechlist=%s prompt_need=%p clientout=%p clientoutlen=%p mech=%p", sasl, mechlist, prompt_need, clientout, clientoutlen, mech); - virMutexLock(&sasl->lock); + virObjectLock(sasl); err = sasl_client_start(sasl->conn, mechlist, prompt_need, @@ -478,7 +440,7 @@ int virNetSASLSessionClientStart(virNetSASLSessionPtr sasl, } cleanup: - virMutexUnlock(&sasl->lock); + virObjectUnlock(sasl); return ret; } @@ -498,7 +460,7 @@ int virNetSASLSessionClientStep(virNetSASLSessionPtr sasl, VIR_DEBUG("sasl=%p serverin=%s serverinlen=%zu prompt_need=%p clientout=%p clientoutlen=%p", sasl, serverin, serverinlen, prompt_need, clientout, clientoutlen); - virMutexLock(&sasl->lock); + virObjectLock(sasl); err = sasl_client_step(sasl->conn, serverin, inlen, @@ -527,7 +489,7 @@ int virNetSASLSessionClientStep(virNetSASLSessionPtr sasl, } cleanup: - virMutexUnlock(&sasl->lock); + virObjectUnlock(sasl); return ret; } @@ -543,7 +505,7 @@ int virNetSASLSessionServerStart(virNetSASLSessionPtr sasl, int err; int ret = -1; - virMutexLock(&sasl->lock); + virObjectLock(sasl); err = sasl_server_start(sasl->conn, mechname, clientin, @@ -573,7 +535,7 @@ int virNetSASLSessionServerStart(virNetSASLSessionPtr sasl, } cleanup: - virMutexUnlock(&sasl->lock); + virObjectUnlock(sasl); return ret; } @@ -589,7 +551,7 @@ int virNetSASLSessionServerStep(virNetSASLSessionPtr sasl, int err; int ret = -1; - virMutexLock(&sasl->lock); + virObjectLock(sasl); err = sasl_server_step(sasl->conn, clientin, inlen, @@ -618,16 +580,16 @@ int virNetSASLSessionServerStep(virNetSASLSessionPtr sasl, } cleanup: - virMutexUnlock(&sasl->lock); + virObjectUnlock(sasl); return ret; } size_t virNetSASLSessionGetMaxBufSize(virNetSASLSessionPtr sasl) { size_t ret; - virMutexLock(&sasl->lock); + virObjectLock(sasl); ret = sasl->maxbufsize; - virMutexUnlock(&sasl->lock); + virObjectUnlock(sasl); return ret; } @@ -642,7 +604,7 @@ ssize_t virNetSASLSessionEncode(virNetSASLSessionPtr sasl, int err; ssize_t ret = -1; - virMutexLock(&sasl->lock); + virObjectLock(sasl); if (inputLen > sasl->maxbufsize) { virReportSystemError(EINVAL, _("SASL data length %zu too long, max %zu"), @@ -666,7 +628,7 @@ ssize_t virNetSASLSessionEncode(virNetSASLSessionPtr sasl, ret = 0; cleanup: - virMutexUnlock(&sasl->lock); + virObjectUnlock(sasl); return ret; } @@ -681,7 +643,7 @@ ssize_t virNetSASLSessionDecode(virNetSASLSessionPtr sasl, int err; ssize_t ret = -1; - virMutexLock(&sasl->lock); + virObjectLock(sasl); if (inputLen > sasl->maxbufsize) { virReportSystemError(EINVAL, _("SASL data length %zu too long, max %zu"), @@ -704,7 +666,7 @@ ssize_t virNetSASLSessionDecode(virNetSASLSessionPtr sasl, ret = 0; cleanup: - virMutexUnlock(&sasl->lock); + virObjectUnlock(sasl); return ret; } @@ -715,5 +677,4 @@ void virNetSASLSessionDispose(void *obj) if (sasl->conn) sasl_dispose(&sasl->conn); - virMutexDestroy(&sasl->lock); } diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 03efbb8..dd104be 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -64,9 +64,7 @@ struct _virNetServerJob { }; struct _virNetServer { - virObject object; - - virMutex lock; + virObjectLockable parent; virThreadPoolPtr workers; @@ -119,7 +117,7 @@ static void virNetServerDispose(void *obj); static int virNetServerOnceInit(void) { - if (!(virNetServerClass = virClassNew(virClassForObject(), + if (!(virNetServerClass = virClassNew(virClassForObjectLockable(), "virNetServer", sizeof(virNetServer), virNetServerDispose))) @@ -131,17 +129,6 @@ static int virNetServerOnceInit(void) VIR_ONCE_GLOBAL_INIT(virNetServer) -static void virNetServerLock(virNetServerPtr srv) -{ - virMutexLock(&srv->lock); -} - -static void virNetServerUnlock(virNetServerPtr srv) -{ - virMutexUnlock(&srv->lock); -} - - static int virNetServerProcessMsg(virNetServerPtr srv, virNetServerClientPtr client, virNetServerProgramPtr prog, @@ -222,7 +209,7 @@ static int virNetServerDispatchNewMessage(virNetServerClientPtr client, VIR_DEBUG("server=%p client=%p message=%p", srv, client, msg); - virNetServerLock(srv); + virObjectLock(srv); for (i = 0 ; i < srv->nprograms ; i++) { if (virNetServerProgramMatches(srv->programs[i], msg)) { prog = srv->programs[i]; @@ -258,7 +245,7 @@ static int virNetServerDispatchNewMessage(virNetServerClientPtr client, } cleanup: - virNetServerUnlock(srv); + virObjectUnlock(srv); return ret; } @@ -267,7 +254,7 @@ cleanup: static int virNetServerAddClient(virNetServerPtr srv, virNetServerClientPtr client) { - virNetServerLock(srv); + virObjectLock(srv); if (srv->nclients >= srv->nclients_max) { virReportError(VIR_ERR_RPC, @@ -293,11 +280,11 @@ static int virNetServerAddClient(virNetServerPtr srv, virNetServerClientInitKeepAlive(client, srv->keepaliveInterval, srv->keepaliveCount); - virNetServerUnlock(srv); + virObjectUnlock(srv); return 0; error: - virNetServerUnlock(srv); + virObjectUnlock(srv); return -1; } @@ -378,7 +365,7 @@ virNetServerPtr virNetServerNew(size_t min_workers, if (virNetServerInitialize() < 0) return NULL; - if (!(srv = virObjectNew(virNetServerClass))) + if (!(srv = virObjectLockableNew(virNetServerClass))) return NULL; if (max_workers && @@ -413,12 +400,6 @@ virNetServerPtr virNetServerNew(size_t min_workers, goto error; } - if (virMutexInit(&srv->lock) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot initialize mutex")); - goto error; - } - if (virEventRegisterDefaultImpl() < 0) goto error; @@ -607,7 +588,7 @@ virJSONValuePtr virNetServerPreExecRestart(virNetServerPtr srv) virJSONValuePtr services; size_t i; - virMutexLock(&srv->lock); + virObjectLock(srv); if (!(object = virJSONValueNewObject())) goto error; @@ -692,13 +673,13 @@ virJSONValuePtr virNetServerPreExecRestart(virNetServerPtr srv) } } - virMutexUnlock(&srv->lock); + virObjectUnlock(srv); return object; error: virJSONValueFree(object); - virMutexUnlock(&srv->lock); + virObjectUnlock(srv); return NULL; } @@ -706,9 +687,9 @@ error: bool virNetServerIsPrivileged(virNetServerPtr srv) { bool priv; - virNetServerLock(srv); + virObjectLock(srv); priv = srv->privileged; - virNetServerUnlock(srv); + virObjectUnlock(srv); return priv; } @@ -716,11 +697,11 @@ bool virNetServerIsPrivileged(virNetServerPtr srv) void virNetServerAutoShutdown(virNetServerPtr srv, unsigned int timeout) { - virNetServerLock(srv); + virObjectLock(srv); srv->autoShutdownTimeout = timeout; - virNetServerUnlock(srv); + virObjectUnlock(srv); } @@ -732,7 +713,7 @@ static void virNetServerGotInhibitReply(DBusPendingCall *pending, DBusMessage *reply; int fd; - virNetServerLock(srv); + virObjectLock(srv); srv->autoShutdownCallingInhibit = false; VIR_DEBUG("srv=%p", srv); @@ -754,7 +735,7 @@ static void virNetServerGotInhibitReply(DBusPendingCall *pending, dbus_message_unref(reply); cleanup: - virNetServerUnlock(srv); + virObjectUnlock(srv); } @@ -808,7 +789,7 @@ static void virNetServerCallInhibit(virNetServerPtr srv, void virNetServerAddShutdownInhibition(virNetServerPtr srv) { - virNetServerLock(srv); + virObjectLock(srv); srv->autoShutdownInhibitions++; VIR_DEBUG("srv=%p inhibitions=%zu", srv, srv->autoShutdownInhibitions); @@ -822,13 +803,13 @@ void virNetServerAddShutdownInhibition(virNetServerPtr srv) "delay"); #endif - virNetServerUnlock(srv); + virObjectUnlock(srv); } void virNetServerRemoveShutdownInhibition(virNetServerPtr srv) { - virNetServerLock(srv); + virObjectLock(srv); srv->autoShutdownInhibitions--; VIR_DEBUG("srv=%p inhibitions=%zu", srv, srv->autoShutdownInhibitions); @@ -836,7 +817,7 @@ void virNetServerRemoveShutdownInhibition(virNetServerPtr srv) if (srv->autoShutdownInhibitions == 0) VIR_FORCE_CLOSE(srv->autoShutdownInhibitFd); - virNetServerUnlock(srv); + virObjectUnlock(srv); } @@ -879,7 +860,7 @@ virNetServerSignalEvent(int watch, siginfo_t siginfo; int i; - virNetServerLock(srv); + virObjectLock(srv); if (saferead(srv->sigread, &siginfo, sizeof(siginfo)) != sizeof(siginfo)) { virReportSystemError(errno, "%s", @@ -893,7 +874,7 @@ virNetServerSignalEvent(int watch, if (siginfo.si_signo == srv->signals[i]->signum) { virNetServerSignalFunc func = srv->signals[i]->func; void *funcopaque = srv->signals[i]->opaque; - virNetServerUnlock(srv); + virObjectUnlock(srv); func(srv, &siginfo, funcopaque); return; } @@ -903,7 +884,7 @@ virNetServerSignalEvent(int watch, _("Unexpected signal received: %d"), siginfo.si_signo); cleanup: - virNetServerUnlock(srv); + virObjectUnlock(srv); } static int virNetServerSignalSetup(virNetServerPtr srv) @@ -948,7 +929,7 @@ int virNetServerAddSignalHandler(virNetServerPtr srv, virNetServerSignalPtr sigdata; struct sigaction sig_action; - virNetServerLock(srv); + virObjectLock(srv); if (virNetServerSignalSetup(srv) < 0) goto error; @@ -972,14 +953,14 @@ int virNetServerAddSignalHandler(virNetServerPtr srv, srv->signals[srv->nsignals-1] = sigdata; - virNetServerUnlock(srv); + virObjectUnlock(srv); return 0; no_memory: virReportOOMError(); error: VIR_FREE(sigdata); - virNetServerUnlock(srv); + virObjectUnlock(srv); return -1; } @@ -989,7 +970,7 @@ int virNetServerAddService(virNetServerPtr srv, virNetServerServicePtr svc, const char *mdnsEntryName) { - virNetServerLock(srv); + virObjectLock(srv); if (VIR_EXPAND_N(srv->services, srv->nservices, 1) < 0) goto no_memory; @@ -1010,32 +991,32 @@ int virNetServerAddService(virNetServerPtr srv, virNetServerDispatchNewClient, srv); - virNetServerUnlock(srv); + virObjectUnlock(srv); return 0; no_memory: virReportOOMError(); error: - virNetServerUnlock(srv); + virObjectUnlock(srv); return -1; } int virNetServerAddProgram(virNetServerPtr srv, virNetServerProgramPtr prog) { - virNetServerLock(srv); + virObjectLock(srv); if (VIR_EXPAND_N(srv->programs, srv->nprograms, 1) < 0) goto no_memory; srv->programs[srv->nprograms-1] = virObjectRef(prog); - virNetServerUnlock(srv); + virObjectUnlock(srv); return 0; no_memory: virReportOOMError(); - virNetServerUnlock(srv); + virObjectUnlock(srv); return -1; } @@ -1053,14 +1034,14 @@ static void virNetServerAutoShutdownTimer(int timerid ATTRIBUTE_UNUSED, void *opaque) { virNetServerPtr srv = opaque; - virNetServerLock(srv); + virObjectLock(srv); if (!srv->autoShutdownInhibitions) { VIR_DEBUG("Automatic shutdown triggered"); srv->quit = 1; } - virNetServerUnlock(srv); + virObjectUnlock(srv); } @@ -1069,11 +1050,11 @@ void virNetServerUpdateServices(virNetServerPtr srv, { int i; - virNetServerLock(srv); + virObjectLock(srv); for (i = 0 ; i < srv->nservices ; i++) virNetServerServiceToggle(srv->services[i], enabled); - virNetServerUnlock(srv); + virObjectUnlock(srv); } @@ -1083,7 +1064,7 @@ void virNetServerRun(virNetServerPtr srv) int timerActive = 0; int i; - virNetServerLock(srv); + virObjectLock(srv); if (srv->mdns && virNetServerMDNSStart(srv->mdns) < 0) @@ -1123,13 +1104,13 @@ void virNetServerRun(virNetServerPtr srv) } } - virNetServerUnlock(srv); + virObjectUnlock(srv); if (virEventRunDefaultImpl() < 0) { - virNetServerLock(srv); + virObjectLock(srv); VIR_DEBUG("Loop iteration error, exiting"); break; } - virNetServerLock(srv); + virObjectLock(srv); reprocess: for (i = 0 ; i < srv->nclients ; i++) { @@ -1156,18 +1137,18 @@ void virNetServerRun(virNetServerPtr srv) } cleanup: - virNetServerUnlock(srv); + virObjectUnlock(srv); } void virNetServerQuit(virNetServerPtr srv) { - virNetServerLock(srv); + virObjectLock(srv); VIR_DEBUG("Quit requested %p", srv); srv->quit = 1; - virNetServerUnlock(srv); + virObjectUnlock(srv); } void virNetServerDispose(void *obj) @@ -1208,8 +1189,6 @@ void virNetServerDispose(void *obj) VIR_FREE(srv->mdnsGroupName); virNetServerMDNSFree(srv->mdns); - - virMutexDestroy(&srv->lock); } void virNetServerClose(virNetServerPtr srv) @@ -1219,20 +1198,20 @@ void virNetServerClose(virNetServerPtr srv) if (!srv) return; - virNetServerLock(srv); + virObjectLock(srv); for (i = 0; i < srv->nservices; i++) { virNetServerServiceClose(srv->services[i]); } - virNetServerUnlock(srv); + virObjectUnlock(srv); } bool virNetServerKeepAliveRequired(virNetServerPtr srv) { bool required; - virNetServerLock(srv); + virObjectLock(srv); required = srv->keepaliveRequired; - virNetServerUnlock(srv); + virObjectUnlock(srv); return required; } diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index ce8bd6d..10ac7e0 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -57,11 +57,10 @@ struct _virNetServerClientFilter { struct _virNetServerClient { - virObject object; + virObjectLockable parent; bool wantClose; bool delayedClose; - virMutex lock; virNetSocketPtr sock; int auth; bool readonly; @@ -112,7 +111,7 @@ static void virNetServerClientDispose(void *obj); static int virNetServerClientOnceInit(void) { - if (!(virNetServerClientClass = virClassNew(virClassForObject(), + if (!(virNetServerClientClass = virClassNew(virClassForObjectLockable(), "virNetServerClient", sizeof(virNetServerClient), virNetServerClientDispose))) @@ -130,17 +129,6 @@ static void virNetServerClientDispatchRead(virNetServerClientPtr client); static int virNetServerClientSendMessageLocked(virNetServerClientPtr client, virNetMessagePtr msg); -static void virNetServerClientLock(virNetServerClientPtr client) -{ - virMutexLock(&client->lock); -} - -static void virNetServerClientUnlock(virNetServerClientPtr client) -{ - virMutexUnlock(&client->lock); -} - - /* * @client: a locked client object */ @@ -253,7 +241,7 @@ int virNetServerClientAddFilter(virNetServerClientPtr client, return -1; } - virNetServerClientLock(client); + virObjectLock(client); filter->id = client->nextFilterID++; filter->func = func; @@ -266,7 +254,7 @@ int virNetServerClientAddFilter(virNetServerClientPtr client, ret = filter->id; - virNetServerClientUnlock(client); + virObjectUnlock(client); return ret; } @@ -276,7 +264,7 @@ void virNetServerClientRemoveFilter(virNetServerClientPtr client, { virNetServerClientFilterPtr tmp, prev; - virNetServerClientLock(client); + virObjectLock(client); prev = NULL; tmp = client->filters; @@ -294,7 +282,7 @@ void virNetServerClientRemoveFilter(virNetServerClientPtr client, tmp = tmp->next; } - virNetServerClientUnlock(client); + virObjectUnlock(client); } @@ -341,13 +329,13 @@ static void virNetServerClientSockTimerFunc(int timer, void *opaque) { virNetServerClientPtr client = opaque; - virNetServerClientLock(client); + virObjectLock(client); virEventUpdateTimeout(timer, -1); /* Although client->rx != NULL when this timer is enabled, it might have * changed since the client was unlocked in the meantime. */ if (client->rx) virNetServerClientDispatchRead(client); - virNetServerClientUnlock(client); + virObjectUnlock(client); } @@ -365,14 +353,9 @@ virNetServerClientNewInternal(virNetSocketPtr sock, if (virNetServerClientInitialize() < 0) return NULL; - if (!(client = virObjectNew(virNetServerClientClass))) + if (!(client = virObjectLockableNew(virNetServerClientClass))) return NULL; - if (virMutexInit(&client->lock) < 0) { - VIR_FREE(client); - return NULL; - } - client->sock = virObjectRef(sock); client->auth = auth; client->readonly = readonly; @@ -544,7 +527,7 @@ virJSONValuePtr virNetServerClientPreExecRestart(virNetServerClientPtr client) if (!object) return NULL; - virNetServerClientLock(client); + virObjectLock(client); if (virJSONValueObjectAppendNumberInt(object, "auth", client->auth) < 0) goto error; @@ -574,11 +557,11 @@ virJSONValuePtr virNetServerClientPreExecRestart(virNetServerClientPtr client) goto error; } - virNetServerClientUnlock(client); + virObjectUnlock(client); return object; error: - virNetServerClientUnlock(client); + virObjectUnlock(client); virJSONValueFree(object); return NULL; } @@ -587,18 +570,18 @@ error: int virNetServerClientGetAuth(virNetServerClientPtr client) { int auth; - virNetServerClientLock(client); + virObjectLock(client); auth = client->auth; - virNetServerClientUnlock(client); + virObjectUnlock(client); return auth; } bool virNetServerClientGetReadonly(virNetServerClientPtr client) { bool readonly; - virNetServerClientLock(client); + virObjectLock(client); readonly = client->readonly; - virNetServerClientUnlock(client); + virObjectUnlock(client); return readonly; } @@ -607,19 +590,19 @@ bool virNetServerClientGetReadonly(virNetServerClientPtr client) bool virNetServerClientHasTLSSession(virNetServerClientPtr client) { bool has; - virNetServerClientLock(client); + virObjectLock(client); has = client->tls ? true : false; - virNetServerClientUnlock(client); + virObjectUnlock(client); return has; } int virNetServerClientGetTLSKeySize(virNetServerClientPtr client) { int size = 0; - virNetServerClientLock(client); + virObjectLock(client); if (client->tls) size = virNetTLSSessionGetKeySize(client->tls); - virNetServerClientUnlock(client); + virObjectUnlock(client); return size; } #endif @@ -627,10 +610,10 @@ int virNetServerClientGetTLSKeySize(virNetServerClientPtr client) int virNetServerClientGetFD(virNetServerClientPtr client) { int fd = -1; - virNetServerClientLock(client); + virObjectLock(client); if (client->sock) fd = virNetSocketGetFD(client->sock); - virNetServerClientUnlock(client); + virObjectUnlock(client); return fd; } @@ -638,17 +621,17 @@ int virNetServerClientGetUNIXIdentity(virNetServerClientPtr client, uid_t *uid, gid_t *gid, pid_t *pid) { int ret = -1; - virNetServerClientLock(client); + virObjectLock(client); if (client->sock) ret = virNetSocketGetUNIXIdentity(client->sock, uid, gid, pid); - virNetServerClientUnlock(client); + virObjectUnlock(client); return ret; } bool virNetServerClientIsSecure(virNetServerClientPtr client) { bool secure = false; - virNetServerClientLock(client); + virObjectLock(client); #if HAVE_GNUTLS if (client->tls) secure = true; @@ -659,7 +642,7 @@ bool virNetServerClientIsSecure(virNetServerClientPtr client) #endif if (client->sock && virNetSocketIsLocal(client->sock)) secure = true; - virNetServerClientUnlock(client); + virObjectUnlock(client); return secure; } @@ -674,9 +657,9 @@ void virNetServerClientSetSASLSession(virNetServerClientPtr client, * in the clear. Only once we complete the next 'tx' * operation do we switch to SASL mode */ - virNetServerClientLock(client); + virObjectLock(client); client->sasl = virObjectRef(sasl); - virNetServerClientUnlock(client); + virObjectUnlock(client); } #endif @@ -685,7 +668,7 @@ int virNetServerClientSetIdentity(virNetServerClientPtr client, const char *identity) { int ret = -1; - virNetServerClientLock(client); + virObjectLock(client); if (!(client->identity = strdup(identity))) { virReportOOMError(); goto error; @@ -693,16 +676,16 @@ int virNetServerClientSetIdentity(virNetServerClientPtr client, ret = 0; error: - virNetServerClientUnlock(client); + virObjectUnlock(client); return ret; } const char *virNetServerClientGetIdentity(virNetServerClientPtr client) { const char *identity; - virNetServerClientLock(client); + virObjectLock(client); identity = client->identity; - virNetServerClientLock(client); + virObjectUnlock(client); return identity; } @@ -710,9 +693,9 @@ const char *virNetServerClientGetIdentity(virNetServerClientPtr client) void *virNetServerClientGetPrivateData(virNetServerClientPtr client) { void *data; - virNetServerClientLock(client); + virObjectLock(client); data = client->privateData; - virNetServerClientUnlock(client); + virObjectUnlock(client); return data; } @@ -720,9 +703,9 @@ void *virNetServerClientGetPrivateData(virNetServerClientPtr client) void virNetServerClientSetCloseHook(virNetServerClientPtr client, virNetServerClientCloseFunc cf) { - virNetServerClientLock(client); + virObjectLock(client); client->privateDataCloseFunc = cf; - virNetServerClientUnlock(client); + virObjectUnlock(client); } @@ -730,10 +713,10 @@ void virNetServerClientSetDispatcher(virNetServerClientPtr client, virNetServerClientDispatchFunc func, void *opaque) { - virNetServerClientLock(client); + virObjectLock(client); client->dispatchFunc = func; client->dispatchOpaque = opaque; - virNetServerClientUnlock(client); + virObjectUnlock(client); } @@ -772,8 +755,7 @@ void virNetServerClientDispose(void *obj) virObjectUnref(client->tlsCtxt); #endif virObjectUnref(client->sock); - virNetServerClientUnlock(client); - virMutexDestroy(&client->lock); + virObjectUnlock(client); } @@ -790,10 +772,10 @@ void virNetServerClientClose(virNetServerClientPtr client) virNetServerClientCloseFunc cf; virKeepAlivePtr ka; - virNetServerClientLock(client); + virObjectLock(client); VIR_DEBUG("client=%p", client); if (!client->sock) { - virNetServerClientUnlock(client); + virObjectUnlock(client); return; } @@ -802,18 +784,18 @@ void virNetServerClientClose(virNetServerClientPtr client) ka = client->keepalive; client->keepalive = NULL; virObjectRef(client); - virNetServerClientUnlock(client); + virObjectUnlock(client); virObjectUnref(ka); - virNetServerClientLock(client); + virObjectLock(client); virObjectUnref(client); } if (client->privateDataCloseFunc) { cf = client->privateDataCloseFunc; virObjectRef(client); - virNetServerClientUnlock(client); + virObjectUnlock(client); (cf)(client); - virNetServerClientLock(client); + virObjectLock(client); virObjectUnref(client); } @@ -847,46 +829,46 @@ void virNetServerClientClose(virNetServerClientPtr client) client->sock = NULL; } - virNetServerClientUnlock(client); + virObjectUnlock(client); } bool virNetServerClientIsClosed(virNetServerClientPtr client) { bool closed; - virNetServerClientLock(client); + virObjectLock(client); closed = client->sock == NULL ? true : false; - virNetServerClientUnlock(client); + virObjectUnlock(client); return closed; } void virNetServerClientDelayedClose(virNetServerClientPtr client) { - virNetServerClientLock(client); + virObjectLock(client); client->delayedClose = true; - virNetServerClientUnlock(client); + virObjectUnlock(client); } void virNetServerClientImmediateClose(virNetServerClientPtr client) { - virNetServerClientLock(client); + virObjectLock(client); client->wantClose = true; - virNetServerClientUnlock(client); + virObjectUnlock(client); } bool virNetServerClientWantClose(virNetServerClientPtr client) { bool wantClose; - virNetServerClientLock(client); + virObjectLock(client); wantClose = client->wantClose; - virNetServerClientUnlock(client); + virObjectUnlock(client); return wantClose; } int virNetServerClientInit(virNetServerClientPtr client) { - virNetServerClientLock(client); + virObjectLock(client); #if HAVE_GNUTLS if (!client->tlsCtxt) { @@ -925,12 +907,12 @@ int virNetServerClientInit(virNetServerClientPtr client) } #endif - virNetServerClientUnlock(client); + virObjectUnlock(client); return 0; error: client->wantClose = true; - virNetServerClientUnlock(client); + virObjectUnlock(client); return -1; } @@ -1256,11 +1238,11 @@ virNetServerClientDispatchEvent(virNetSocketPtr sock, int events, void *opaque) { virNetServerClientPtr client = opaque; - virNetServerClientLock(client); + virObjectLock(client); if (client->sock != sock) { virNetSocketRemoveIOCallback(sock); - virNetServerClientUnlock(client); + virObjectUnlock(client); return; } @@ -1289,7 +1271,7 @@ virNetServerClientDispatchEvent(virNetSocketPtr sock, int events, void *opaque) VIR_EVENT_HANDLE_HANGUP)) client->wantClose = true; - virNetServerClientUnlock(client); + virObjectUnlock(client); } @@ -1323,9 +1305,9 @@ int virNetServerClientSendMessage(virNetServerClientPtr client, { int ret; - virNetServerClientLock(client); + virObjectLock(client); ret = virNetServerClientSendMessageLocked(client, msg); - virNetServerClientUnlock(client); + virObjectUnlock(client); return ret; } @@ -1334,10 +1316,10 @@ int virNetServerClientSendMessage(virNetServerClientPtr client, bool virNetServerClientNeedAuth(virNetServerClientPtr client) { bool need = false; - virNetServerClientLock(client); + virObjectLock(client); if (client->auth && !client->identity) need = true; - virNetServerClientUnlock(client); + virObjectUnlock(client); return need; } @@ -1364,7 +1346,7 @@ virNetServerClientInitKeepAlive(virNetServerClientPtr client, virKeepAlivePtr ka; int ret = -1; - virNetServerClientLock(client); + virObjectLock(client); if (!(ka = virKeepAliveNew(interval, count, client, virNetServerClientKeepAliveSendCB, @@ -1378,7 +1360,7 @@ virNetServerClientInitKeepAlive(virNetServerClientPtr client, ka = NULL; cleanup: - virNetServerClientUnlock(client); + virObjectUnlock(client); if (ka) virKeepAliveStop(ka); virObjectUnref(ka); @@ -1390,8 +1372,8 @@ int virNetServerClientStartKeepAlive(virNetServerClientPtr client) { int ret; - virNetServerClientLock(client); + virObjectLock(client); ret = virKeepAliveStart(client->keepalive, 0, 0); - virNetServerClientUnlock(client); + virObjectUnlock(client); return ret; } diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index f96b47c..d4558c9 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -59,9 +59,7 @@ struct _virNetSocket { - virObject object; - - virMutex lock; + virObjectLockable parent; int fd; int watch; @@ -104,7 +102,7 @@ static void virNetSocketDispose(void *obj); static int virNetSocketOnceInit(void) { - if (!(virNetSocketClass = virClassNew(virClassForObject(), + if (!(virNetSocketClass = virClassNew(virClassForObjectLockable(), "virNetSocket", sizeof(virNetSocket), virNetSocketDispose))) @@ -163,15 +161,8 @@ static virNetSocketPtr virNetSocketNew(virSocketAddrPtr localAddr, return NULL; } - if (!(sock = virObjectNew(virNetSocketClass))) - return NULL; - - if (virMutexInit(&sock->lock) < 0) { - virReportSystemError(errno, "%s", - _("Unable to initialize mutex")); - VIR_FREE(sock); + if (!(sock = virObjectLockableNew(virNetSocketClass))) return NULL; - } if (localAddr) sock->localAddr = *localAddr; @@ -942,7 +933,7 @@ virJSONValuePtr virNetSocketPreExecRestart(virNetSocketPtr sock) { virJSONValuePtr object = NULL; - virMutexLock(&sock->lock); + virObjectLock(sock); #if HAVE_SASL if (sock->saslSession) { @@ -988,11 +979,11 @@ virJSONValuePtr virNetSocketPreExecRestart(virNetSocketPtr sock) goto error; } - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); return object; error: - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); virJSONValueFree(object); return NULL; } @@ -1037,17 +1028,15 @@ void virNetSocketDispose(void *obj) VIR_FREE(sock->localAddrStr); VIR_FREE(sock->remoteAddrStr); - - virMutexDestroy(&sock->lock); } int virNetSocketGetFD(virNetSocketPtr sock) { int fd; - virMutexLock(&sock->lock); + virObjectLock(sock); fd = sock->fd; - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); return fd; } @@ -1072,10 +1061,10 @@ int virNetSocketDupFD(virNetSocketPtr sock, bool cloexec) bool virNetSocketIsLocal(virNetSocketPtr sock) { bool isLocal = false; - virMutexLock(&sock->lock); + virObjectLock(sock); if (sock->localAddr.data.sa.sa_family == AF_UNIX) isLocal = true; - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); return isLocal; } @@ -1083,10 +1072,10 @@ bool virNetSocketIsLocal(virNetSocketPtr sock) bool virNetSocketHasPassFD(virNetSocketPtr sock) { bool hasPassFD = false; - virMutexLock(&sock->lock); + virObjectLock(sock); if (sock->localAddr.data.sa.sa_family == AF_UNIX) hasPassFD = true; - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); return hasPassFD; } @@ -1094,9 +1083,9 @@ bool virNetSocketHasPassFD(virNetSocketPtr sock) int virNetSocketGetPort(virNetSocketPtr sock) { int port; - virMutexLock(&sock->lock); + virObjectLock(sock); port = virSocketAddrGetPort(&sock->localAddr); - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); return port; } @@ -1109,12 +1098,12 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock, { struct ucred cr; socklen_t cr_len = sizeof(cr); - virMutexLock(&sock->lock); + virObjectLock(sock); if (getsockopt(sock->fd, SOL_SOCKET, SO_PEERCRED, &cr, &cr_len) < 0) { virReportSystemError(errno, "%s", _("Failed to get client socket identity")); - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); return -1; } @@ -1122,7 +1111,7 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock, *uid = cr.uid; *gid = cr.gid; - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); return 0; } #elif defined(LOCAL_PEERCRED) @@ -1133,12 +1122,12 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock, { struct xucred cr; socklen_t cr_len = sizeof(cr); - virMutexLock(&sock->lock); + virObjectLock(sock); if (getsockopt(sock->fd, SOL_SOCKET, LOCAL_PEERCRED, &cr, &cr_len) < 0) { virReportSystemError(errno, "%s", _("Failed to get client socket identity")); - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); return -1; } @@ -1146,7 +1135,7 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock, *uid = cr.cr_uid; *gid = cr.cr_gid; - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); return 0; } #else @@ -1167,9 +1156,9 @@ int virNetSocketSetBlocking(virNetSocketPtr sock, bool blocking) { int ret; - virMutexLock(&sock->lock); + virObjectLock(sock); ret = virSetBlocking(sock->fd, blocking); - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); return ret; } @@ -1207,14 +1196,14 @@ static ssize_t virNetSocketTLSSessionRead(char *buf, void virNetSocketSetTLSSession(virNetSocketPtr sock, virNetTLSSessionPtr sess) { - virMutexLock(&sock->lock); + virObjectLock(sock); virObjectUnref(sock->tlsSession); sock->tlsSession = virObjectRef(sess); virNetTLSSessionSetIOCallbacks(sess, virNetSocketTLSSessionWrite, virNetSocketTLSSessionRead, sock); - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); } #endif @@ -1222,10 +1211,10 @@ void virNetSocketSetTLSSession(virNetSocketPtr sock, void virNetSocketSetSASLSession(virNetSocketPtr sock, virNetSASLSessionPtr sess) { - virMutexLock(&sock->lock); + virObjectLock(sock); virObjectUnref(sock->saslSession); sock->saslSession = virObjectRef(sess); - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); } #endif @@ -1233,7 +1222,7 @@ void virNetSocketSetSASLSession(virNetSocketPtr sock, bool virNetSocketHasCachedData(virNetSocketPtr sock ATTRIBUTE_UNUSED) { bool hasCached = false; - virMutexLock(&sock->lock); + virObjectLock(sock); #if HAVE_LIBSSH2 if (virNetSSHSessionHasCachedData(sock->sshSession)) @@ -1244,7 +1233,7 @@ bool virNetSocketHasCachedData(virNetSocketPtr sock ATTRIBUTE_UNUSED) if (sock->saslDecoded) hasCached = true; #endif - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); return hasCached; } @@ -1267,12 +1256,12 @@ static ssize_t virNetSocketLibSSH2Write(virNetSocketPtr sock, bool virNetSocketHasPendingData(virNetSocketPtr sock ATTRIBUTE_UNUSED) { bool hasPending = false; - virMutexLock(&sock->lock); + virObjectLock(sock); #if HAVE_SASL if (sock->saslEncoded) hasPending = true; #endif - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); return hasPending; } @@ -1481,14 +1470,14 @@ static ssize_t virNetSocketWriteSASL(virNetSocketPtr sock, const char *buf, size ssize_t virNetSocketRead(virNetSocketPtr sock, char *buf, size_t len) { ssize_t ret; - virMutexLock(&sock->lock); + virObjectLock(sock); #if HAVE_SASL if (sock->saslSession) ret = virNetSocketReadSASL(sock, buf, len); else #endif ret = virNetSocketReadWire(sock, buf, len); - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); return ret; } @@ -1496,14 +1485,14 @@ ssize_t virNetSocketWrite(virNetSocketPtr sock, const char *buf, size_t len) { ssize_t ret; - virMutexLock(&sock->lock); + virObjectLock(sock); #if HAVE_SASL if (sock->saslSession) ret = virNetSocketWriteSASL(sock, buf, len); else #endif ret = virNetSocketWriteWire(sock, buf, len); - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); return ret; } @@ -1519,7 +1508,7 @@ int virNetSocketSendFD(virNetSocketPtr sock, int fd) _("Sending file descriptors is not supported on this socket")); return -1; } - virMutexLock(&sock->lock); + virObjectLock(sock); PROBE(RPC_SOCKET_SEND_FD, "sock=%p fd=%d", sock, fd); if (sendfd(sock->fd, fd) < 0) { @@ -1534,7 +1523,7 @@ int virNetSocketSendFD(virNetSocketPtr sock, int fd) ret = 1; cleanup: - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); return ret; } @@ -1553,7 +1542,7 @@ int virNetSocketRecvFD(virNetSocketPtr sock, int *fd) _("Receiving file descriptors is not supported on this socket")); return -1; } - virMutexLock(&sock->lock); + virObjectLock(sock); if ((*fd = recvfd(sock->fd, O_CLOEXEC)) < 0) { if (errno == EAGAIN) @@ -1568,20 +1557,20 @@ int virNetSocketRecvFD(virNetSocketPtr sock, int *fd) ret = 1; cleanup: - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); return ret; } int virNetSocketListen(virNetSocketPtr sock, int backlog) { - virMutexLock(&sock->lock); + virObjectLock(sock); if (listen(sock->fd, backlog > 0 ? backlog : 30) < 0) { virReportSystemError(errno, "%s", _("Unable to listen on socket")); - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); return -1; } - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); return 0; } @@ -1592,7 +1581,7 @@ int virNetSocketAccept(virNetSocketPtr sock, virNetSocketPtr *clientsock) virSocketAddr remoteAddr; int ret = -1; - virMutexLock(&sock->lock); + virObjectLock(sock); *clientsock = NULL; @@ -1629,7 +1618,7 @@ int virNetSocketAccept(virNetSocketPtr sock, virNetSocketPtr *clientsock) cleanup: VIR_FORCE_CLOSE(fd); - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); return ret; } @@ -1643,10 +1632,10 @@ static void virNetSocketEventHandle(int watch ATTRIBUTE_UNUSED, virNetSocketIOFunc func; void *eopaque; - virMutexLock(&sock->lock); + virObjectLock(sock); func = sock->func; eopaque = sock->opaque; - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); if (func) func(sock, events, eopaque); @@ -1659,13 +1648,13 @@ static void virNetSocketEventFree(void *opaque) virFreeCallback ff; void *eopaque; - virMutexLock(&sock->lock); + virObjectLock(sock); ff = sock->ff; eopaque = sock->opaque; sock->func = NULL; sock->ff = NULL; sock->opaque = NULL; - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); if (ff) ff(eopaque); @@ -1682,7 +1671,7 @@ int virNetSocketAddIOCallback(virNetSocketPtr sock, int ret = -1; virObjectRef(sock); - virMutexLock(&sock->lock); + virObjectLock(sock); if (sock->watch > 0) { VIR_DEBUG("Watch already registered on socket %p", sock); goto cleanup; @@ -1703,7 +1692,7 @@ int virNetSocketAddIOCallback(virNetSocketPtr sock, ret = 0; cleanup: - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); if (ret != 0) virObjectUnref(sock); return ret; @@ -1712,31 +1701,31 @@ cleanup: void virNetSocketUpdateIOCallback(virNetSocketPtr sock, int events) { - virMutexLock(&sock->lock); + virObjectLock(sock); if (sock->watch <= 0) { VIR_DEBUG("Watch not registered on socket %p", sock); - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); return; } virEventUpdateHandle(sock->watch, events); - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); } void virNetSocketRemoveIOCallback(virNetSocketPtr sock) { - virMutexLock(&sock->lock); + virObjectLock(sock); if (sock->watch <= 0) { VIR_DEBUG("Watch not registered on socket %p", sock); - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); return; } virEventRemoveHandle(sock->watch); - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); } void virNetSocketClose(virNetSocketPtr sock) @@ -1744,7 +1733,7 @@ void virNetSocketClose(virNetSocketPtr sock) if (!sock) return; - virMutexLock(&sock->lock); + virObjectLock(sock); VIR_FORCE_CLOSE(sock->fd); @@ -1758,5 +1747,5 @@ void virNetSocketClose(virNetSocketPtr sock) } #endif - virMutexUnlock(&sock->lock); + virObjectUnlock(sock); } diff --git a/src/rpc/virnetsshsession.c b/src/rpc/virnetsshsession.c index ca7d52e..9ce99f8 100644 --- a/src/rpc/virnetsshsession.c +++ b/src/rpc/virnetsshsession.c @@ -78,9 +78,8 @@ struct _virNetSSHAuthMethod { }; struct _virNetSSHSession { - virObject object; + virObjectLockable parent; virNetSSHSessionState state; - virMutex lock; /* libssh2 internal stuff */ LIBSSH2_SESSION *session; @@ -161,7 +160,7 @@ static virClassPtr virNetSSHSessionClass; static int virNetSSHSessionOnceInit(void) { - if (!(virNetSSHSessionClass = virClassNew(virClassForObject(), + if (!(virNetSSHSessionClass = virClassNew(virClassForObjectLockable(), "virNetSSHSession", sizeof(virNetSSHSession), virNetSSHSessionDispose))) @@ -927,18 +926,18 @@ int virNetSSHSessionAuthSetCallback(virNetSSHSessionPtr sess, virConnectAuthPtr auth) { - virMutexLock(&sess->lock); + virObjectLock(sess); sess->cred = auth; - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); return 0; } void virNetSSHSessionAuthReset(virNetSSHSessionPtr sess) { - virMutexLock(&sess->lock); + virObjectLock(sess); virNetSSHSessionAuthMethodsFree(sess); - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); } int @@ -957,7 +956,7 @@ virNetSSHSessionAuthAddPasswordAuth(virNetSSHSessionPtr sess, return -1; } - virMutexLock(&sess->lock); + virObjectLock(sess); if (!(user = strdup(username)) || !(pass = strdup(password))) @@ -970,14 +969,14 @@ virNetSSHSessionAuthAddPasswordAuth(virNetSSHSessionPtr sess, auth->password = pass; auth->method = VIR_NET_SSH_AUTH_PASSWORD; - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); return 0; no_memory: VIR_FREE(user); VIR_FREE(pass); virReportOOMError(); - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); return -1; } @@ -995,7 +994,7 @@ virNetSSHSessionAuthAddAgentAuth(virNetSSHSessionPtr sess, return -1; } - virMutexLock(&sess->lock); + virObjectLock(sess); if (!(user = strdup(username))) goto no_memory; @@ -1006,13 +1005,13 @@ virNetSSHSessionAuthAddAgentAuth(virNetSSHSessionPtr sess, auth->username = user; auth->method = VIR_NET_SSH_AUTH_AGENT; - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); return 0; no_memory: VIR_FREE(user); virReportOOMError(); - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); return -1; } @@ -1035,7 +1034,7 @@ virNetSSHSessionAuthAddPrivKeyAuth(virNetSSHSessionPtr sess, return -1; } - virMutexLock(&sess->lock); + virObjectLock(sess); if (!(user = strdup(username)) || !(file = strdup(keyfile))) @@ -1052,7 +1051,7 @@ virNetSSHSessionAuthAddPrivKeyAuth(virNetSSHSessionPtr sess, auth->filename = file; auth->method = VIR_NET_SSH_AUTH_PRIVKEY; - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); return 0; no_memory: @@ -1060,7 +1059,7 @@ no_memory: VIR_FREE(pass); VIR_FREE(file); virReportOOMError(); - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); return -1; } @@ -1079,7 +1078,7 @@ virNetSSHSessionAuthAddKeyboardAuth(virNetSSHSessionPtr sess, return -1; } - virMutexLock(&sess->lock); + virObjectLock(sess); if (!(user = strdup(username))) goto no_memory; @@ -1091,13 +1090,13 @@ virNetSSHSessionAuthAddKeyboardAuth(virNetSSHSessionPtr sess, auth->tries = tries; auth->method = VIR_NET_SSH_AUTH_KEYBOARD_INTERACTIVE; - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); return 0; no_memory: VIR_FREE(user); virReportOOMError(); - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); return -1; } @@ -1107,7 +1106,7 @@ virNetSSHSessionSetChannelCommand(virNetSSHSessionPtr sess, const char *command) { int ret = 0; - virMutexLock(&sess->lock); + virObjectLock(sess); VIR_FREE(sess->channelCommand); @@ -1116,7 +1115,7 @@ virNetSSHSessionSetChannelCommand(virNetSSHSessionPtr sess, ret = -1; } - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); return ret; } @@ -1130,7 +1129,7 @@ virNetSSHSessionSetHostKeyVerification(virNetSSHSessionPtr sess, { char *errmsg; - virMutexLock(&sess->lock); + virObjectLock(sess); sess->port = port; sess->hostKeyVerify = opt; @@ -1167,13 +1166,13 @@ virNetSSHSessionSetHostKeyVerification(virNetSSHSessionPtr sess, } } - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); return 0; no_memory: virReportOOMError(); error: - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); return -1; } @@ -1185,16 +1184,9 @@ virNetSSHSessionPtr virNetSSHSessionNew(void) if (virNetSSHSessionInitialize() < 0) goto error; - if (!(sess = virObjectNew(virNetSSHSessionClass))) + if (!(sess = virObjectLockableNew(virNetSSHSessionClass))) goto error; - /* initialize internal structures */ - if (virMutexInit(&sess->lock) < 0) { - virReportError(VIR_ERR_SSH, "%s", - _("Failed to initialize mutex")); - goto error; - } - /* initialize session data, use the internal data for callbacks * and stick to default memory management functions */ if (!(sess->session = libssh2_session_init_ex(NULL, @@ -1250,7 +1242,7 @@ virNetSSHSessionConnect(virNetSSHSessionPtr sess, return -1; } - virMutexLock(&sess->lock); + virObjectLock(sess); /* check if configuration is valid */ if ((ret = virNetSSHValidateConfig(sess)) < 0) @@ -1284,12 +1276,12 @@ virNetSSHSessionConnect(virNetSSHSessionPtr sess, libssh2_session_set_blocking(sess->session, 0); sess->state = VIR_NET_SSH_STATE_HANDSHAKE_COMPLETE; - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); return ret; error: sess->state = VIR_NET_SSH_STATE_ERROR; - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); return ret; } @@ -1302,7 +1294,7 @@ virNetSSHChannelRead(virNetSSHSessionPtr sess, ssize_t ret = -1; ssize_t read_n = 0; - virMutexLock(&sess->lock); + virObjectLock(sess); if (sess->state != VIR_NET_SSH_STATE_HANDSHAKE_COMPLETE) { if (sess->state == VIR_NET_SSH_STATE_ERROR_REMOTE) @@ -1314,7 +1306,7 @@ virNetSSHChannelRead(virNetSSHSessionPtr sess, virReportError(VIR_ERR_SSH, "%s", _("Tried to write socket in error state")); - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); return -1; } @@ -1387,22 +1379,22 @@ virNetSSHChannelRead(virNetSSHSessionPtr sess, libssh2_channel_get_exit_status(sess->channel)); sess->channelCommandReturnValue = libssh2_channel_get_exit_status(sess->channel); sess->state = VIR_NET_SSH_STATE_ERROR_REMOTE; - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); return -1; } sess->state = VIR_NET_SSH_STATE_CLOSED; - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); return -1; } success: - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); return read_n; error: sess->state = VIR_NET_SSH_STATE_ERROR; - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); return ret; } @@ -1413,7 +1405,7 @@ virNetSSHChannelWrite(virNetSSHSessionPtr sess, { ssize_t ret; - virMutexLock(&sess->lock); + virObjectLock(sess); if (sess->state != VIR_NET_SSH_STATE_HANDSHAKE_COMPLETE) { if (sess->state == VIR_NET_SSH_STATE_ERROR_REMOTE) @@ -1459,7 +1451,7 @@ virNetSSHChannelWrite(virNetSSHSessionPtr sess, } cleanup: - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); return ret; } @@ -1471,10 +1463,10 @@ virNetSSHSessionHasCachedData(virNetSSHSessionPtr sess) if (!sess) return false; - virMutexLock(&sess->lock); + virObjectLock(sess); ret = sess->bufUsed > 0; - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); return ret; } diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index 3e194f9..0f0ddff 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -50,9 +50,7 @@ #define VIR_FROM_THIS VIR_FROM_RPC struct _virNetTLSContext { - virObject object; - - virMutex lock; + virObjectLockable parent; gnutls_certificate_credentials_t x509cred; gnutls_dh_params_t dhParams; @@ -63,9 +61,7 @@ struct _virNetTLSContext { }; struct _virNetTLSSession { - virObject object; - - virMutex lock; + virObjectLockable parent; bool handshakeComplete; @@ -85,13 +81,13 @@ static void virNetTLSSessionDispose(void *obj); static int virNetTLSContextOnceInit(void) { - if (!(virNetTLSContextClass = virClassNew(virClassForObject(), + if (!(virNetTLSContextClass = virClassNew(virClassForObjectLockable(), "virNetTLSContext", sizeof(virNetTLSContext), virNetTLSContextDispose))) return -1; - if (!(virNetTLSSessionClass = virClassNew(virClassForObject(), + if (!(virNetTLSSessionClass = virClassNew(virClassForObjectLockable(), "virNetTLSSession", sizeof(virNetTLSSession), virNetTLSSessionDispose))) @@ -676,15 +672,8 @@ static virNetTLSContextPtr virNetTLSContextNew(const char *cacert, if (virNetTLSContextInitialize() < 0) return NULL; - if (!(ctxt = virObjectNew(virNetTLSContextClass))) - return NULL; - - if (virMutexInit(&ctxt->lock) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Failed to initialized mutex")); - VIR_FREE(ctxt); + if (!(ctxt = virObjectLockableNew(virNetTLSContextClass))) return NULL; - } if ((gnutlsdebug = getenv("LIBVIRT_GNUTLS_DEBUG")) != NULL) { int val; @@ -1097,8 +1086,8 @@ int virNetTLSContextCheckCertificate(virNetTLSContextPtr ctxt, { int ret = -1; - virMutexLock(&ctxt->lock); - virMutexLock(&sess->lock); + virObjectLock(ctxt); + virObjectLock(sess); if (virNetTLSContextValidCertificate(ctxt, sess) < 0) { virErrorPtr err = virGetLastError(); VIR_WARN("Certificate check failed %s", err && err->message ? err->message : "<unknown>"); @@ -1114,8 +1103,8 @@ int virNetTLSContextCheckCertificate(virNetTLSContextPtr ctxt, ret = 0; cleanup: - virMutexUnlock(&ctxt->lock); - virMutexUnlock(&sess->lock); + virObjectUnlock(ctxt); + virObjectUnlock(sess); return ret; } @@ -1126,7 +1115,6 @@ void virNetTLSContextDispose(void *obj) gnutls_dh_params_deinit(ctxt->dhParams); gnutls_certificate_free_credentials(ctxt->x509cred); - virMutexDestroy(&ctxt->lock); } @@ -1167,16 +1155,9 @@ virNetTLSSessionPtr virNetTLSSessionNew(virNetTLSContextPtr ctxt, VIR_DEBUG("ctxt=%p hostname=%s isServer=%d", ctxt, NULLSTR(hostname), ctxt->isServer); - if (!(sess = virObjectNew(virNetTLSSessionClass))) + if (!(sess = virObjectLockableNew(virNetTLSSessionClass))) return NULL; - if (virMutexInit(&sess->lock) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Failed to initialized mutex")); - VIR_FREE(sess); - return NULL; - } - if (hostname && !(sess->hostname = strdup(hostname))) { virReportOOMError(); @@ -1243,11 +1224,11 @@ void virNetTLSSessionSetIOCallbacks(virNetTLSSessionPtr sess, virNetTLSSessionReadFunc readFunc, void *opaque) { - virMutexLock(&sess->lock); + virObjectLock(sess); sess->writeFunc = writeFunc; sess->readFunc = readFunc; sess->opaque = opaque; - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); } @@ -1256,7 +1237,7 @@ ssize_t virNetTLSSessionWrite(virNetTLSSessionPtr sess, { ssize_t ret; - virMutexLock(&sess->lock); + virObjectLock(sess); ret = gnutls_record_send(sess->session, buf, len); if (ret >= 0) @@ -1280,7 +1261,7 @@ ssize_t virNetTLSSessionWrite(virNetTLSSessionPtr sess, ret = -1; cleanup: - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); return ret; } @@ -1289,7 +1270,7 @@ ssize_t virNetTLSSessionRead(virNetTLSSessionPtr sess, { ssize_t ret; - virMutexLock(&sess->lock); + virObjectLock(sess); ret = gnutls_record_recv(sess->session, buf, len); if (ret >= 0) @@ -1310,7 +1291,7 @@ ssize_t virNetTLSSessionRead(virNetTLSSessionPtr sess, ret = -1; cleanup: - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); return ret; } @@ -1318,7 +1299,7 @@ int virNetTLSSessionHandshake(virNetTLSSessionPtr sess) { int ret; VIR_DEBUG("sess=%p", sess); - virMutexLock(&sess->lock); + virObjectLock(sess); ret = gnutls_handshake(sess->session); VIR_DEBUG("Ret=%d", ret); if (ret == 0) { @@ -1342,7 +1323,7 @@ int virNetTLSSessionHandshake(virNetTLSSessionPtr sess) ret = -1; cleanup: - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); return ret; } @@ -1350,14 +1331,14 @@ virNetTLSSessionHandshakeStatus virNetTLSSessionGetHandshakeStatus(virNetTLSSessionPtr sess) { virNetTLSSessionHandshakeStatus ret; - virMutexLock(&sess->lock); + virObjectLock(sess); if (sess->handshakeComplete) ret = VIR_NET_TLS_HANDSHAKE_COMPLETE; else if (gnutls_record_get_direction(sess->session) == 0) ret = VIR_NET_TLS_HANDSHAKE_RECVING; else ret = VIR_NET_TLS_HANDSHAKE_SENDING; - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); return ret; } @@ -1365,7 +1346,7 @@ int virNetTLSSessionGetKeySize(virNetTLSSessionPtr sess) { gnutls_cipher_algorithm_t cipher; int ssf; - virMutexLock(&sess->lock); + virObjectLock(sess); cipher = gnutls_cipher_get(sess->session); if (!(ssf = gnutls_cipher_get_key_size(cipher))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -1375,7 +1356,7 @@ int virNetTLSSessionGetKeySize(virNetTLSSessionPtr sess) } cleanup: - virMutexUnlock(&sess->lock); + virObjectUnlock(sess); return ssf; } @@ -1386,7 +1367,6 @@ void virNetTLSSessionDispose(void *obj) VIR_FREE(sess->hostname); gnutls_deinit(sess->session); - virMutexDestroy(&sess->lock); } /* -- 1.8.0.1
On 01/11/2013 05:13 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
--- src/rpc/virkeepalive.c | 55 +++++----------- src/rpc/virnetclient.c | 128 +++++++++++++++-------------------- src/rpc/virnetclientstream.c | 67 ++++++++----------- src/rpc/virnetsaslcontext.c | 109 ++++++++++-------------------- src/rpc/virnetserver.c | 117 ++++++++++++++------------------ src/rpc/virnetserverclient.c | 154 +++++++++++++++++++------------------------ src/rpc/virnetsocket.c | 125 ++++++++++++++++------------------- src/rpc/virnetsshsession.c | 82 +++++++++++------------ src/rpc/virnettlscontext.c | 64 +++++++----------- 9 files changed, 366 insertions(+), 535 deletions(-)
Another mostly-mechanical cleanup. ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Introduce a virPortAllocator for managing TCP port allocations. --- .gitignore | 1 + src/Makefile.am | 1 + src/libvirt_private.syms | 6 ++ src/util/virportallocator.c | 188 +++++++++++++++++++++++++++++++++++++++++ src/util/virportallocator.h | 40 +++++++++ tests/Makefile.am | 17 +++- tests/virportallocatortest.c | 195 +++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 447 insertions(+), 1 deletion(-) create mode 100644 src/util/virportallocator.c create mode 100644 src/util/virportallocator.h create mode 100644 tests/virportallocatortest.c diff --git a/.gitignore b/.gitignore index 882ae4c..e5d09fa 100644 --- a/.gitignore +++ b/.gitignore @@ -175,6 +175,7 @@ /tests/virkeyfiletest /tests/virlockspacetest /tests/virnet*test +/tests/virportallocatortest /tests/virshtest /tests/virstringtest /tests/virtimetest diff --git a/src/Makefile.am b/src/Makefile.am index da571c7..df58933 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -97,6 +97,7 @@ UTIL_SOURCES = \ util/virobject.c util/virobject.h \ util/virpci.c util/virpci.h \ util/virpidfile.c util/virpidfile.h \ + util/virportallocator.c util/virportallocator.h \ util/virprocess.c util/virprocess.h \ util/virrandom.h util/virrandom.c \ util/virsexpr.c util/virsexpr.h \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cbdbb32..a4e818b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1796,6 +1796,12 @@ virPidFileWrite; virPidFileWritePath; +# virportallocator.h +virPortAllocatorAcquire; +virPortAllocatorNew; +virPortAllocatorRelease; + + # virprocess.h virProcessAbort; virProcessGetAffinity; diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c new file mode 100644 index 0000000..2c27711 --- /dev/null +++ b/src/util/virportallocator.c @@ -0,0 +1,188 @@ +/* + * virportallocator.c: Allocate & track TCP port allocations + * + * Copyright (C) 2013 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + */ + +#include <config.h> + +#include <sys/socket.h> +#include <arpa/inet.h> + +#include "viralloc.h" +#include "virbitmap.h" +#include "virportallocator.h" +#include "virthread.h" +#include "virerror.h" +#include "virfile.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +struct _virPortAllocator { + virObjectLockable parent; + virBitmapPtr bitmap; + + unsigned int start; + unsigned int end; +}; + +static virClassPtr virPortAllocatorClass; + +static void +virPortAllocatorDispose(void *obj) +{ + virPortAllocatorPtr pa = obj; + + virBitmapFree(pa->bitmap); +} + +static int virPortAllocatorOnceInit(void) +{ + if (!(virPortAllocatorClass = virClassNew(virClassForObjectLockable(), + "virPortAllocator", + sizeof(virPortAllocator), + virPortAllocatorDispose))) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virPortAllocator) + +virPortAllocatorPtr virPortAllocatorNew(unsigned short start, + unsigned short end) + +{ + virPortAllocatorPtr pa; + + if (start >= end) { + virReportInvalidArg(start, "start port %d must be less than end port %d", + start, end); + return NULL; + } + + if (virPortAllocatorInitialize() < 0) + return NULL; + + if (!(pa = virObjectLockableNew(virPortAllocatorClass))) + return NULL; + + pa->start = start; + pa->end = end; + + if (!(pa->bitmap = virBitmapNew(end-start))) { + virObjectUnref(pa); + return NULL; + } + + return pa; +} + +int virPortAllocatorAcquire(virPortAllocatorPtr pa, + unsigned short *port) +{ + int ret = -1; + unsigned short i; + int fd = -1; + + *port = 0; + virObjectLock(pa); + + for (i = pa->start ; i < pa->end && fd == -1; i++) { + int reuse = 1; + struct sockaddr_in addr; + bool used = false; + + if (virBitmapGetBit(pa->bitmap, + i - pa->start, &used) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to query port %d"), i); + goto cleanup; + } + + if (used) + continue; + + addr.sin_family = AF_INET; + addr.sin_port = htons(i); + addr.sin_addr.s_addr = htonl(INADDR_ANY); + fd = socket(PF_INET, SOCK_STREAM, 0); + if (fd < 0) { + virReportSystemError(errno, "%s", + _("Unable to open test socket")); + goto cleanup; + } + + if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (void*)&reuse, sizeof(reuse)) < 0) { + virReportSystemError(errno, "%s", + _("Unable to set socket reuse addr flag")); + goto cleanup; + } + + if (bind(fd, (struct sockaddr*)&addr, sizeof(addr)) < 0) { + if (errno != EADDRINUSE) { + virReportSystemError(errno, + _("Unable to bind to port %d"), i); + goto cleanup; + } + /* In use, try next */ + VIR_FORCE_CLOSE(fd); + } else { + /* Add port to bitmap of reserved ports */ + if (virBitmapSetBit(pa->bitmap, + i - pa->start) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to reserve port %d"), i); + goto cleanup; + } + } + } + + ret = 0; +cleanup: + virObjectUnlock(pa); + VIR_FORCE_CLOSE(fd); + return ret; +} + +int virPortAllocatorRelease(virPortAllocatorPtr pa, + unsigned short port) +{ + int ret = -1; + virObjectLock(pa); + + if (port < pa->start || + port >= pa->end) { + virReportInvalidArg(port, "port %d must be in range (%d, %d)", + port, pa->start, pa->end); + goto cleanup; + } + + if (virBitmapClearBit(pa->bitmap, + port - pa->start) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to release port %d"), + port); + goto cleanup; + } + + ret = 0; +cleanup: + virObjectUnlock(pa); + return ret; +} diff --git a/src/util/virportallocator.h b/src/util/virportallocator.h new file mode 100644 index 0000000..a5e68f7 --- /dev/null +++ b/src/util/virportallocator.h @@ -0,0 +1,40 @@ +/* + * virportallocator.h: Allocate & track TCP port allocations + * + * Copyright (C) 2013 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + */ + +#ifndef __VIR_PORT_ALLOCATOR_H__ +# define __VIR_PORT_ALLOCATOR_H__ + +# include "internal.h" +# include "virobject.h" + +typedef struct _virPortAllocator virPortAllocator; +typedef virPortAllocator *virPortAllocatorPtr; + +virPortAllocatorPtr virPortAllocatorNew(unsigned short start, + unsigned short end); + +int virPortAllocatorAcquire(virPortAllocatorPtr pa, + unsigned short *port); + +int virPortAllocatorRelease(virPortAllocatorPtr pa, + unsigned short port); + +#endif /* __VIR_PORT_ALLOCATOR_H__ */ diff --git a/tests/Makefile.am b/tests/Makefile.am index 9c7c6fb..d2f27fa 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -97,6 +97,7 @@ test_programs = virshtest sockettest \ virbitmaptest \ virlockspacetest \ virstringtest \ + virportallocatortest \ sysinfotest \ $(NULL) @@ -231,7 +232,9 @@ endif EXTRA_DIST += $(test_scripts) -test_libraries = libshunload.la +test_libraries = libshunload.la \ + libvirportallocatormock.la \ + $(NULL) if WITH_QEMU test_libraries += libqemumonitortestutils.la endif @@ -561,6 +564,18 @@ virlockspacetest_SOURCES = \ virlockspacetest_CFLAGS = -Dabs_builddir="\"$(abs_builddir)\"" $(AM_CFLAGS) virlockspacetest_LDADD = $(LDADDS) +virportallocatortest_SOURCES = \ + virportallocatortest.c testutils.h testutils.c +virportallocatortest_CFLAGS = -Dabs_builddir="\"$(abs_builddir)\"" $(AM_CFLAGS) +virportallocatortest_LDADD = $(LDADDS) + +libvirportallocatormock_la_SOURCES = \ + virportallocatortest.c +libvirportallocatormock_la_CFLAGS = $(AM_CFLAGS) -DMOCK_HELPER=1 +libvirportallocatormock_la_LDFLAGS = -module -avoid-version \ + -rpath /evil/libtool/hack/to/force/shared/lib/creation + + viruritest_SOURCES = \ viruritest.c testutils.h testutils.c viruritest_CFLAGS = -Dabs_builddir="\"$(abs_builddir)\"" $(AM_CFLAGS) diff --git a/tests/virportallocatortest.c b/tests/virportallocatortest.c new file mode 100644 index 0000000..e448be6 --- /dev/null +++ b/tests/virportallocatortest.c @@ -0,0 +1,195 @@ +/* + * Copyright (C) 2013 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + +#include <config.h> + +#ifdef MOCK_HELPER +# include "internal.h" +# include <sys/socket.h> +# include <errno.h> +# include <arpa/inet.h> + +int bind(int sockfd ATTRIBUTE_UNUSED, + const struct sockaddr *addr, + socklen_t addrlen ATTRIBUTE_UNUSED) +{ + struct sockaddr_in *saddr = (struct sockaddr_in *)addr; + + if (saddr->sin_port == htons(5900) || + saddr->sin_port == htons(5904) || + saddr->sin_port == htons(5905) || + saddr->sin_port == htons(5906)) { + errno = EADDRINUSE; + return -1; + } + + return 0; +} + +#else +# include <stdlib.h> +# include <signal.h> + +# include "testutils.h" +# include "virutil.h" +# include "virerror.h" +# include "viralloc.h" +# include "virlog.h" + +# include "virportallocator.h" + +# define VIR_FROM_THIS VIR_FROM_RPC + + +static int testAllocAll(const void *args ATTRIBUTE_UNUSED) +{ + virPortAllocatorPtr alloc = virPortAllocatorNew(5900, 5910); + int ret = -1; + unsigned short p1, p2, p3, p4, p5, p6, p7; + + if (virPortAllocatorAcquire(alloc, &p1) < 0) + goto cleanup; + if (p1 != 5901) { + if (virTestGetDebug()) + fprintf(stderr, "Expected 5901, got %d", p1); + goto cleanup; + } + + if (virPortAllocatorAcquire(alloc, &p2) < 0) + goto cleanup; + if (p2 != 5902) { + if (virTestGetDebug()) + fprintf(stderr, "Expected 5902, got %d", p2); + goto cleanup; + } + + if (virPortAllocatorAcquire(alloc, &p3) < 0) + goto cleanup; + if (p3 != 5903) { + if (virTestGetDebug()) + fprintf(stderr, "Expected 5903, got %d", p3); + goto cleanup; + } + + if (virPortAllocatorAcquire(alloc, &p4) < 0) + goto cleanup; + if (p4 != 5907) { + if (virTestGetDebug()) + fprintf(stderr, "Expected 5907, got %d", p4); + goto cleanup; + } + + if (virPortAllocatorAcquire(alloc, &p5) < 0) + goto cleanup; + if (p5 != 5908) { + if (virTestGetDebug()) + fprintf(stderr, "Expected 5908, got %d", p5); + goto cleanup; + } + + if (virPortAllocatorAcquire(alloc, &p6) < 0) + goto cleanup; + if (p6 != 5909) { + if (virTestGetDebug()) + fprintf(stderr, "Expected 5909, got %d", p6); + goto cleanup; + } + + if (virPortAllocatorAcquire(alloc, &p7) < 0) + goto cleanup; + if (p7 != 0) { + if (virTestGetDebug()) + fprintf(stderr, "Expected 0, got %d", p7); + goto cleanup; + } + + ret = 0; +cleanup: + virObjectUnref(alloc); + return ret; +} + + + +static int testAllocReuse(const void *args ATTRIBUTE_UNUSED) +{ + virPortAllocatorPtr alloc = virPortAllocatorNew(5900, 5910); + int ret = -1; + unsigned short p1, p2, p3, p4; + + if (virPortAllocatorAcquire(alloc, &p1) < 0) + goto cleanup; + if (p1 != 5901) { + if (virTestGetDebug()) + fprintf(stderr, "Expected 5901, got %d", p1); + goto cleanup; + } + + if (virPortAllocatorAcquire(alloc, &p2) < 0) + goto cleanup; + if (p2 != 5902) { + if (virTestGetDebug()) + fprintf(stderr, "Expected 5902, got %d", p2); + goto cleanup; + } + + if (virPortAllocatorAcquire(alloc, &p3) < 0) + goto cleanup; + if (p3 != 5903) { + if (virTestGetDebug()) + fprintf(stderr, "Expected 5903, got %d", p3); + goto cleanup; + } + + + if (virPortAllocatorRelease(alloc, p2) < 0) + goto cleanup; + + if (virPortAllocatorAcquire(alloc, &p4) < 0) + goto cleanup; + if (p4 != 5902) { + if (virTestGetDebug()) + fprintf(stderr, "Expected 5902, got %d", p4); + goto cleanup; + } + + ret = 0; +cleanup: + virObjectUnref(alloc); + return ret; +} + + +static int +mymain(void) +{ + int ret = 0; + + if (virtTestRun("Test alloc all", 1, testAllocAll, NULL) < 0) + ret = -1; + + if (virtTestRun("Test alloc reuse", 1, testAllocReuse, NULL) < 0) + ret = -1; + + return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/libvirportallocatormock.so") +#endif -- 1.8.0.1
On 01/11/2013 05:13 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Introduce a virPortAllocator for managing TCP port allocations. --- .gitignore | 1 + src/Makefile.am | 1 + src/libvirt_private.syms | 6 ++ src/util/virportallocator.c | 188 +++++++++++++++++++++++++++++++++++++++++ src/util/virportallocator.h | 40 +++++++++ tests/Makefile.am | 17 +++- tests/virportallocatortest.c | 195 +++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 447 insertions(+), 1 deletion(-) create mode 100644 src/util/virportallocator.c create mode 100644 src/util/virportallocator.h create mode 100644 tests/virportallocatortest.c
+ + unsigned int start; + unsigned int end; +};
+ +virPortAllocatorPtr virPortAllocatorNew(unsigned short start, + unsigned short end) + +{
Spurious blank line. Any reason you allocate with short, but store the values in int internally? (unsigned short in the struct should be fine)
+ virPortAllocatorPtr pa; + + if (start >= end) { + virReportInvalidArg(start, "start port %d must be less than end port %d", + start, end); + return NULL; + }
Since this error gave a message,
+ + if (virPortAllocatorInitialize() < 0) + return NULL; + + if (!(pa = virObjectLockableNew(virPortAllocatorClass))) + return NULL;
does this error need to call virReportOOMError()?
+ + pa->start = start; + pa->end = end; + + if (!(pa->bitmap = virBitmapNew(end-start))) { + virObjectUnref(pa); + return NULL;
Same question here. Callers can't tell if a NULL return means OOM or usage error.
+++ b/tests/Makefile.am @@ -97,6 +97,7 @@ test_programs = virshtest sockettest \ virbitmaptest \ virlockspacetest \ virstringtest \ + virportallocatortest \
Space vs. tab issue (one of the few files where we prefer tab).
+ +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/libvirportallocatormock.so")
Nice way to fake out bind() - this PRELOAD test idiom is turning out to be useful. ACK if you address the points above. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Tue, Jan 15, 2013 at 05:30:23PM -0700, Eric Blake wrote:
On 01/11/2013 05:13 AM, Daniel P. Berrange wrote:
+ + unsigned int start; + unsigned int end; +};
+ +virPortAllocatorPtr virPortAllocatorNew(unsigned short start, + unsigned short end) + +{
Spurious blank line. Any reason you allocate with short, but store the values in int internally? (unsigned short in the struct should be fine)
Yes, using a short in the struct would be better & was my intention.
+ virPortAllocatorPtr pa; + + if (start >= end) { + virReportInvalidArg(start, "start port %d must be less than end port %d", + start, end); + return NULL; + }
Since this error gave a message,
+ + if (virPortAllocatorInitialize() < 0) + return NULL; + + if (!(pa = virObjectLockableNew(virPortAllocatorClass))) + return NULL;
does this error need to call virReportOOMError()?
This function raises an error already.
+ + pa->start = start; + pa->end = end; + + if (!(pa->bitmap = virBitmapNew(end-start))) { + virObjectUnref(pa); + return NULL;
Same question here. Callers can't tell if a NULL return means OOM or usage error.
I thought this did too, but it appears to leave it upto the caller, so I'll add it here. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
From: "Daniel P. Berrange" <berrange@redhat.com> Replace the current QEMU driver code for managing port reservations with the new virPortAllocator APIs. --- src/qemu/qemu_conf.h | 4 +- src/qemu/qemu_driver.c | 9 ++-- src/qemu/qemu_process.c | 100 ++++++++------------------------------------ src/util/virportallocator.c | 3 +- 4 files changed, 27 insertions(+), 89 deletions(-) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 965eff7..6009118 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -39,7 +39,7 @@ # include "virusb.h" # include "cpu_conf.h" # include "driver.h" -# include "virbitmap.h" +# include "virportallocator.h" # include "vircommand.h" # include "virthreadpool.h" # include "locking/lock_manager.h" @@ -150,7 +150,7 @@ struct _virQEMUDriver { virHashTablePtr sharedDisks; - virBitmapPtr reservedRemotePorts; + virPortAllocatorPtr remotePorts; virSysinfoDefPtr hostsysinfo; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ab1fd4c..2147983 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -809,9 +809,10 @@ qemuStartup(bool privileged, /* Allocate bitmap for remote display port reservations. We cannot * do this before the config is loaded properly, since the port * numbers are configurable now */ - if ((qemu_driver->reservedRemotePorts = - virBitmapNew(qemu_driver->remotePortMax - qemu_driver->remotePortMin)) == NULL) - goto out_of_memory; + if ((qemu_driver->remotePorts = + virPortAllocatorNew(qemu_driver->remotePortMin, + qemu_driver->remotePortMax)) == NULL) + goto error; /* We should always at least have the 'nop' manager, so * NULLs here are a fatal error @@ -1104,7 +1105,7 @@ qemuShutdown(void) { qemuCapsCacheFree(qemu_driver->capsCache); virDomainObjListDeinit(&qemu_driver->domains); - virBitmapFree(qemu_driver->reservedRemotePorts); + virObjectUnref(qemu_driver->remotePorts); virSysinfoDefFree(qemu_driver->hostsysinfo); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c8c898f..84ba53b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2603,73 +2603,6 @@ qemuProcessInitPCIAddresses(virQEMUDriverPtr driver, } -static int qemuProcessNextFreePort(virQEMUDriverPtr driver, - int startPort) -{ - int i; - - for (i = startPort ; i < driver->remotePortMax; i++) { - int fd; - int reuse = 1; - struct sockaddr_in addr; - bool used = false; - - if (virBitmapGetBit(driver->reservedRemotePorts, - i - driver->remotePortMin, &used) < 0) - VIR_DEBUG("virBitmapGetBit failed on bit %d", i - driver->remotePortMin); - - if (used) - continue; - - addr.sin_family = AF_INET; - addr.sin_port = htons(i); - addr.sin_addr.s_addr = htonl(INADDR_ANY); - fd = socket(PF_INET, SOCK_STREAM, 0); - if (fd < 0) - return -1; - - if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (void*)&reuse, sizeof(reuse)) < 0) { - VIR_FORCE_CLOSE(fd); - break; - } - - if (bind(fd, (struct sockaddr*)&addr, sizeof(addr)) == 0) { - /* Not in use, lets grab it */ - VIR_FORCE_CLOSE(fd); - /* Add port to bitmap of reserved ports */ - if (virBitmapSetBit(driver->reservedRemotePorts, - i - driver->remotePortMin) < 0) { - VIR_DEBUG("virBitmapSetBit failed on bit %d", - i - driver->remotePortMin); - } - return i; - } - VIR_FORCE_CLOSE(fd); - - if (errno == EADDRINUSE) { - /* In use, try next */ - continue; - } - /* Some other bad failure, get out.. */ - break; - } - return -1; -} - - -static void -qemuProcessReturnPort(virQEMUDriverPtr driver, - int port) -{ - if (port < driver->remotePortMin) - return; - - if (virBitmapClearBit(driver->reservedRemotePorts, - port - driver->remotePortMin) < 0) - VIR_DEBUG("Could not mark port %d as unused", port); -} - - static int qemuProcessPrepareChardevDevice(virDomainDefPtr def ATTRIBUTE_UNUSED, virDomainChrDefPtr dev, @@ -3664,20 +3597,18 @@ int qemuProcessStart(virConnectPtr conn, if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && !graphics->data.vnc.socket && graphics->data.vnc.autoport) { - int port = qemuProcessNextFreePort(driver, driver->remotePortMin); - if (port < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Unable to find an unused port for VNC")); + unsigned short port; + if (virPortAllocatorAcquire(driver->remotePorts, &port) < 0) goto cleanup; - } graphics->data.vnc.port = port; } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { - int port = -1; + unsigned short port = 0; if (graphics->data.spice.autoport || graphics->data.spice.port == -1) { - port = qemuProcessNextFreePort(driver, driver->remotePortMin); + if (virPortAllocatorAcquire(driver->remotePorts, &port) < 0) + goto cleanup; - if (port < 0) { + if (port == 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find an unused port for SPICE")); goto cleanup; @@ -3688,12 +3619,14 @@ int qemuProcessStart(virConnectPtr conn, if (driver->spiceTLS && (graphics->data.spice.autoport || graphics->data.spice.tlsPort == -1)) { - int tlsPort = qemuProcessNextFreePort(driver, - graphics->data.spice.port + 1); - if (tlsPort < 0) { + unsigned short tlsPort; + if (virPortAllocatorAcquire(driver->remotePorts, &tlsPort) < 0) + goto cleanup; + + if (tlsPort == 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find an unused port for SPICE TLS")); - qemuProcessReturnPort(driver, port); + virPortAllocatorRelease(driver->remotePorts, port); goto cleanup; } @@ -4362,12 +4295,15 @@ retry: virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && graphics->data.vnc.autoport) { - qemuProcessReturnPort(driver, graphics->data.vnc.port); + ignore_value(virPortAllocatorRelease(driver->remotePorts, + graphics->data.vnc.port)); } if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && graphics->data.spice.autoport) { - qemuProcessReturnPort(driver, graphics->data.spice.port); - qemuProcessReturnPort(driver, graphics->data.spice.tlsPort); + ignore_value(virPortAllocatorRelease(driver->remotePorts, + graphics->data.spice.port)); + ignore_value(virPortAllocatorRelease(driver->remotePorts, + graphics->data.spice.tlsPort)); } } diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c index 2c27711..1b45d33 100644 --- a/src/util/virportallocator.c +++ b/src/util/virportallocator.c @@ -103,7 +103,7 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa, *port = 0; virObjectLock(pa); - for (i = pa->start ; i < pa->end && fd == -1; i++) { + for (i = pa->start ; i < pa->end && !*port; i++) { int reuse = 1; struct sockaddr_in addr; bool used = false; @@ -150,6 +150,7 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa, _("Failed to reserve port %d"), i); goto cleanup; } + *port = i; } } -- 1.8.0.1
On 01/11/2013 05:13 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Replace the current QEMU driver code for managing port reservations with the new virPortAllocator APIs. --- src/qemu/qemu_conf.h | 4 +- src/qemu/qemu_driver.c | 9 ++-- src/qemu/qemu_process.c | 100 ++++++++------------------------------------ src/util/virportallocator.c | 3 +- 4 files changed, 27 insertions(+), 89 deletions(-)
Nice reduction in size by reusing common code.
+++ b/src/util/virportallocator.c @@ -103,7 +103,7 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa, *port = 0; virObjectLock(pa);
- for (i = pa->start ; i < pa->end && fd == -1; i++) { + for (i = pa->start ; i < pa->end && !*port; i++) { int reuse = 1; struct sockaddr_in addr; bool used = false; @@ -150,6 +150,7 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa, _("Failed to reserve port %d"), i); goto cleanup; } + *port = i; } }
These two hunks should be squashed into 5/7. ACK with that change. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Replace the current libxl driver code for managing port reservations with the new virPortAllocator APIs. --- src/libxl/libxl_conf.c | 62 ++++-------------------------------------------- src/libxl/libxl_conf.h | 4 ++-- src/libxl/libxl_driver.c | 13 +++++----- 3 files changed, 14 insertions(+), 65 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 2705e65..9245a24 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -64,60 +64,6 @@ static const char *xen_cap_re = "(xen|hvm)-[[:digit:]]+\\.[[:digit:]]+-(x86_32|x static regex_t xen_cap_rec; -static int -libxlNextFreeVncPort(libxlDriverPrivatePtr driver, int startPort) -{ - int i; - - for (i = startPort ; i < LIBXL_VNC_PORT_MAX; i++) { - int fd; - int reuse = 1; - struct sockaddr_in addr; - bool used = false; - - if (virBitmapGetBit(driver->reservedVNCPorts, - i - LIBXL_VNC_PORT_MIN, &used) < 0) - VIR_DEBUG("virBitmapGetBit failed on bit %d", i - LIBXL_VNC_PORT_MIN); - - if (used) - continue; - - addr.sin_family = AF_INET; - addr.sin_port = htons(i); - addr.sin_addr.s_addr = htonl(INADDR_ANY); - fd = socket(PF_INET, SOCK_STREAM, 0); - if (fd < 0) - return -1; - - if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (void*)&reuse, sizeof(reuse)) < 0) { - VIR_FORCE_CLOSE(fd); - break; - } - - if (bind(fd, (struct sockaddr*)&addr, sizeof(addr)) == 0) { - /* Not in use, lets grab it */ - VIR_FORCE_CLOSE(fd); - /* Add port to bitmap of reserved ports */ - if (virBitmapSetBit(driver->reservedVNCPorts, - i - LIBXL_VNC_PORT_MIN) < 0) { - VIR_DEBUG("virBitmapSetBit failed on bit %d", - i - LIBXL_VNC_PORT_MIN); - } - return i; - } - VIR_FORCE_CLOSE(fd); - - if (errno == EADDRINUSE) { - /* In use, try next */ - continue; - } - /* Some other bad failure, get out.. */ - break; - } - return -1; -} - - static int libxlDefaultConsoleType(const char *ostype, virArch arch ATTRIBUTE_UNUSED) { @@ -712,7 +658,7 @@ libxlMakeVfb(libxlDriverPrivatePtr driver, virDomainGraphicsDefPtr l_vfb, libxl_device_vfb *x_vfb) { - int port; + unsigned short port; const char *listenAddr; switch (l_vfb->type) { @@ -735,8 +681,10 @@ libxlMakeVfb(libxlDriverPrivatePtr driver, /* driver handles selection of free port */ libxl_defbool_set(&x_vfb->vnc.findunused, 0); if (l_vfb->data.vnc.autoport) { - port = libxlNextFreeVncPort(driver, LIBXL_VNC_PORT_MIN); - if (port < 0) { + + if (virPortAllocatorAcquire(driver->reservedVNCPorts, &port) < 0) + return -1; + if (port == 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find an unused VNC port")); return -1; diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index c8808a1..a3cce08 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -34,7 +34,7 @@ # include "domain_event.h" # include "capabilities.h" # include "configmake.h" -# include "virbitmap.h" +# include "virportallocator.h" # define LIBXL_VNC_PORT_MIN 5900 @@ -60,7 +60,7 @@ struct _libxlDriverPrivate { /* libxl ctx for driver wide ops; getVersion, getNodeInfo, ... */ libxl_ctx *ctx; - virBitmapPtr reservedVNCPorts; + virPortAllocatorPtr reservedVNCPorts; size_t nactive; virStateInhibitCallback inhibitCallback; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 8017a4a..64a52fb 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -466,8 +466,8 @@ libxlVmCleanup(libxlDriverPrivatePtr driver, vm->def->graphics[0]->data.vnc.autoport) { vnc_port = vm->def->graphics[0]->data.vnc.port; if (vnc_port >= LIBXL_VNC_PORT_MIN) { - if (virBitmapClearBit(driver->reservedVNCPorts, - vnc_port - LIBXL_VNC_PORT_MIN) < 0) + if (virPortAllocatorRelease(driver->reservedVNCPorts, + vnc_port) < 0) VIR_DEBUG("Could not mark port %d as unused", vnc_port); } } @@ -923,7 +923,7 @@ libxlShutdown(void) if (libxl_driver->logger_file) VIR_FORCE_FCLOSE(libxl_driver->logger_file); - virBitmapFree(libxl_driver->reservedVNCPorts); + virObjectUnref(libxl_driver->reservedVNCPorts); VIR_FREE(libxl_driver->configDir); VIR_FREE(libxl_driver->autostartDir); @@ -979,9 +979,10 @@ libxlStartup(bool privileged, libxlDriverLock(libxl_driver); /* Allocate bitmap for vnc port reservation */ - if ((libxl_driver->reservedVNCPorts = - virBitmapNew(LIBXL_VNC_PORT_MAX - LIBXL_VNC_PORT_MIN)) == NULL) - goto out_of_memory; + if (!(libxl_driver->reservedVNCPorts = + virPortAllocatorNew(LIBXL_VNC_PORT_MIN, + LIBXL_VNC_PORT_MAX))) + goto error; if (virDomainObjListInit(&libxl_driver->domains) < 0) goto out_of_memory; -- 1.8.0.1
On 01/11/2013 05:13 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Replace the current libxl driver code for managing port reservations with the new virPortAllocator APIs. --- src/libxl/libxl_conf.c | 62 ++++-------------------------------------------- src/libxl/libxl_conf.h | 4 ++-- src/libxl/libxl_driver.c | 13 +++++----- 3 files changed, 14 insertions(+), 65 deletions(-)
ACK, and nice that the two drivers now share a common resource (host port allocation) without stomping on one another. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Daniel P. Berrange -
Eric Blake