[libvirt] [PATCH] event: clean up client side RPC code
Eric Blake
eblake at redhat.com
Wed Jan 8 17:23:16 UTC 2014
Commit cfd62c1 was incomplete; I found more cases where error
messages were being overwritten, and where the code between
the three registration/deregistration APIs was not consistent.
Since it is fairly easy to trigger an attempt to register an
unregistered object, I also changed the error message from
VIR_ERR_INTERNAL_ERROR to VIR_ERR_INVALID_ARG.
* src/conf/object_event.c (virObjectEventCallbackListEventID):
Inline...
(virObjectEventStateEventID): ...into lone caller, and report
error on failure.
(virObjectEventCallbackListAddID, virObjectEventStateCallbackID)
(virObjectEventCallbackListRemoveID)
(virObjectEventCallbackListMarkDeleteID): Tweak error category.
* src/remote/remote_driver.c (remoteConnectDomainEventRegister):
Don't leak registration on failure.
(remoteConnectDomainEventDeregisterAny)
(remoteConnectNetworkEventDeregisterAny): Don't overwrite error.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
src/conf/object_event.c | 57 ++++++++++++++++++++++------------------------
src/remote/remote_driver.c | 56 ++++++++++++++++++++++-----------------------
2 files changed, 55 insertions(+), 58 deletions(-)
diff --git a/src/conf/object_event.c b/src/conf/object_event.c
index b01ffe5..b69387a 100644
--- a/src/conf/object_event.c
+++ b/src/conf/object_event.c
@@ -210,8 +210,9 @@ virObjectEventCallbackListRemoveID(virConnectPtr conn,
}
}
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("could not find event callback for removal"));
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("could not find event callback %d for deletion"),
+ callbackID);
return -1;
}
@@ -233,8 +234,9 @@ virObjectEventCallbackListMarkDeleteID(virConnectPtr conn,
}
}
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("could not find event callback for deletion"));
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("could not find event callback %d for deletion"),
+ callbackID);
return -1;
}
@@ -357,7 +359,7 @@ virObjectEventCallbackListAddID(virConnectPtr conn,
if (virObjectEventCallbackLookup(conn, cbList, uuid,
klass, eventID, callback,
!callbackID) != -1) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
_("event callback already tracked"));
return -1;
}
@@ -398,27 +400,6 @@ cleanup:
}
-static int
-virObjectEventCallbackListEventID(virConnectPtr conn,
- virObjectEventCallbackListPtr cbList,
- int callbackID)
-{
- size_t i;
-
- for (i = 0; i < cbList->count; i++) {
- virObjectEventCallbackPtr cb = cbList->callbacks[i];
-
- if (cb->deleted)
- continue;
-
- if (cb->callbackID == callbackID && cb->conn == conn)
- return cb->eventID;
- }
-
- return -1;
-}
-
-
/**
* virObjectEventQueueClear:
* @queue: pointer to the queue
@@ -897,7 +878,7 @@ virObjectEventStateCallbackID(virConnectPtr conn,
virObjectEventStateUnlock(state);
if (ret < 0)
- virReportError(VIR_ERR_INTERNAL_ERROR,
+ virReportError(VIR_ERR_INVALID_ARG,
_("event callback function %p not registered"),
callback);
return ret;
@@ -920,11 +901,27 @@ virObjectEventStateEventID(virConnectPtr conn,
virObjectEventStatePtr state,
int callbackID)
{
- int ret;
+ int ret = -1;
+ size_t i;
+ virObjectEventCallbackListPtr cbList = state->callbacks;
virObjectEventStateLock(state);
- ret = virObjectEventCallbackListEventID(conn,
- state->callbacks, callbackID);
+ for (i = 0; i < cbList->count; i++) {
+ virObjectEventCallbackPtr cb = cbList->callbacks[i];
+
+ if (cb->deleted)
+ continue;
+
+ if (cb->callbackID == callbackID && cb->conn == conn) {
+ ret = cb->eventID;
+ break;
+ }
+ }
virObjectEventStateUnlock(state);
+
+ if (ret < 0)
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("event callback id %d not registered"),
+ callbackID);
return ret;
}
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index fecb9b2..64d9d92 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -2911,6 +2911,7 @@ done:
return rv;
}
+
static int
remoteConnectNetworkEventRegisterAny(virConnectPtr conn,
virNetworkPtr net,
@@ -2968,16 +2969,12 @@ remoteConnectNetworkEventDeregisterAny(virConnectPtr conn,
remoteDriverLock(priv);
if ((eventID = virObjectEventStateEventID(conn, priv->eventState,
- callbackID)) < 0) {
- virReportError(VIR_ERR_RPC, _("unable to find callback ID %d"), callbackID);
+ callbackID)) < 0)
goto done;
- }
if ((count = virObjectEventStateDeregisterID(conn, priv->eventState,
- callbackID)) < 0) {
- virReportError(VIR_ERR_RPC, _("unable to find callback ID %d"), callbackID);
+ callbackID)) < 0)
goto done;
- }
/* If that was the last callback for this eventID, we need to disable
* events on the server */
@@ -2997,6 +2994,7 @@ done:
return rv;
}
+
static int
remoteConnectListAllInterfaces(virConnectPtr conn,
virInterfacePtr **ifaces,
@@ -4405,10 +4403,11 @@ out:
#endif /* WITH_POLKIT */
/*----------------------------------------------------------------------*/
-static int remoteConnectDomainEventRegister(virConnectPtr conn,
- virConnectDomainEventCallback callback,
- void *opaque,
- virFreeCallback freecb)
+static int
+remoteConnectDomainEventRegister(virConnectPtr conn,
+ virConnectDomainEventCallback callback,
+ void *opaque,
+ virFreeCallback freecb)
{
int rv = -1;
struct private_data *priv = conn->privateData;
@@ -4421,11 +4420,13 @@ static int remoteConnectDomainEventRegister(virConnectPtr conn,
goto done;
if (count == 1) {
- /* Tell the server when we are the first callback deregistering */
+ /* Tell the server when we are the first callback registering */
if (call(conn, priv, 0, REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER,
(xdrproc_t) xdr_void, (char *) NULL,
- (xdrproc_t) xdr_void, (char *) NULL) == -1)
+ (xdrproc_t) xdr_void, (char *) NULL) == -1) {
+ virDomainEventStateDeregister(conn, priv->eventState, callback);
goto done;
+ }
}
rv = 0;
@@ -4435,8 +4436,9 @@ done:
return rv;
}
-static int remoteConnectDomainEventDeregister(virConnectPtr conn,
- virConnectDomainEventCallback callback)
+static int
+remoteConnectDomainEventDeregister(virConnectPtr conn,
+ virConnectDomainEventCallback callback)
{
struct private_data *priv = conn->privateData;
int rv = -1;
@@ -5205,12 +5207,13 @@ static virStreamDriver remoteStreamDrv = {
};
-static int remoteConnectDomainEventRegisterAny(virConnectPtr conn,
- virDomainPtr dom,
- int eventID,
- virConnectDomainEventGenericCallback callback,
- void *opaque,
- virFreeCallback freecb)
+static int
+remoteConnectDomainEventRegisterAny(virConnectPtr conn,
+ virDomainPtr dom,
+ int eventID,
+ virConnectDomainEventGenericCallback callback,
+ void *opaque,
+ virFreeCallback freecb)
{
int rv = -1;
struct private_data *priv = conn->privateData;
@@ -5248,8 +5251,9 @@ done:
}
-static int remoteConnectDomainEventDeregisterAny(virConnectPtr conn,
- int callbackID)
+static int
+remoteConnectDomainEventDeregisterAny(virConnectPtr conn,
+ int callbackID)
{
struct private_data *priv = conn->privateData;
int rv = -1;
@@ -5260,16 +5264,12 @@ static int remoteConnectDomainEventDeregisterAny(virConnectPtr conn,
remoteDriverLock(priv);
if ((eventID = virObjectEventStateEventID(conn, priv->eventState,
- callbackID)) < 0) {
- virReportError(VIR_ERR_RPC, _("unable to find callback ID %d"), callbackID);
+ callbackID)) < 0)
goto done;
- }
if ((count = virObjectEventStateDeregisterID(conn, priv->eventState,
- callbackID)) < 0) {
- virReportError(VIR_ERR_RPC, _("unable to find callback ID %d"), callbackID);
+ callbackID)) < 0)
goto done;
- }
/* If that was the last callback for this eventID, we need to disable
* events on the server */
--
1.8.4.2
More information about the libvir-list
mailing list