From: VMware, Inc <> Date: Tue, 19 Oct 2010 19:19:09 +0000 (-0700) Subject: Minor changes to the Tools plugin API. X-Git-Tag: 2010.10.18-313025~30 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8f8cf6acf57a4bb1de0aa939bd94f46f3dbbe1d0;p=thirdparty%2Fopen-vm-tools.git Minor changes to the Tools plugin API. While working on a VMCF provider plugin, I ran into some issues that are better solved by making some slight modifications to the API. Since the incompatible changes are not really used by any production code yet, and the API is not yet public, these should be OK without the need to change the current "API version". The changes are: . provide a pointer to the plugin that owns an app registration to the app provider's callback. This allows app providers to call the plugin's error callback directly at their own convenience. . add an inline function that plugins can use to print state information in a consistent manner. . add a convenience macro to make calls to VMTools_WrapArray look less cluttered. . stop app providers before sending the SHUTDOWN signal, so plugins can then safely free their internal state without causing issues with app providers that reference that data. Also only call a provider's shutdown handler when it was successfully activated. . allow debug plugins to also export a regular plugin interface. This should make it safer for these plugins to register for signals at the right times, and to also test more complicated applications. . allow debug plugins without a send function. Signed-off-by: Marcelo Vanzin --- diff --git a/open-vm-tools/lib/include/vmware/tools/plugin.h b/open-vm-tools/lib/include/vmware/tools/plugin.h index d75d73d70..9f8ea95cd 100644 --- a/open-vm-tools/lib/include/vmware/tools/plugin.h +++ b/open-vm-tools/lib/include/vmware/tools/plugin.h @@ -68,6 +68,36 @@ g_source_attach(__src, g_main_loop_get_context((ctx)->mainLoop)); \ } while (0) +/* Indentation leves for the state log function below. */ +#define TOOLS_STATE_LOG_ROOT 0 +#define TOOLS_STATE_LOG_CONTAINER 1 +#define TOOLS_STATE_LOG_PLUGIN 2 + +/** + * Convenience function for printing state logs. This function makes sure + * all code logging state information uses the same log domain, level and + * use the same indentation. + * + * @param[in] level Indentation level (see constants above). + * @param[in] fmt Message format. + * @param[in] ... Message arguments. + */ + +static inline void +ToolsCore_LogState(guint level, + const char *fmt, + ...) +{ + gchar *indented = g_strdup_printf("%*s%s", 3 * level, "", fmt); + + va_list args; + va_start(args, fmt); + g_logv("state", G_LOG_LEVEL_INFO, indented, args); + va_end(args); + + g_free(indented); +} + /** * Signal sent when registering or unregistering capabilities. @@ -300,6 +330,8 @@ typedef enum { } ToolsAppType; +struct ToolsPluginData; + /** * Defines the registration data for an "application provider". Application * providers allow plugins to hook into new application frameworks that will @@ -334,18 +366,24 @@ typedef struct ToolsAppProvider { * Registration callback. This is called after "activate", to register an * application provided by a plugin. * - * @param[in] ctx The application context. - * @param[in] prov The provider instance. - * @param[in] reg The application registration data. + * @param[in] ctx The application context. + * @param[in] prov The provider instance. + * @param[in] plugin The plugin that owns the registration. + * @param[in] reg The application registration data. * * @return Whether registration succeeded. */ - gboolean (*registerApp)(ToolsAppCtx *ctx, struct ToolsAppProvider *prov, gpointer reg); + gboolean (*registerApp)(ToolsAppCtx *ctx, + struct ToolsAppProvider *prov, + struct ToolsPluginData *plugin, + gpointer reg); /** * Shutdown callback (optional). Called when the service is being shut down. * The provider is responsible for keeping track of registrations and * cleaning them up during shutdown. * + * This method is only called if the provider was successfully activated. + * * @param[in] ctx The application context. * @param[in] prov The provider instance. */ diff --git a/open-vm-tools/lib/include/vmware/tools/rpcdebug.h b/open-vm-tools/lib/include/vmware/tools/rpcdebug.h index 04f2c9b44..af2d1ac1b 100644 --- a/open-vm-tools/lib/include/vmware/tools/rpcdebug.h +++ b/open-vm-tools/lib/include/vmware/tools/rpcdebug.h @@ -121,6 +121,8 @@ typedef struct RpcDebugPlugin { RpcDebugSendFn sendFn; /** Shutdown function. */ RpcDebugShutdownFn shutdownFn; + /** Plugin data that debug plugins can also export. */ + ToolsPluginData *plugin; } RpcDebugPlugin; diff --git a/open-vm-tools/lib/include/vmware/tools/utils.h b/open-vm-tools/lib/include/vmware/tools/utils.h index a64899e6d..a0221bbc3 100644 --- a/open-vm-tools/lib/include/vmware/tools/utils.h +++ b/open-vm-tools/lib/include/vmware/tools/utils.h @@ -89,6 +89,10 @@ # define VMTOOLS_RELEASE_FILENAME_LOCAL(path) g_free(path) #endif +/** Convenience macro around VMTools_WrapArray. */ +#define VMTOOLS_WRAP_ARRAY(a) VMTools_WrapArray((a), sizeof *(a), G_N_ELEMENTS(a)) + + G_BEGIN_DECLS void diff --git a/open-vm-tools/services/plugins/vmbackup/stateMachine.c b/open-vm-tools/services/plugins/vmbackup/stateMachine.c index a60ef1180..8e8410b5a 100644 --- a/open-vm-tools/services/plugins/vmbackup/stateMachine.c +++ b/open-vm-tools/services/plugins/vmbackup/stateMachine.c @@ -899,10 +899,11 @@ VmBackupDumpState(gpointer src, gpointer data) { if (gBackupState == NULL) { - g_message("Backup is idle.\n"); + ToolsCore_LogState(TOOLS_STATE_LOG_PLUGIN, "Backup is idle.\n"); } else { - g_message("Backup is in state: %s\n", - VmBackupGetStateName(gBackupState->machineState)); + ToolsCore_LogState(TOOLS_STATE_LOG_PLUGIN, + "Backup is in state: %s\n", + VmBackupGetStateName(gBackupState->machineState)); } } diff --git a/open-vm-tools/services/vmtoolsd/mainLoop.c b/open-vm-tools/services/vmtoolsd/mainLoop.c index 8daebaaa7..5f8937d85 100644 --- a/open-vm-tools/services/vmtoolsd/mainLoop.c +++ b/open-vm-tools/services/vmtoolsd/mainLoop.c @@ -254,16 +254,21 @@ ToolsCore_DumpState(ToolsServiceState *state) ASSERT_ON_COMPILE(ARRAYSIZE(providerStates) == TOOLS_PROVIDER_MAX); - g_message("VM Tools Service '%s':\n", state->name); - g_message(" Plugin path: %s\n", state->pluginPath); + ToolsCore_LogState(TOOLS_STATE_LOG_ROOT, + "VM Tools Service '%s':\n", + state->name); + ToolsCore_LogState(TOOLS_STATE_LOG_CONTAINER, + "Plugin path: %s\n", + state->pluginPath); for (i = 0; i < state->providers->len; i++) { ToolsAppProviderReg *prov = &g_array_index(state->providers, ToolsAppProviderReg, i); - g_message(" App provider: %s (%s)\n", - prov->prov->name, - providerStates[prov->state]); + ToolsCore_LogState(TOOLS_STATE_LOG_CONTAINER, + "App provider: %s (%s)\n", + prov->prov->name, + providerStates[prov->state]); if (prov->prov->dumpState != NULL) { prov->prov->dumpState(&state->ctx, prov->prov, NULL); } diff --git a/open-vm-tools/services/vmtoolsd/pluginMgr.c b/open-vm-tools/services/vmtoolsd/pluginMgr.c index 68c68ca8e..729329010 100644 --- a/open-vm-tools/services/vmtoolsd/pluginMgr.c +++ b/open-vm-tools/services/vmtoolsd/pluginMgr.c @@ -68,10 +68,14 @@ ToolsCoreDumpAppInfo(ToolsServiceState *state, if (preg->prov->dumpState != NULL) { preg->prov->dumpState(&state->ctx, preg->prov, reg); } else { - g_message(" App type %u (no provider info).\n", type); + ToolsCore_LogState(TOOLS_STATE_LOG_PLUGIN, + "App type %u (no provider info).\n", + type); } } else { - g_message(" App type %u (no provider).\n", type); + ToolsCore_LogState(TOOLS_STATE_LOG_PLUGIN, + "App type %u (no provider).\n", + type); } return TRUE; } @@ -88,10 +92,10 @@ static void ToolsCoreDumpPluginInfo(ToolsServiceState *state, ToolsPluginData *plugin) { - g_message(" Plugin: %s\n", plugin->name); + ToolsCore_LogState(TOOLS_STATE_LOG_CONTAINER, "Plugin: %s\n", plugin->name); if (plugin->regs == NULL) { - g_message(" No registrations.\n"); + ToolsCore_LogState(TOOLS_STATE_LOG_PLUGIN, "No registrations.\n"); } } @@ -111,7 +115,7 @@ ToolsCoreDumpRPC(ToolsAppCtx *ctx, { if (reg != NULL) { RpcChannelCallback *cb = reg; - g_message(" RPC callback: %s\n", cb->name); + ToolsCore_LogState(TOOLS_STATE_LOG_PLUGIN, "RPC callback: %s\n", cb->name); } } @@ -131,7 +135,7 @@ ToolsCoreDumpSignal(ToolsAppCtx *ctx, { if (reg != NULL) { ToolsPluginSignalCb *sig = reg; - g_message(" Signal callback: %s\n", sig->signame); + ToolsCore_LogState(TOOLS_STATE_LOG_PLUGIN, "Signal callback: %s\n", sig->signame); } } @@ -189,7 +193,7 @@ ToolsCoreRegisterApp(ToolsServiceState *state, preg->state = TOOLS_PROVIDER_ACTIVE; } - if (!preg->prov->registerApp(&state->ctx, preg->prov, reg)) { + if (!preg->prov->registerApp(&state->ctx, preg->prov, plugin, reg)) { g_warning("Failed registration of app type %d (%s) from plugin %s.", type, preg->prov->name, plugin->name); goto exit; @@ -323,9 +327,10 @@ ToolsCoreForEachPlugin(ToolsServiceState *state, /** * Registration callback for GuestRPC applications. * - * @param[in] ctx The application context. - * @param[in] prov Unused. - * @param[in] reg The application registration data. + * @param[in] ctx The application context. + * @param[in] prov Unused. + * @param[in] plugin Unused. + * @param[in] reg The application registration data. * * @return TRUE. */ @@ -333,6 +338,7 @@ ToolsCoreForEachPlugin(ToolsServiceState *state, static gboolean ToolsCoreRegisterRPC(ToolsAppCtx *ctx, ToolsAppProvider *prov, + ToolsPluginData *plugin, gpointer reg) { RpcChannel_RegisterCallback(ctx->rpc, reg); @@ -343,9 +349,10 @@ ToolsCoreRegisterRPC(ToolsAppCtx *ctx, /** * Registration callback for signal connections. * - * @param[in] ctx The application context. - * @param[in] prov Unused. - * @param[in] reg The application registration data. + * @param[in] ctx The application context. + * @param[in] prov Unused. + * @param[in] plugin Unused. + * @param[in] reg The application registration data. * * @return TRUE if the signal exists. */ @@ -353,6 +360,7 @@ ToolsCoreRegisterRPC(ToolsAppCtx *ctx, static gboolean ToolsCoreRegisterSignal(ToolsAppCtx *ctx, ToolsAppProvider *prov, + ToolsPluginData *plugin, gpointer reg) { ToolsPluginSignalCb *sig = reg; @@ -614,6 +622,19 @@ ToolsCore_LoadPlugins(ToolsServiceState *state) goto exit; } + /* + * If there is a debug plugin, see if it exports standard plugin registration + * data too. + */ + if (state->debugData != NULL && state->debugData->debugPlugin->plugin != NULL) { + ToolsPluginData *data = state->debugData->debugPlugin->plugin; + ToolsPlugin *plugin = g_malloc(sizeof *plugin); + plugin->module = NULL; + plugin->data = data; + VMTools_BindTextDomain(data->name, NULL, NULL); + g_ptr_array_add(state->plugins, plugin); + } + ret = TRUE; exit: @@ -730,8 +751,6 @@ ToolsCore_UnloadPlugins(ToolsServiceState *state) g_array_free(pcaps, TRUE); } - g_signal_emit_by_name(state->ctx.serviceObj, TOOLS_CORE_SIG_SHUTDOWN, &state->ctx); - /* * Stop all app providers, and free the memory we allocated for the two * internal app providers. @@ -741,7 +760,7 @@ ToolsCore_UnloadPlugins(ToolsServiceState *state) ToolsAppProviderReg, i); - if (preg->prov->shutdown != NULL) { + if (preg->prov->shutdown != NULL && preg->state == TOOLS_PROVIDER_ACTIVE) { preg->prov->shutdown(&state->ctx, preg->prov); } @@ -752,6 +771,8 @@ ToolsCore_UnloadPlugins(ToolsServiceState *state) } } + g_signal_emit_by_name(state->ctx.serviceObj, TOOLS_CORE_SIG_SHUTDOWN, &state->ctx); + while (state->plugins->len > 0) { ToolsPlugin *plugin = g_ptr_array_index(state->plugins, state->plugins->len - 1); GArray *regs = (plugin->data != NULL) ? plugin->data->regs : NULL; @@ -770,7 +791,9 @@ ToolsCore_UnloadPlugins(ToolsServiceState *state) } g_ptr_array_remove_index(state->plugins, state->plugins->len - 1); - g_module_close(plugin->module); + if (plugin->module != NULL) { + g_module_close(plugin->module); + } g_free(plugin); } diff --git a/open-vm-tools/tests/testDebug/testDebug.c b/open-vm-tools/tests/testDebug/testDebug.c index 1bcaba92f..0d4a77a2f 100644 --- a/open-vm-tools/tests/testDebug/testDebug.c +++ b/open-vm-tools/tests/testDebug/testDebug.c @@ -96,21 +96,8 @@ static gboolean TestDebugValidateReset(RpcInData *data, gboolean ret) { - ToolsAppCtx *ctx = data->appCtx; RPCDEBUG_ASSERT(data->result != NULL, FALSE); CU_ASSERT_STRING_EQUAL(data->result, "ATR debug"); - - /* - * If reset was successful, connect the "test-signal" signal so we - * test custom registration of signals. The test plugin will emit - * this signal after it sends an "test.rpcout.msg1" RPC as part of - * handling a "test.rpcin.msg1" RPC. - */ - g_signal_connect(ctx->serviceObj, - "test-signal", - G_CALLBACK(TestDebugHandleSignal), - NULL); - return (gboolean) ret; } @@ -267,25 +254,47 @@ RpcDebugOnLoad(ToolsAppCtx *ctx) xdr_TestPluginData, sizeof (TestPluginData) }, { NULL, NULL } }; + static ToolsPluginData pluginData = { + "testDebug", + NULL, + NULL, + NULL, + }; static RpcDebugPlugin regData = { recvFns, NULL, TestDebugSendNext, - NULL + NULL, + &pluginData, }; - TestPluginData testdata; - testdata.data = "rpc1test"; - testdata.f_int = 1357; - testdata.f_bool = TRUE; - - /* Build the command for the "test.rpcin.msg1" RPC. */ - if (!RpcChannel_BuildXdrCommand("test.rpcin.msg1", - xdr_TestPluginData, - &testdata, - &gRpcMessages[4].message, - &gRpcMessages[4].messageLen)) { - g_error("Failed to create test.rpcin.msg1 command.\n"); + /* Standard plugin interface, used to listen for signals. */ + { + ToolsPluginSignalCb sigs[] = { + { "test-signal", TestDebugHandleSignal, NULL }, + }; + ToolsAppReg regs[] = { + { TOOLS_APP_SIGNALS, VMTOOLS_WRAP_ARRAY(sigs) }, + }; + + pluginData.regs = VMTOOLS_WRAP_ARRAY(regs); + } + + /* Initialize the paylod of the "test.rpcin.msg1" RPC. */ + { + TestPluginData testdata; + testdata.data = "rpc1test"; + testdata.f_int = 1357; + testdata.f_bool = TRUE; + + /* Build the command for the "test.rpcin.msg1" RPC. */ + if (!RpcChannel_BuildXdrCommand("test.rpcin.msg1", + xdr_TestPluginData, + &testdata, + &gRpcMessages[4].message, + &gRpcMessages[4].messageLen)) { + g_error("Failed to create test.rpcin.msg1 command.\n"); + } } gCtx = ctx; diff --git a/open-vm-tools/tests/testPlugin/testPlugin.c b/open-vm-tools/tests/testPlugin/testPlugin.c index 3e774f220..8b42cc0ad 100644 --- a/open-vm-tools/tests/testPlugin/testPlugin.c +++ b/open-vm-tools/tests/testPlugin/testPlugin.c @@ -280,6 +280,7 @@ TestPluginSetOption(gpointer src, * * @param[in] ctx Unused. * @param[in] prov Unused. + * @param[in] plugin Unused. * @param[in] reg Registration data (should be a string). * * @retval FALSE if registration value is TEST_APP_ERROR. @@ -289,6 +290,7 @@ TestPluginSetOption(gpointer src, static gboolean TestProviderRegisterApp(ToolsAppCtx *ctx, ToolsAppProvider *prov, + ToolsPluginData *plugin, gpointer reg) { TestApp *app = reg; @@ -389,11 +391,11 @@ ToolsOnLoad(ToolsAppCtx *ctx) { "TestAppNoProvider" } }; ToolsAppReg regs[] = { - { TOOLS_APP_GUESTRPC, VMTools_WrapArray(rpcs, sizeof *rpcs, ARRAYSIZE(rpcs)) }, - { TOOLS_APP_PROVIDER, VMTools_WrapArray(provs, sizeof *provs, ARRAYSIZE(provs)) }, - { TOOLS_APP_SIGNALS, VMTools_WrapArray(sigs, sizeof *sigs, ARRAYSIZE(sigs)) }, - { 42, VMTools_WrapArray(tapp, sizeof *tapp, ARRAYSIZE(tapp)) }, - { 43, VMTools_WrapArray(tnoprov, sizeof *tnoprov, ARRAYSIZE(tnoprov)) }, + { TOOLS_APP_GUESTRPC, VMTOOLS_WRAP_ARRAY(rpcs) }, + { TOOLS_APP_PROVIDER, VMTOOLS_WRAP_ARRAY(provs) }, + { TOOLS_APP_SIGNALS, VMTOOLS_WRAP_ARRAY(sigs) }, + { 42, VMTOOLS_WRAP_ARRAY(tapp) }, + { 43, VMTOOLS_WRAP_ARRAY(tnoprov) }, }; g_signal_new("test-signal", @@ -406,7 +408,7 @@ ToolsOnLoad(ToolsAppCtx *ctx) G_TYPE_NONE, 0); - regData.regs = VMTools_WrapArray(regs, sizeof *regs, ARRAYSIZE(regs)); + regData.regs = VMTOOLS_WRAP_ARRAY(regs); return ®Data; } diff --git a/open-vm-tools/tests/vmrpcdbg/debugChannel.c b/open-vm-tools/tests/vmrpcdbg/debugChannel.c index b8e0e008a..3f8727f6b 100644 --- a/open-vm-tools/tests/vmrpcdbg/debugChannel.c +++ b/open-vm-tools/tests/vmrpcdbg/debugChannel.c @@ -68,7 +68,7 @@ RpcDebugDispatch(gpointer _chan) memset(&data, 0, sizeof data); memset(&rpcdata, 0, sizeof rpcdata); - if (!plugin->sendFn(&rpcdata)) { + if (plugin->sendFn == NULL || !plugin->sendFn(&rpcdata)) { RpcDebug_DecRef(cdata->ctx); cdata->hasLibRef = FALSE; return FALSE; diff --git a/open-vm-tools/tests/vmrpcdbg/vmrpcdbg.c b/open-vm-tools/tests/vmrpcdbg/vmrpcdbg.c index 8504fb70c..13195f5ef 100644 --- a/open-vm-tools/tests/vmrpcdbg/vmrpcdbg.c +++ b/open-vm-tools/tests/vmrpcdbg/vmrpcdbg.c @@ -116,7 +116,7 @@ RpcDebugRun(ToolsAppCtx *ctx, err = CU_basic_run_tests(); /* Clean up internal library / debug plugin state. */ - ASSERT(g_atomic_int_get(&gRefCount) == 0); + ASSERT(g_atomic_int_get(&gRefCount) >= 0); ASSERT(ldata != NULL); if (ldata->debugPlugin->shutdownFn != NULL) {