From: VMware, Inc <> Date: Wed, 24 Feb 2010 21:49:48 +0000 (-0800) Subject: Allow outbound-only RpcChannels, cleanup API. X-Git-Tag: 2010.02.23-236320~47 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ec13f4f13d470e95fa083a4922acd5996cb62bc4;p=thirdparty%2Fopen-vm-tools.git Allow outbound-only RpcChannels, cleanup API. . allow a channel to be set up for outbound operations only. This defers creation of an RpcIn channel to until its needed, and doesn't enforce that data for RpcIn operations is available when starting the channel. This allows clients who only want to send outbound messages to more easily use the channel. Before, because of the code enforcing the call to the setup() function, these apps could run into issues like failing to properly start the channel because of some conflict caused by the RpcIn channel name (among other possible issues). . make the private data in the RpcChannel structure private to the lib/rpcChannel code. This only exposes the bare minimum of the structure in the header file to allow for new implementations of RpcChannel. Signed-off-by: Marcelo Vanzin --- diff --git a/open-vm-tools/lib/include/vmware/tools/guestrpc.h b/open-vm-tools/lib/include/vmware/tools/guestrpc.h index eae0a41ef..0b98c7600 100644 --- a/open-vm-tools/lib/include/vmware/tools/guestrpc.h +++ b/open-vm-tools/lib/include/vmware/tools/guestrpc.h @@ -116,6 +116,10 @@ typedef gboolean (*RpcChannelSendFn)(struct RpcChannel *, size_t dataLen, char **result, size_t *resultLen); +typedef void (*RpcChannelSetupFn)(struct RpcChannel *chan, + GMainContext *mainCtx, + const char *appName, + gpointer appCtx); /** @@ -135,18 +139,8 @@ typedef struct RpcChannel { RpcChannelStartFn start; RpcChannelStopFn stop; RpcChannelSendFn send; - /* Private section: don't use the fields below directly. */ + RpcChannelSetupFn setup; RpcChannelShutdownFn shutdown; - gchar *appName; - GHashTable *rpcs; - GMainContext *mainCtx; - GSource *resetCheck; - gpointer appCtx; - RpcChannelCallback resetReg; - RpcChannelResetCb resetCb; - gpointer resetData; - gboolean rpcError; - guint rpcErrorCount; gpointer _private; } RpcChannel; @@ -217,6 +211,9 @@ RpcChannel_BuildXdrCommand(const char *cmd, char **result, size_t *resultLen); +RpcChannel * +RpcChannel_Create(void); + gboolean RpcChannel_Destroy(RpcChannel *chan); @@ -246,7 +243,7 @@ RpcChannel_UnregisterCallback(RpcChannel *chan, RpcChannel * -RpcChannel_NewBackdoorChannel(GMainContext *mainCtx); +BackdoorChannel_New(void); G_END_DECLS diff --git a/open-vm-tools/lib/rpcChannel/bdoorChannel.c b/open-vm-tools/lib/rpcChannel/bdoorChannel.c index 577720754..b9f3b154e 100644 --- a/open-vm-tools/lib/rpcChannel/bdoorChannel.c +++ b/open-vm-tools/lib/rpcChannel/bdoorChannel.c @@ -33,6 +33,7 @@ #define RPCIN_MAX_DELAY 10 typedef struct BackdoorChannel { + GMainContext *mainCtx; GStaticMutex outLock; RpcIn *in; RpcOut *out; @@ -41,6 +42,28 @@ typedef struct BackdoorChannel { } BackdoorChannel; +/** + * Initializes internal state for the inbound channel. + * + * @param[in] chan The RPC channel instance. + * @param[in] ctx Main application context. + * @param[in] appName Unused. + * @param[in] appCtx Unused. + */ + +static void +RpcInSetup(RpcChannel *chan, + GMainContext *ctx, + const char *appName, + gpointer appCtx) +{ + BackdoorChannel *bdoor = chan->_private; + bdoor->mainCtx = g_main_context_ref(ctx); + bdoor->in = RpcIn_Construct(ctx, RpcChannel_Dispatch, chan); + ASSERT(bdoor->in != NULL); +} + + /** * Starts the RpcIn loop and the RpcOut channel. * @@ -52,21 +75,22 @@ typedef struct BackdoorChannel { static gboolean RpcInStart(RpcChannel *chan) { - gboolean ret; + gboolean ret = TRUE; BackdoorChannel *bdoor = chan->_private; - ASSERT(chan->appName != NULL); - ASSERT(!bdoor->inStarted); + ASSERT(bdoor->in == NULL || !bdoor->inStarted); ASSERT(!bdoor->outStarted); - ret = RpcIn_start(bdoor->in, RPCIN_MAX_DELAY, RpcChannel_Error, chan); + if (bdoor->in != NULL) { + ret = RpcIn_start(bdoor->in, RPCIN_MAX_DELAY, RpcChannel_Error, chan); + } if (ret) { ret = RpcOut_start(bdoor->out); if (!ret) { RpcIn_stop(bdoor->in); } } - bdoor->inStarted = TRUE; + bdoor->inStarted = (bdoor->in != NULL); bdoor->outStarted = TRUE; return ret; } @@ -88,7 +112,6 @@ RpcInStop(RpcChannel *chan) { BackdoorChannel *bdoor = chan->_private; - ASSERT(chan->appName != NULL); g_static_mutex_lock(&bdoor->outLock); if (bdoor->out != NULL) { if (bdoor->outStarted) { @@ -124,9 +147,14 @@ RpcInShutdown(RpcChannel *chan) { BackdoorChannel *bdoor = chan->_private; RpcInStop(chan); - RpcIn_Destruct(bdoor->in); + if (bdoor->in != NULL) { + RpcIn_Destruct(bdoor->in); + } RpcOut_Destruct(bdoor->out); g_static_mutex_free(&bdoor->outLock); + if (bdoor->mainCtx != NULL) { + g_main_context_unref(bdoor->mainCtx); + } g_free(bdoor); } @@ -155,14 +183,11 @@ RpcInSend(RpcChannel *chan, size_t replyLen; BackdoorChannel *bdoor = chan->_private; - ASSERT(chan->appName != NULL); - g_static_mutex_lock(&bdoor->outLock); if (!bdoor->outStarted) { goto exit; } - ret = RpcOut_send(bdoor->out, data, dataLen, &reply, &replyLen); /* @@ -222,24 +247,19 @@ exit: /** - * Creates a new RpcIn channel. - * - * @param[in] mainCtx The app's main context. + * Creates a new RpcChannel channel that uses the backdoor for communication. * * @return A new channel instance (never NULL). */ RpcChannel * -RpcChannel_NewBackdoorChannel(GMainContext *mainCtx) +BackdoorChannel_New(void) { RpcChannel *ret; BackdoorChannel *bdoor; - ret = g_malloc0(sizeof *ret); - - bdoor = g_malloc(sizeof *bdoor); - bdoor->in = RpcIn_Construct(mainCtx, RpcChannel_Dispatch, ret); - ASSERT(bdoor->in != NULL); + ret = RpcChannel_Create(); + bdoor = g_malloc0(sizeof *bdoor); g_static_mutex_init(&bdoor->outLock); bdoor->out = RpcOut_Construct(); @@ -251,6 +271,7 @@ RpcChannel_NewBackdoorChannel(GMainContext *mainCtx) ret->start = RpcInStart; ret->stop = RpcInStop; ret->send = RpcInSend; + ret->setup = RpcInSetup; ret->shutdown = RpcInShutdown; ret->_private = bdoor; diff --git a/open-vm-tools/lib/rpcChannel/rpcChannel.c b/open-vm-tools/lib/rpcChannel/rpcChannel.c index 3cc0c4d5a..1f1c6c779 100644 --- a/open-vm-tools/lib/rpcChannel/rpcChannel.c +++ b/open-vm-tools/lib/rpcChannel/rpcChannel.c @@ -31,6 +31,22 @@ #include "vmxrpc.h" #include "xdrutil.h" +/** Internal state of a channel. */ +typedef struct RpcChannelInt { + RpcChannel impl; + gchar *appName; + GHashTable *rpcs; + GMainContext *mainCtx; + GSource *resetCheck; + gpointer appCtx; + RpcChannelCallback resetReg; + RpcChannelResetCb resetCb; + gpointer resetData; + gboolean rpcError; + guint rpcErrorCount; +} RpcChannelInt; + + /** Max number of times to attempt a channel restart. */ #define RPCIN_MAX_RESTARTS 60 @@ -68,13 +84,13 @@ RpcChannelPing(RpcInData *data) static gboolean RpcChannelRestart(gpointer _chan) { - RpcChannel *chan = _chan; + RpcChannelInt *chan = _chan; - chan->stop(chan); - if (!chan->start(chan)) { + RpcChannel_Stop(&chan->impl); + if (!RpcChannel_Start(&chan->impl)) { g_warning("Channel restart failed [%d]\n", chan->rpcErrorCount); if (chan->resetCb != NULL) { - chan->resetCb(chan, FALSE, chan->resetData); + chan->resetCb(&chan->impl, FALSE, chan->resetData); } } else { chan->rpcError = FALSE; @@ -97,7 +113,7 @@ static gboolean RpcChannelCheckReset(gpointer _chan) { static int channelTimeoutAttempts = RPCIN_MAX_RESTARTS; - RpcChannel *chan = _chan; + RpcChannelInt *chan = _chan; /* Check the channel state. */ if (chan->rpcError) { @@ -107,7 +123,7 @@ RpcChannelCheckReset(gpointer _chan) g_warning("Failed to reset channel after %u attempts\n", chan->rpcErrorCount - 1); if (chan->resetCb != NULL) { - chan->resetCb(chan, FALSE, chan->resetData); + chan->resetCb(&chan->impl, FALSE, chan->resetData); } goto exit; } @@ -126,7 +142,7 @@ RpcChannelCheckReset(gpointer _chan) chan->rpcErrorCount = 0; if (chan->resetCb != NULL) { - chan->resetCb(chan, TRUE, chan->resetData); + chan->resetCb(&chan->impl, TRUE, chan->resetData); } exit: @@ -148,7 +164,7 @@ static gboolean RpcChannelReset(RpcInData *data) { gchar *msg; - RpcChannel *chan = data->clientData; + RpcChannelInt *chan = data->clientData; if (chan->resetCheck == NULL) { chan->resetCheck = g_idle_source_new(); @@ -300,6 +316,26 @@ exit: } +/** + * Creates a new RpcChannel without any implementation. + * + * This is mainly for use of code that is implementing a custom RpcChannel. + * Such implementations should provide their own "constructor"-type function + * which should then call this function to get an RpcChannel instance. They + * should then fill in the function pointers that provide the implementation + * for the channel before making the channel available to the callers. + * + * @return A new RpcChannel instance. + */ + +RpcChannel * +RpcChannel_Create(void) +{ + RpcChannelInt *chan = g_new0(RpcChannelInt, 1); + return &chan->impl; +} + + /** * Dispatches the given RPC to the registered handler. This mimics the behavior * of the RpcIn library (but is not tied to that particular implementation of @@ -318,7 +354,7 @@ RpcChannel_Dispatch(RpcInData *data) size_t nameLen; Bool status; RpcChannelCallback *rpc = NULL; - RpcChannel *chan = data->clientData; + RpcChannelInt *chan = data->clientData; name = StrUtil_GetNextToken(&index, data->args, " "); if (name == NULL) { @@ -370,37 +406,40 @@ gboolean RpcChannel_Destroy(RpcChannel *chan) { size_t i; + RpcChannelInt *cdata = (RpcChannelInt *) chan; - if (chan->shutdown != NULL) { - chan->shutdown(chan); + if (cdata->impl.shutdown != NULL) { + cdata->impl.shutdown(chan); } - RpcChannel_UnregisterCallback(chan, &chan->resetReg); + RpcChannel_UnregisterCallback(chan, &cdata->resetReg); for (i = 0; i < ARRAYSIZE(gRpcHandlers); i++) { RpcChannel_UnregisterCallback(chan, &gRpcHandlers[i]); } - if (chan->rpcs != NULL) { - g_hash_table_destroy(chan->rpcs); - chan->rpcs = NULL; + if (cdata->rpcs != NULL) { + g_hash_table_destroy(cdata->rpcs); + cdata->rpcs = NULL; } - chan->resetCb = NULL; - chan->resetData = NULL; - chan->appCtx = NULL; + cdata->resetCb = NULL; + cdata->resetData = NULL; + cdata->appCtx = NULL; - g_free(chan->appName); - chan->appName = NULL; + g_free(cdata->appName); + cdata->appName = NULL; - g_main_context_unref(chan->mainCtx); - chan->mainCtx = NULL; + if (cdata->mainCtx != NULL) { + g_main_context_unref(cdata->mainCtx); + cdata->mainCtx = NULL; + } - if (chan->resetCheck != NULL) { - g_source_destroy(chan->resetCheck); - chan->resetCheck = NULL; + if (cdata->resetCheck != NULL) { + g_source_destroy(cdata->resetCheck); + cdata->resetCheck = NULL; } - g_free(chan); + g_free(cdata); return TRUE; } @@ -417,7 +456,7 @@ void RpcChannel_Error(void *_chan, char const *status) { - RpcChannel *chan = _chan; + RpcChannelInt *chan = _chan; chan->rpcError = TRUE; g_warning("Error in the RPC receive loop: %s.\n", status); @@ -430,8 +469,11 @@ RpcChannel_Error(void *_chan, /** - * Initializes the RPC channel for use. This function must be called before - * starting the channel. + * Initializes the RPC channel for inbound operations. + * + * This function must be called before starting the channel if the application + * wants to receive messages on the channel. Applications don't need to call it + * if only using the outbound functionality. * * @param[in] chan The RPC channel. * @param[in] appName TCLO application name. @@ -450,25 +492,29 @@ RpcChannel_Setup(RpcChannel *chan, gpointer resetData) { size_t i; + RpcChannelInt *cdata = (RpcChannelInt *) chan; - chan->appName = g_strdup(appName); - chan->appCtx = appCtx; - chan->mainCtx = g_main_context_ref(mainCtx); - chan->resetCb = resetCb; - chan->resetData = resetData; + cdata->appName = g_strdup(appName); + cdata->appCtx = appCtx; + cdata->mainCtx = g_main_context_ref(mainCtx); + cdata->resetCb = resetCb; + cdata->resetData = resetData; - chan->resetReg.name = "reset"; - chan->resetReg.callback = RpcChannelReset; - chan->resetReg.clientData = chan; + cdata->resetReg.name = "reset"; + cdata->resetReg.callback = RpcChannelReset; + cdata->resetReg.clientData = chan; /* Register the callbacks handled by the rpcChannel library. */ - RpcChannel_RegisterCallback(chan, &chan->resetReg); + RpcChannel_RegisterCallback(chan, &cdata->resetReg); for (i = 0; i < ARRAYSIZE(gRpcHandlers); i++) { RpcChannel_RegisterCallback(chan, &gRpcHandlers[i]); } -} + if (cdata->impl.setup != NULL) { + cdata->impl.setup(&cdata->impl, mainCtx, appName, appCtx); + } +} /** @@ -508,16 +554,17 @@ void RpcChannel_RegisterCallback(RpcChannel *chan, RpcChannelCallback *rpc) { + RpcChannelInt *cdata = (RpcChannelInt *) chan; ASSERT(rpc->name != NULL && strlen(rpc->name) > 0); ASSERT(rpc->callback); ASSERT(rpc->xdrIn == NULL || rpc->xdrInSize > 0); - if (chan->rpcs == NULL) { - chan->rpcs = g_hash_table_new(g_str_hash, g_str_equal); + if (cdata->rpcs == NULL) { + cdata->rpcs = g_hash_table_new(g_str_hash, g_str_equal); } - if (g_hash_table_lookup(chan->rpcs, rpc->name) != NULL) { + if (g_hash_table_lookup(cdata->rpcs, rpc->name) != NULL) { g_error("Trying to overwrite existing RPC registration for %s!\n", rpc->name); } - g_hash_table_insert(chan->rpcs, (gpointer) rpc->name, rpc); + g_hash_table_insert(cdata->rpcs, (gpointer) rpc->name, rpc); } @@ -533,8 +580,9 @@ void RpcChannel_UnregisterCallback(RpcChannel *chan, RpcChannelCallback *rpc) { - if (chan->rpcs != NULL) { - g_hash_table_remove(chan->rpcs, rpc->name); + RpcChannelInt *cdata = (RpcChannelInt *) chan; + if (cdata->rpcs != NULL) { + g_hash_table_remove(cdata->rpcs, rpc->name); } } diff --git a/open-vm-tools/services/vmtoolsd/toolsRpc.c b/open-vm-tools/services/vmtoolsd/toolsRpc.c index c583d0b20..9cdca15fb 100644 --- a/open-vm-tools/services/vmtoolsd/toolsRpc.c +++ b/open-vm-tools/services/vmtoolsd/toolsRpc.c @@ -244,7 +244,7 @@ ToolsCore_InitRpc(ToolsServiceState *state) state->name); state->ctx.rpc = NULL; } else { - state->ctx.rpc = RpcChannel_NewBackdoorChannel(mainCtx); + state->ctx.rpc = BackdoorChannel_New(); } app = ToolsCore_GetTcloName(state); ASSERT(app != NULL); diff --git a/open-vm-tools/tests/vmrpcdbg/debugChannel.c b/open-vm-tools/tests/vmrpcdbg/debugChannel.c index a099736cc..3251dfe43 100644 --- a/open-vm-tools/tests/vmrpcdbg/debugChannel.c +++ b/open-vm-tools/tests/vmrpcdbg/debugChannel.c @@ -37,6 +37,7 @@ #include "vmware/tools/utils.h" typedef struct DbgChannelData { + ToolsAppCtx *ctx; gboolean hasLibRef; RpcDebugPlugin *plugin; GSource *msgTimer; @@ -68,7 +69,7 @@ RpcDebugDispatch(gpointer _chan) memset(&rpcdata, 0, sizeof rpcdata); if (!plugin->sendFn(&rpcdata)) { - RpcDebug_DecRef(chan->appCtx); + RpcDebug_DecRef(cdata->ctx); cdata->hasLibRef = FALSE; return FALSE; } else if (rpcdata.message == NULL) { @@ -80,7 +81,7 @@ RpcDebugDispatch(gpointer _chan) } data.clientData = chan; - data.appCtx = chan->appCtx; + data.appCtx = cdata->ctx; data.args = rpcdata.message; data.argsSize = rpcdata.messageLen; @@ -100,8 +101,8 @@ RpcDebugDispatch(gpointer _chan) } if (!ret) { - VMTOOLSAPP_ERROR((ToolsAppCtx *) chan->appCtx, 1); - RpcDebug_DecRef(chan->appCtx); + VMTOOLSAPP_ERROR(cdata->ctx, 1); + RpcDebug_DecRef(cdata->ctx); cdata->hasLibRef = FALSE; return FALSE; } @@ -124,11 +125,11 @@ RpcDebugStart(RpcChannel *chan) { DbgChannelData *data = chan->_private; - ASSERT(chan->appName != NULL); + ASSERT(data->ctx != NULL); ASSERT(data->msgTimer == NULL); data->msgTimer = g_timeout_source_new(100); - VMTOOLSAPP_ATTACH_SOURCE((ToolsAppCtx *)chan->appCtx, + VMTOOLSAPP_ATTACH_SOURCE(data->ctx, data->msgTimer, RpcDebugDispatch, chan, @@ -179,12 +180,13 @@ RpcDebugSend(RpcChannel *chan, { char *copy; gpointer xdrdata = NULL; - RpcDebugPlugin *plugin = ((DbgChannelData *)chan->_private)->plugin; + DbgChannelData *cdata = chan->_private; + RpcDebugPlugin *plugin = cdata->plugin; RpcDebugRecvMapping *mapping = NULL; RpcDebugRecvFn recvFn = NULL; gboolean ret = TRUE; - ASSERT(chan->appName != NULL); + ASSERT(cdata->ctx != NULL); /* Be paranoid. Like the VMX, NULL-terminate the incoming data. */ copy = g_malloc(dataLen + 1); @@ -256,7 +258,27 @@ exit: /** - * Does nothing. + * Intiializes internal state for the inbound channel. + * + * @param[in] chan The RPC channel instance. + * @param[in] ctx Unused. + * @param[in] appName Unused. + * @param[in] appCtx A ToolsAppCtx instance. + */ + +static void +RpcDebugSetup(RpcChannel *chan, + GMainContext *ctx, + const char *appName, + gpointer appCtx) +{ + DbgChannelData *cdata = chan->_private; + cdata->ctx = appCtx; +} + + +/** + * Cleans up the internal channel state. * * @param[in] chan The RPC channel instance. */ @@ -265,9 +287,9 @@ static void RpcDebugShutdown(RpcChannel *chan) { DbgChannelData *cdata = chan->_private; - ASSERT(chan->appName != NULL); + ASSERT(cdata->ctx != NULL); if (cdata->hasLibRef) { - RpcDebug_DecRef(chan->appCtx); + RpcDebug_DecRef(cdata->ctx); } g_free(chan->_private); } @@ -293,11 +315,11 @@ RpcDebug_NewDebugChannel(ToolsAppCtx *ctx, RpcChannel *ret; ASSERT(data != NULL); - - ret = g_malloc0(sizeof *ret); + ret = RpcChannel_Create(); ret->start = RpcDebugStart; ret->stop = RpcDebugStop; ret->send = RpcDebugSend; + ret->setup = RpcDebugSetup; ret->shutdown = RpcDebugShutdown; cdata = g_malloc0(sizeof *cdata);