Re: [libvirt] [PATCH 5/6] Remove the event namespace concept
On 11.12.2013 16:28, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange"
>
> The event namespace concept is redundant since the same
> information is available by looking at the class hierarchy
> of the events.
>
> Signed-off-by: Daniel P. Berrange
> ---
> src/conf/domain_event.c | 18 --
> src/conf/domain_event.h | 2 ++
> src/conf/network_event.c| 22 --
> src/conf/network_event.h| 3 +++
> src/conf/object_event.c | 28 +++-
> src/conf/object_event.h | 11 +++
> src/conf/object_event_private.h | 2 ++
> 7 files changed, 57 insertions(+), 29 deletions(-)
>
> diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
> index 64035f7..03164dc 100644
> --- a/src/conf/domain_event.c
> +++ b/src/conf/domain_event.c
> @@ -437,6 +437,7 @@ virDomainEventCallbackListAdd(virConnectPtr conn,
>virFreeCallback freecb)
> {
> return virObjectEventCallbackListAddID(conn, cbList, NULL, NULL, 0,
> + virDomainEventClass,
> VIR_DOMAIN_EVENT_ID_LIFECYCLE,
>
> VIR_OBJECT_EVENT_CALLBACK(callback),
> opaque, freecb, NULL);
> @@ -1366,6 +1367,9 @@ virDomainEventStateRegister(virConnectPtr conn,
> {
> int ret = -1;
>
> +if (virDomainEventsInitialize() < 0)
> +return -1;
> +
> virObjectEventStateLock(state);
>
> if ((state->callbacks->count == 0) &&
> @@ -1420,14 +1424,17 @@ virDomainEventStateRegisterID(virConnectPtr conn,
>virFreeCallback freecb,
>int *callbackID)
> {
> +if (virDomainEventsInitialize() < 0)
> +return -1;
> +
> if (dom)
> return virObjectEventStateRegisterID(conn, state, dom->uuid,
> dom->name,
> - dom->id, eventID,
> + dom->id, virDomainEventClass,
> eventID,
> VIR_OBJECT_EVENT_CALLBACK(cb),
> opaque, freecb, callbackID);
> else
> return virObjectEventStateRegisterID(conn, state, NULL, NULL, 0,
> - eventID,
> + virDomainEventClass, eventID,
> VIR_OBJECT_EVENT_CALLBACK(cb),
> opaque, freecb, callbackID);
> }
> @@ -1468,3 +1475,10 @@ virDomainEventStateDeregister(virConnectPtr conn,
> virObjectEventStateUnlock(state);
> return ret;
> }
> +
> +
> +virClassPtr
> +virDomainEventGetBaseClass(void)
> +{
> +return virDomainEventClass;
> +}
> diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h
> index 30156ed..5433950 100644
> --- a/src/conf/domain_event.h
> +++ b/src/conf/domain_event.h
> @@ -30,6 +30,8 @@
> # include "object_event.h"
> # include "domain_conf.h"
>
> +virClassPtr
> +virDomainEventGetBaseClass(void);
>
> virObjectEventPtr
> virDomainEventLifecycleNew(int id,
> diff --git a/src/conf/network_event.c b/src/conf/network_event.c
> index b1312b7..3108fea 100644
> --- a/src/conf/network_event.c
> +++ b/src/conf/network_event.c
> @@ -98,7 +98,7 @@ virNetworkEventDispatchDefaultFunc(virConnectPtr conn,
> if (!net)
> return;
>
> -switch ((virNetworkEventID) (event->eventID &0xFF)) {
> +switch ((virNetworkEventID)event->eventID) {
Honestly, I was never big fan of misusing eventID to carry the namespace
too.
ACK if you fix the crash Cederic pointed out.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/6] Remove the event namespace concept
On Wed, 2013-12-11 at 15:28 +, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange"
...
> @@ -633,22 +640,17 @@ virObjectEventStateDispatchFunc(virConnectPtr conn,
> void *opaque)
> {
> virObjectEventStatePtr state = opaque;
> -virEventNamespaceID namespace = (event->eventID & 0xFF00) >> 8;
>
> /* Drop the lock whle dispatching, for sake of re-entrancy */
> virObjectEventStateUnlock(state);
> -switch (namespace) {
> -case VIR_EVENT_NAMESPACE_DOMAIN:
> +if (virObjectIsClass(event, virDomainEventGetBaseClass()))
> virDomainEventDispatchDefaultFunc(conn, event,
> VIR_DOMAIN_EVENT_CALLBACK(cb), cbopaque, NULL);
> -break;
> -case VIR_EVENT_NAMESPACE_NETWORK:
> +else if (virObjectIsClass(event, virNetworkEventGetBaseClass()))
> virNetworkEventDispatchDefaultFunc(conn, event,
> VIR_NETWORK_EVENT_CALLBACK(cb), cbopaque, NULL);
> -break;
> -default:
> +else
> VIR_ERROR(_("Unknown event namespace to dispatch"));
> -}
> virObjectEventStateLock(state);
> }
In this function we may get a crash if we have network event callback
registered but nothing registered for domain events. Indeed, in such a
case the virDomainEventGetBaseClass() would return NULL and we'll have a
crash when checking the inheritance.
This is why I see a crash with the patched virt-manager, and not with
the event-test.
--
Cedric
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/6] Remove the event namespace concept
From: "Daniel P. Berrange"
The event namespace concept is redundant since the same
information is available by looking at the class hierarchy
of the events.
Signed-off-by: Daniel P. Berrange
---
src/conf/domain_event.c | 18 --
src/conf/domain_event.h | 2 ++
src/conf/network_event.c| 22 --
src/conf/network_event.h| 3 +++
src/conf/object_event.c | 28 +++-
src/conf/object_event.h | 11 +++
src/conf/object_event_private.h | 2 ++
7 files changed, 57 insertions(+), 29 deletions(-)
diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index 64035f7..03164dc 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -437,6 +437,7 @@ virDomainEventCallbackListAdd(virConnectPtr conn,
virFreeCallback freecb)
{
return virObjectEventCallbackListAddID(conn, cbList, NULL, NULL, 0,
+ virDomainEventClass,
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
VIR_OBJECT_EVENT_CALLBACK(callback),
opaque, freecb, NULL);
@@ -1366,6 +1367,9 @@ virDomainEventStateRegister(virConnectPtr conn,
{
int ret = -1;
+if (virDomainEventsInitialize() < 0)
+return -1;
+
virObjectEventStateLock(state);
if ((state->callbacks->count == 0) &&
@@ -1420,14 +1424,17 @@ virDomainEventStateRegisterID(virConnectPtr conn,
virFreeCallback freecb,
int *callbackID)
{
+if (virDomainEventsInitialize() < 0)
+return -1;
+
if (dom)
return virObjectEventStateRegisterID(conn, state, dom->uuid, dom->name,
- dom->id, eventID,
+ dom->id, virDomainEventClass,
eventID,
VIR_OBJECT_EVENT_CALLBACK(cb),
opaque, freecb, callbackID);
else
return virObjectEventStateRegisterID(conn, state, NULL, NULL, 0,
- eventID,
+ virDomainEventClass, eventID,
VIR_OBJECT_EVENT_CALLBACK(cb),
opaque, freecb, callbackID);
}
@@ -1468,3 +1475,10 @@ virDomainEventStateDeregister(virConnectPtr conn,
virObjectEventStateUnlock(state);
return ret;
}
+
+
+virClassPtr
+virDomainEventGetBaseClass(void)
+{
+return virDomainEventClass;
+}
diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h
index 30156ed..5433950 100644
--- a/src/conf/domain_event.h
+++ b/src/conf/domain_event.h
@@ -30,6 +30,8 @@
# include "object_event.h"
# include "domain_conf.h"
+virClassPtr
+virDomainEventGetBaseClass(void);
virObjectEventPtr
virDomainEventLifecycleNew(int id,
diff --git a/src/conf/network_event.c b/src/conf/network_event.c
index b1312b7..3108fea 100644
--- a/src/conf/network_event.c
+++ b/src/conf/network_event.c
@@ -98,7 +98,7 @@ virNetworkEventDispatchDefaultFunc(virConnectPtr conn,
if (!net)
return;
-switch ((virNetworkEventID) (event->eventID &0xFF)) {
+switch ((virNetworkEventID)event->eventID) {
case VIR_NETWORK_EVENT_ID_LIFECYCLE:
{
virNetworkEventLifecyclePtr networkLifecycleEvent;
@@ -147,14 +147,18 @@ virNetworkEventStateRegisterID(virConnectPtr conn,
virFreeCallback freecb,
int *callbackID)
{
-int nsEventID = (VIR_EVENT_NAMESPACE_NETWORK << 8) + eventID;
+if (virNetworkEventsInitialize() < 0)
+return -1;
+
if (net)
return virObjectEventStateRegisterID(conn, state,
- net->uuid, net->name, 0,
nsEventID,
+ net->uuid, net->name, 0,
+ virNetworkEventClass, eventID,
cb, opaque, freecb, callbackID);
else
return virObjectEventStateRegisterID(conn, state,
- NULL, NULL, 0, nsEventID,
+ NULL, NULL, 0,
+ virNetworkEventClass, eventID,
cb, opaque, freecb, callbackID);
}
@@ -165,13 +169,12 @@ virNetworkEventLifecycleNew(const char *name,
int detail)
{
virNetworkEventLifecyclePtr event;
-int eventId = (VIR_EVENT_NAMESPACE_NETWORK << 8) +
VIR_NETWORK_EVENT_ID_LIFECYCLE;
if (virNetworkEventsInitialize() < 0)
return NULL;
if (!(event = virObjectEventNew(virNetworkEventLifecycleClass,
-
