[ofa-general] Re: [PATCH RFC] opensm/event_plugin: plugin API version 2
Hal Rosenstock
hrosenstock at xsigo.com
Thu Jun 26 10:26:58 PDT 2008
On Thu, 2008-06-26 at 01:23 +0300, Sasha Khapyorsky wrote:
> Main difference is that construct() method is renamed to create() and
> gets reference to osm_opensm_t object instead of just osm_log_t. This
> will provide to event plugin access to all OpenSM data structures and
> should help to create more generic plugins.
This exposes almost all internal data structures! While that is
powerful, it also could be dangerous as plugins can now more readily
corrupt OpenSM. It's certainly the most flexible but leaves everything
totally open. Is this the best approach ?
If this goes ahead, versioning is an issue. Not sure osm_version is the
best barometer for everything. It may be for include files for building
a plugin but what about a coarser versioning (like library versioning) ?
It would be nice if a single plugin can handle as many OpenSM versions
as possible (by something simpler than a range of osm_versions) rather
than needing a separate plugin per OpenSM version at the other end of
the spectrum.
-- Hal
> For consistency with above
> destroy() method is renamed to delete(). Event plugin interface version
> is increased and osmeventplugin example updated accordingly.
> Signed-off-by: Sasha Khapyorsky <sashak at voltaire.com>
> ---
> opensm/include/opensm/osm_event_plugin.h | 12 ++++++------
> opensm/opensm/osm_event_plugin.c | 23 +++++++++++------------
> opensm/opensm/osm_opensm.c | 2 +-
> opensm/osmeventplugin/src/osmeventplugin.c | 18 ++++++++++++------
> 4 files changed, 30 insertions(+), 25 deletions(-)
>
> diff --git a/opensm/include/opensm/osm_event_plugin.h b/opensm/include/opensm/osm_event_plugin.h
> index 35d5b7d..953709d 100644
> --- a/opensm/include/opensm/osm_event_plugin.h
> +++ b/opensm/include/opensm/osm_event_plugin.h
> @@ -37,7 +37,6 @@
> #include <time.h>
> #include <iba/ib_types.h>
> #include <complib/cl_qlist.h>
> -#include <opensm/osm_log.h>
>
> #ifdef __cplusplus
> # define BEGIN_C_DECLS extern "C" {
> @@ -60,6 +59,8 @@ BEGIN_C_DECLS
> *********/
>
> #define OSM_EPI_NODE_NAME_LEN (128)
> +
> +struct osm_opensm;
> /** =========================================================================
> * Event types
> */
> @@ -145,11 +146,11 @@ typedef struct osm_epi_trap_event {
> * The version should be set to OSM_EVENT_PLUGIN_INTERFACE_VER
> */
> #define OSM_EVENT_PLUGIN_IMPL_NAME "osm_event_plugin"
> -#define OSM_EVENT_PLUGIN_INTERFACE_VER (1)
> +#define OSM_EVENT_PLUGIN_INTERFACE_VER 2
> typedef struct osm_event_plugin {
> int interface_version;
> - void *(*construct) (osm_log_t * osm_log);
> - void (*destroy) (void *plugin_data);
> + void *(*create) (struct osm_opensm *osm);
> + void (*delete) (void *plugin_data);
> void (*report) (void *plugin_data,
> osm_epi_event_id_t event_id, void *event_data);
> } osm_event_plugin_t;
> @@ -162,14 +163,13 @@ typedef struct osm_epi_plugin {
> void *handle;
> osm_event_plugin_t *impl;
> void *plugin_data;
> - osm_log_t *p_log;
> char *plugin_name;
> } osm_epi_plugin_t;
>
> /**
> * functions
> */
> -osm_epi_plugin_t *osm_epi_construct(osm_log_t * p_log, char *plugin_name);
> +osm_epi_plugin_t *osm_epi_construct(struct osm_opensm *osm, char *plugin_name);
> void osm_epi_destroy(osm_epi_plugin_t * plugin);
>
> /** =========================================================================
> diff --git a/opensm/opensm/osm_event_plugin.c b/opensm/opensm/osm_event_plugin.c
> index e579fad..98d5302 100644
> --- a/opensm/opensm/osm_event_plugin.c
> +++ b/opensm/opensm/osm_event_plugin.c
> @@ -44,8 +44,8 @@
>
> #include <stdlib.h>
> #include <dlfcn.h>
> -
> #include <opensm/osm_event_plugin.h>
> +#include <opensm/osm_opensm.h>
>
> #if defined(PATH_MAX)
> #define OSM_PATH_MAX (PATH_MAX + 1)
> @@ -58,7 +58,7 @@
> /**
> * functions
> */
> -osm_epi_plugin_t *osm_epi_construct(osm_log_t * p_log, char *plugin_name)
> +osm_epi_plugin_t *osm_epi_construct(osm_opensm_t *osm, char *plugin_name)
> {
> char lib_name[OSM_PATH_MAX];
> osm_epi_plugin_t *rc = NULL;
> @@ -75,7 +75,7 @@ osm_epi_plugin_t *osm_epi_construct(osm_log_t * p_log, char *plugin_name)
>
> rc->handle = dlopen(lib_name, RTLD_LAZY);
> if (!rc->handle) {
> - OSM_LOG(p_log, OSM_LOG_ERROR,
> + OSM_LOG(&osm->log, OSM_LOG_ERROR,
> "Failed to open event plugin \"%s\" : \"%s\"\n",
> lib_name, dlerror());
> goto DLOPENFAIL;
> @@ -85,7 +85,7 @@ osm_epi_plugin_t *osm_epi_construct(osm_log_t * p_log, char *plugin_name)
> (osm_event_plugin_t *) dlsym(rc->handle,
> OSM_EVENT_PLUGIN_IMPL_NAME);
> if (!rc->impl) {
> - OSM_LOG(p_log, OSM_LOG_ERROR,
> + OSM_LOG(&osm->log, OSM_LOG_ERROR,
> "Failed to find \"%s\" symbol in \"%s\" : \"%s\"\n",
> OSM_EVENT_PLUGIN_IMPL_NAME, lib_name, dlerror());
> goto Exit;
> @@ -93,7 +93,7 @@ osm_epi_plugin_t *osm_epi_construct(osm_log_t * p_log, char *plugin_name)
>
> /* Check the version to make sure this module will work with us */
> if (rc->impl->interface_version != OSM_EVENT_PLUGIN_INTERFACE_VER) {
> - OSM_LOG(p_log, OSM_LOG_ERROR,
> + OSM_LOG(&osm->log, OSM_LOG_ERROR,
> "Error opening %s: "
> "%s symbol is the wrong version %d != %d\n",
> plugin_name,
> @@ -103,19 +103,18 @@ osm_epi_plugin_t *osm_epi_construct(osm_log_t * p_log, char *plugin_name)
> goto Exit;
> }
>
> - if (!rc->impl->construct) {
> - OSM_LOG(p_log, OSM_LOG_ERROR,
> - "%s symbol has no construct function\n",
> + if (!rc->impl->create) {
> + OSM_LOG(&osm->log, OSM_LOG_ERROR,
> + "%s symbol has no create() function\n",
> OSM_EVENT_PLUGIN_IMPL_NAME);
> goto Exit;
> }
>
> - rc->plugin_data = rc->impl->construct(p_log);
> + rc->plugin_data = rc->impl->create(osm);
>
> if (!rc->plugin_data)
> goto Exit;
>
> - rc->p_log = p_log;
> rc->plugin_name = strdup(plugin_name);
> return (rc);
>
> @@ -129,8 +128,8 @@ DLOPENFAIL:
> void osm_epi_destroy(osm_epi_plugin_t * plugin)
> {
> if (plugin) {
> - if (plugin->impl->destroy)
> - plugin->impl->destroy(plugin->plugin_data);
> + if (plugin->impl->delete)
> + plugin->impl->delete(plugin->plugin_data);
> dlclose(plugin->handle);
> free(plugin->plugin_name);
> free(plugin);
> diff --git a/opensm/opensm/osm_opensm.c b/opensm/opensm/osm_opensm.c
> index 0101b97..6cf4726 100644
> --- a/opensm/opensm/osm_opensm.c
> +++ b/opensm/opensm/osm_opensm.c
> @@ -251,7 +251,7 @@ static void load_plugins(osm_opensm_t *osm, char *plugin_names)
>
> name = strtok_r(plugin_names, " \t\n", &p);
> while (name && *name) {
> - epi = osm_epi_construct(&osm->log, name);
> + epi = osm_epi_construct(osm, name);
> if (!epi)
> osm_log(&osm->log, OSM_LOG_ERROR,
> "cannot load plugin \'%s\'\n", name);
> diff --git a/opensm/osmeventplugin/src/osmeventplugin.c b/opensm/osmeventplugin/src/osmeventplugin.c
> index 6cc4c70..a56d3c3 100644
> --- a/opensm/osmeventplugin/src/osmeventplugin.c
> +++ b/opensm/osmeventplugin/src/osmeventplugin.c
> @@ -38,12 +38,15 @@
> #include <errno.h>
> #include <string.h>
> #include <stdlib.h>
> +#include <stdio.h>
> #include <time.h>
> #include <dlfcn.h>
> #include <stdint.h>
> -#include <opensm/osm_event_plugin.h>
> #include <complib/cl_qmap.h>
> #include <complib/cl_passivelock.h>
> +#include <opensm/osm_version.h>
> +#include <opensm/osm_opensm.h>
> +#include <opensm/osm_log.h>
>
> /** =========================================================================
> * This is a simple example plugin which logs some of the events the OSM
> @@ -57,8 +60,11 @@ typedef struct _log_events {
>
> /** =========================================================================
> */
> -static void *construct(osm_log_t * osmlog)
> +static void *construct(osm_opensm_t *osm)
> {
> + if (strcmp(osm->osm_version, OSM_VERSION))
> + return NULL;
> +
> _log_events_t *log = malloc(sizeof(*log));
> if (!log)
> return (NULL);
> @@ -66,14 +72,14 @@ static void *construct(osm_log_t * osmlog)
> log->log_file = fopen(SAMPLE_PLUGIN_OUTPUT_FILE, "a+");
>
> if (!(log->log_file)) {
> - osm_log(osmlog, OSM_LOG_ERROR,
> + osm_log(&osm->log, OSM_LOG_ERROR,
> "Sample Event Plugin: Failed to open output file \"%s\"\n",
> SAMPLE_PLUGIN_OUTPUT_FILE);
> free(log);
> return (NULL);
> }
>
> - log->osmlog = osmlog;
> + log->osmlog = &osm->log;
> return ((void *)log);
> }
>
> @@ -173,7 +179,7 @@ static void report(void *_log, osm_epi_event_id_t event_id, void *event_data)
> */
> osm_event_plugin_t osm_event_plugin = {
> interface_version:OSM_EVENT_PLUGIN_INTERFACE_VER,
> - construct:construct,
> - destroy:destroy,
> + create:construct,
> + delete:destroy,
> report:report
> };
More information about the general
mailing list