]> git.ipfire.org Git - thirdparty/open-vm-tools.git/commitdiff
Minor changes to the Tools plugin API.
authorVMware, Inc <>
Tue, 19 Oct 2010 19:19:09 +0000 (12:19 -0700)
committerMarcelo Vanzin <mvanzin@vmware.com>
Tue, 19 Oct 2010 19:19:09 +0000 (12:19 -0700)
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 <mvanzin@vmware.com>
open-vm-tools/lib/include/vmware/tools/plugin.h
open-vm-tools/lib/include/vmware/tools/rpcdebug.h
open-vm-tools/lib/include/vmware/tools/utils.h
open-vm-tools/services/plugins/vmbackup/stateMachine.c
open-vm-tools/services/vmtoolsd/mainLoop.c
open-vm-tools/services/vmtoolsd/pluginMgr.c
open-vm-tools/tests/testDebug/testDebug.c
open-vm-tools/tests/testPlugin/testPlugin.c
open-vm-tools/tests/vmrpcdbg/debugChannel.c
open-vm-tools/tests/vmrpcdbg/vmrpcdbg.c

index d75d73d7028f1b09c2246499aa840fdcb1c42ce4..9f8ea95cdd19ad700c301d0270799d1b303bc163 100644 (file)
    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.
     */
index 04f2c9b4409e88625c17960f9214082f45d5cc83..af2d1ac1b82ff7579a9b00914e8b9e5fa657f76f 100644 (file)
@@ -121,6 +121,8 @@ typedef struct RpcDebugPlugin {
    RpcDebugSendFn       sendFn;
    /** Shutdown function. */
    RpcDebugShutdownFn   shutdownFn;
+   /** Plugin data that debug plugins can also export. */
+   ToolsPluginData     *plugin;
 } RpcDebugPlugin;
 
 
index a64899e6d76ff52c2657f30d7415786b41691f22..a0221bbc35c2bdfa3013b13a4a848c51eebc1e40 100644 (file)
 #  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
index a60ef118076e6c5e86f73b24e401c9cacbc0d915..8e8410b5a9e3e69ed397e283411660b2eb9ba885 100644 (file)
@@ -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));
    }
 }
 
index 8daebaaa787beed5eaca4d0d3a11136b1338846c..5f8937d85854e18f4c0d4262357e0cf402a99c53 100644 (file)
@@ -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);
       }
index 68c68ca8edbcb56f12a03c75af726d9cc9438094..7293290105b70065aab61dcc6bcd98cc0ffb5170 100644 (file)
@@ -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);
    }
 
index 1bcaba92fb6e70d1a46a33d424bd2c412244f873..0d4a77a2f343aecd1c54c5456ba5996abad0ef85 100644 (file)
@@ -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;
index 3e774f2207641cacc65314f52653c0ba5ba34687..8b42cc0ad13f0695ce37368555d74668144e4b75 100644 (file)
@@ -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 &regData;
 }
 
index b8e0e008a16fef320f8ae48031a168529c1a06dc..3f8727f6b89edaa00cef647e025a155ac2f1fe78 100644 (file)
@@ -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;
index 8504fb70c63bbeef3bf6331ac6bfec98e039c573..13195f5ef1c117b78aef77b8af3ce5854844f63a 100644 (file)
@@ -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) {