event: clean up client side RPC code

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 deregister an
unregistered object through public API, 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@redhat.com>
This commit is contained in:
Eric Blake 2014-01-08 10:10:49 -07:00
parent 41d6e49dc3
commit 6d8233fea2
2 changed files with 55 additions and 58 deletions

View File

@ -210,8 +210,9 @@ virObjectEventCallbackListRemoveID(virConnectPtr conn,
} }
} }
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", virReportError(VIR_ERR_INVALID_ARG,
_("could not find event callback for removal")); _("could not find event callback %d for deletion"),
callbackID);
return -1; return -1;
} }
@ -233,8 +234,9 @@ virObjectEventCallbackListMarkDeleteID(virConnectPtr conn,
} }
} }
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", virReportError(VIR_ERR_INVALID_ARG,
_("could not find event callback for deletion")); _("could not find event callback %d for deletion"),
callbackID);
return -1; return -1;
} }
@ -357,7 +359,7 @@ virObjectEventCallbackListAddID(virConnectPtr conn,
if (virObjectEventCallbackLookup(conn, cbList, uuid, if (virObjectEventCallbackLookup(conn, cbList, uuid,
klass, eventID, callback, klass, eventID, callback,
!callbackID) != -1) { !callbackID) != -1) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", virReportError(VIR_ERR_INVALID_ARG, "%s",
_("event callback already tracked")); _("event callback already tracked"));
return -1; 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: * virObjectEventQueueClear:
* @queue: pointer to the queue * @queue: pointer to the queue
@ -897,7 +878,7 @@ virObjectEventStateCallbackID(virConnectPtr conn,
virObjectEventStateUnlock(state); virObjectEventStateUnlock(state);
if (ret < 0) if (ret < 0)
virReportError(VIR_ERR_INTERNAL_ERROR, virReportError(VIR_ERR_INVALID_ARG,
_("event callback function %p not registered"), _("event callback function %p not registered"),
callback); callback);
return ret; return ret;
@ -920,11 +901,27 @@ virObjectEventStateEventID(virConnectPtr conn,
virObjectEventStatePtr state, virObjectEventStatePtr state,
int callbackID) int callbackID)
{ {
int ret; int ret = -1;
size_t i;
virObjectEventCallbackListPtr cbList = state->callbacks;
virObjectEventStateLock(state); virObjectEventStateLock(state);
ret = virObjectEventCallbackListEventID(conn, for (i = 0; i < cbList->count; i++) {
state->callbacks, callbackID); virObjectEventCallbackPtr cb = cbList->callbacks[i];
if (cb->deleted)
continue;
if (cb->callbackID == callbackID && cb->conn == conn) {
ret = cb->eventID;
break;
}
}
virObjectEventStateUnlock(state); virObjectEventStateUnlock(state);
if (ret < 0)
virReportError(VIR_ERR_INVALID_ARG,
_("event callback id %d not registered"),
callbackID);
return ret; return ret;
} }

View File

@ -2911,6 +2911,7 @@ done:
return rv; return rv;
} }
static int static int
remoteConnectNetworkEventRegisterAny(virConnectPtr conn, remoteConnectNetworkEventRegisterAny(virConnectPtr conn,
virNetworkPtr net, virNetworkPtr net,
@ -2968,16 +2969,12 @@ remoteConnectNetworkEventDeregisterAny(virConnectPtr conn,
remoteDriverLock(priv); remoteDriverLock(priv);
if ((eventID = virObjectEventStateEventID(conn, priv->eventState, if ((eventID = virObjectEventStateEventID(conn, priv->eventState,
callbackID)) < 0) { callbackID)) < 0)
virReportError(VIR_ERR_RPC, _("unable to find callback ID %d"), callbackID);
goto done; goto done;
}
if ((count = virObjectEventStateDeregisterID(conn, priv->eventState, if ((count = virObjectEventStateDeregisterID(conn, priv->eventState,
callbackID)) < 0) { callbackID)) < 0)
virReportError(VIR_ERR_RPC, _("unable to find callback ID %d"), callbackID);
goto done; goto done;
}
/* If that was the last callback for this eventID, we need to disable /* If that was the last callback for this eventID, we need to disable
* events on the server */ * events on the server */
@ -2997,6 +2994,7 @@ done:
return rv; return rv;
} }
static int static int
remoteConnectListAllInterfaces(virConnectPtr conn, remoteConnectListAllInterfaces(virConnectPtr conn,
virInterfacePtr **ifaces, virInterfacePtr **ifaces,
@ -4405,7 +4403,8 @@ out:
#endif /* WITH_POLKIT */ #endif /* WITH_POLKIT */
/*----------------------------------------------------------------------*/ /*----------------------------------------------------------------------*/
static int remoteConnectDomainEventRegister(virConnectPtr conn, static int
remoteConnectDomainEventRegister(virConnectPtr conn,
virConnectDomainEventCallback callback, virConnectDomainEventCallback callback,
void *opaque, void *opaque,
virFreeCallback freecb) virFreeCallback freecb)
@ -4421,12 +4420,14 @@ static int remoteConnectDomainEventRegister(virConnectPtr conn,
goto done; goto done;
if (count == 1) { 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, if (call(conn, priv, 0, REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER,
(xdrproc_t) xdr_void, (char *) NULL, (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; goto done;
} }
}
rv = 0; rv = 0;
@ -4435,7 +4436,8 @@ done:
return rv; return rv;
} }
static int remoteConnectDomainEventDeregister(virConnectPtr conn, static int
remoteConnectDomainEventDeregister(virConnectPtr conn,
virConnectDomainEventCallback callback) virConnectDomainEventCallback callback)
{ {
struct private_data *priv = conn->privateData; struct private_data *priv = conn->privateData;
@ -5205,7 +5207,8 @@ static virStreamDriver remoteStreamDrv = {
}; };
static int remoteConnectDomainEventRegisterAny(virConnectPtr conn, static int
remoteConnectDomainEventRegisterAny(virConnectPtr conn,
virDomainPtr dom, virDomainPtr dom,
int eventID, int eventID,
virConnectDomainEventGenericCallback callback, virConnectDomainEventGenericCallback callback,
@ -5248,7 +5251,8 @@ done:
} }
static int remoteConnectDomainEventDeregisterAny(virConnectPtr conn, static int
remoteConnectDomainEventDeregisterAny(virConnectPtr conn,
int callbackID) int callbackID)
{ {
struct private_data *priv = conn->privateData; struct private_data *priv = conn->privateData;
@ -5260,16 +5264,12 @@ static int remoteConnectDomainEventDeregisterAny(virConnectPtr conn,
remoteDriverLock(priv); remoteDriverLock(priv);
if ((eventID = virObjectEventStateEventID(conn, priv->eventState, if ((eventID = virObjectEventStateEventID(conn, priv->eventState,
callbackID)) < 0) { callbackID)) < 0)
virReportError(VIR_ERR_RPC, _("unable to find callback ID %d"), callbackID);
goto done; goto done;
}
if ((count = virObjectEventStateDeregisterID(conn, priv->eventState, if ((count = virObjectEventStateDeregisterID(conn, priv->eventState,
callbackID)) < 0) { callbackID)) < 0)
virReportError(VIR_ERR_RPC, _("unable to find callback ID %d"), callbackID);
goto done; goto done;
}
/* If that was the last callback for this eventID, we need to disable /* If that was the last callback for this eventID, we need to disable
* events on the server */ * events on the server */