[ofa-general] Re: [PATCH v2] opensm/osm_console.c : Added dump_portguid function to console to generate a list of port guids matching one or more regexps

[Nicolas Morey-Chaisemartin]

Sasha Khapyorsky a écrit :
> Hi Nicolas,
>
> On 09:08 Tue 10 Feb     , Nicolas Morey Chaisemartin wrote:
>   
>> This add a dump_portguid functionnality to openSM console which makes it 
>> really easy to generate cn_guid_file, root_guid_file and such
>> by dumping into a file all port guids whom nodedesc contains at least one 
>> of the provided regexps
>>
>> Signed-off-by: Nicolas Morey-Chaisemartin 
>> <nicolas.morey-chaisemartin at ext.bull.net>
>> ---
>>
>> Repost without exit_after_run flag, active sleep init loop and indented.
>>
>>  opensm/opensm/osm_console.c |  105 
>> +++++++++++++++++++++++++++++++++++++++++++
>>  1 files changed, 105 insertions(+), 0 deletions(-)
>>
>>
>>     
>
>   
>> diff --git a/opensm/opensm/osm_console.c b/opensm/opensm/osm_console.c
>> index c6e8e59..5fbcd43 100644
>> --- a/opensm/opensm/osm_console.c
>> +++ b/opensm/opensm/osm_console.c
>> @@ -42,6 +42,7 @@
>>  #include <sys/types.h>
>>  #include <sys/socket.h>
>>  #include <netdb.h>
>> +#include <regex.h>
>>  #ifdef ENABLE_OSM_CONSOLE_SOCKET
>>  #include <arpa/inet.h>
>>  #endif
>> @@ -1173,6 +1174,109 @@ static void version_parse(char **p_last, osm_opensm_t * p_osm, FILE * out)
>>  }
>>  
>>  /* more parse routines go here */
>> +typedef struct _regexp_list {
>> +	regex_t exp;
>> +	struct _regexp_list *next;
>> +} regexp_list_t;
>> +
>> +static void dump_portguid_parse(char **p_last, osm_opensm_t * p_osm, FILE * out)
>> +{
>> +	cl_qmap_t *p_port_guid_tbl;
>> +	osm_port_t *p_port;
>> +	osm_port_t *p_next_port;
>> +
>> +	regexp_list_t *p_head_regexp = NULL;
>> +	regexp_list_t *p_regexp;
>> +
>> +	/* Option variables */
>> +	char *p_cmd = NULL;
>> +	FILE *output = out;
>> +
>> +	/* Read commande line */
>> +
>> +	while (1) {
>> +		p_cmd = next_token(p_last);
>> +		if (p_cmd) {
>> +			if (strcmp(p_cmd, "file") == 0) {
>> +				p_cmd = next_token(p_last);
>> +				if (p_cmd) {
>> +					output = fopen(p_cmd, "w+");
>> +					if (output == NULL) {
>> +						fprintf(out,
>> +							"Could not open file %s: %s\n",
>> +							p_cmd, strerror(errno));
>> +						output = out;
>> +					}
>> +				} else
>> +					fprintf(out, "No file name passed\n");
>> +			} else {
>> +				p_regexp = malloc(sizeof(*p_regexp));
>> +				if (regcomp
>> +				    (&(p_regexp->exp), p_cmd,
>> +				     REG_NOSUB | REG_EXTENDED) != 0) {
>> +					fprintf(out,
>> +						"Couldn't parse regular expression %s. Skipping it.\n",
>> +						p_cmd);
>> +				}
>> +				p_regexp->next = p_head_regexp;
>> +				p_head_regexp = p_regexp;
>> +			}
>> +		} else
>> +			break;	/* No more tokens */
>> +
>> +	}
>> +
>> +	/* Check we have at least one expression to match */
>> +	if (p_head_regexp == NULL) {
>> +		fprintf(out, "No valid expression provided. Aborting\n");
>> +		return;
>> +	}
>> +
>> +	cl_spinlock_release(&p_osm->sm.state_lock);
>>     
>
> What is this cl_spinlock_release()? Typo?
>
>   
>> +	if (p_osm->sm.p_subn->need_update != 0) {
>> +		fprintf(out, "Subnet is not ready yet. Try again later.\n");
>> +		return;
>> +	}
>> +
>> +	/* Subnet doesn't need to be updated so we can carry on */
>> +
>> +	CL_PLOCK_EXCL_ACQUIRE(p_osm->sm.p_lock);
>> +	p_port_guid_tbl = &(p_osm->sm.p_subn->port_guid_tbl);
>>     
>
> Do we really need exclusive locking here? port_guid_table content is
> rad-only, I guess "read-only" lock (CL_PLOCK_ACQUIRE()) should be enough.
>
> The rest looks fine for me.
>
> Sasha
>
>   

Read only is fine. I didn't know complib provided different kinds of lock.

Nicolas