[ofa-general] Re: [PATCH RFC] opensm/event_plugin: plugin API version 2

Ira Weiny weiny2 at llnl.gov
Thu Jun 26 09:39:26 PDT 2008


One comment, which I mentioned to you off-list, was that passing struct
osm_opensm without having that header public currently breaks my plugin and
will make it impossible for them to compile against the installed headers.

I am really leaning toward including all the headers in the opensm-devel
package so plugin writers don't have to have a git tree to build.

Ira


On Thu, 26 Jun 2008 01:23:31 +0300
Sasha Khapyorsky <sashak at voltaire.com> 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. 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
>  };
> -- 
> 1.5.5.1.178.g1f811
> 



More information about the general mailing list