Re: [PATCH] opensm: Add a name to IB subnet and include it in syslog messages
Hi Sasha,
I have incorporated your feedback and sent you the modified patch with a new
subject line "[PATCH V2] opensm: Add option to specify prefix to syslog
messages"
Regards,
Benjamin
Sasha Khapyorsky wrote:
> Hi Benjamin,
>
> On 14:16 Thu 21 Jan , Arputham Benjamin wrote:
>> Added a text based name to an IB subnet to help user in identifying
>> an IB subnet or understanding its function in a multi-fabric IB cluster.
>> For example, in a dual-fabric (or dual-rail) IB cluster, one subnet
>> could be named "mpi" and the other subnet could be named "storage".
>>
>> Summary of changes:
>> o Added the option 'subnet_name' to OpenSM command line and config file.
>> o Enhanced OpenSM logging facility to include the subnet name in
>>syslog messages.
>
> Looking at the usage below I can see that his is done as adding "free
> text" prefix to syslog prints. This is fine and seems could be useful
> for any purpose (not only different subnets) when syslog message mark
> is desired.
>
> Assuming so, wouldn't it be better to change "subnet_name" to something
> more generic, let say "log_prefix"?
>
>> Signed-off-by: Arputham Benjamin
>> ---
>> diff -rup a/include/opensm/osm_log.h b/include/opensm/osm_log.h
>> --- a/include/opensm/osm_log.h 2010-01-18 21:32:12.195328129 -0800
>> +++ b/include/opensm/osm_log.h 2010-01-18 21:34:46.573932164 -0800
>> @@ -120,6 +120,7 @@ typedef struct osm_log {
>> boolean_t accum_log_file;
>> boolean_t daemon;
>> char *log_file_name;
>> +char *subnet_name;
>> } osm_log_t;
>> /*/
>>
>> diff -rup a/include/opensm/osm_subnet.h b/include/opensm/osm_subnet.h
>> --- a/include/opensm/osm_subnet.h2010-01-18 21:32:12.195328129 -0800
>> +++ b/include/opensm/osm_subnet.h2010-01-18 21:34:55.782087826 -0800
>> @@ -224,6 +224,7 @@ typedef struct osm_subn_opt {
>> char *event_plugin_name;
>> char *node_name_map_name;
>> char *prefix_routes_file;
>> +char *subnet_name;
>> boolean_t consolidate_ipv6_snm_req;
>> struct osm_subn_opt *file_opts; /* used for update */
>> uint8_t lash_start_vl; /* starting vl to use in lash */
>> diff -rup a/man/opensm.8.in b/man/opensm.8.in
>> --- a/man/opensm.8.in2010-01-19 11:29:03.954832199 -0800
>> +++ b/man/opensm.8.in2010-01-21 10:56:58.901836423 -0800
>> @@ -49,6 +49,7 @@ opensm \- InfiniBand subnet manager and
>> [\-\-perfmgr_sweep_time_s ]
>> [\-\-prefix_routes_file ]
>> [\-\-consolidate_ipv6_snm_req]
>> +[\-\-subnet_name ]
>> [\-v(erbose)] [\-V] [\-D ] [\-d(ebug) ]
>> [\-h(elp)] [\-?]
>>
>> @@ -345,6 +346,9 @@ effect if --enable-perfmgr was specified
>> Use shared MLID for IPv6 Solicited Node Multicast groups per MGID scope
>> and P_Key.
>> .TP
>> +\fB\-\-subnet_name\fR
>> +This option specifies the text based name of the subnet.
>> +.TP
>> \fB\-v\fR, \fB\-\-verbose\fR
>> This option increases the log verbosity level.
>> The -v option may be specified multiple times
>> diff -rup a/opensm/main.c b/opensm/main.c
>> --- a/opensm/main.c 2010-01-18 21:31:43.318842260 -0800
>> +++ b/opensm/main.c 2010-01-19 11:51:45.566967909 -0800
>> @@ -324,6 +324,8 @@ static void show_usage(void)
>> printf("--consolidate_ipv6_snm_req\n"
>> " Use shared MLID for IPv6 Solicited Node Multicast
>> groups\n"
>> " per MGID scope and P_Key.\n\n");
>> +printf("--subnet_name \n"
>> + " Text based name of the IB subnet.\n\n");
>> printf("--verbose, -v\n"
>> " This option increases the log verbosity level.\n"
>> " The -v option may be specified multiple times\n"
>> @@ -607,6 +609,7 @@ int main(int argc, char *argv[])
>> {"lash_start_vl", 1, NULL, 6},
>> {"sm_sl", 1, NULL, 7},
>> {"retries", 1, NULL, 8},
>> +{"subnet_name", 1, NULL, 9},
>> {NULL, 0, NULL, 0} /* Required at the end of the array */
>> };
>>
>> @@ -985,6 +988,11 @@ int main(int argc, char *argv[])
>> printf(" Transaction retries = %u\n",
>> opt.transaction_retries);
>> break;
>> +case 9:
>> +SET_STR_OPT(opt.subnet_name, optarg);
>> +printf("IB subnet name = %s\n",
>> + opt.subnet_name);
>> +break;
>> case 'h':
>> case '?':
>> case ':':
>> diff -rup a/opensm/osm_log.c b/opensm/osm_log.c
>> --- a/opensm/osm_log.c 2010-01-18 21:31:43.318842260 -0800
>> +++ b/opensm/osm_log.c 2010-01-18 21:33:47.808939648 -0800
>> @@ -107,6 +107,7 @@ void osm_log(IN osm_log_t * p_log, IN os
>> char buffer[LOG_ENTRY_SIZE_MAX];
>> va_list args;
>> int ret;
>> +uint8_t ind = 0;
>> #ifdef __WIN__
>> SYSTEMTIME st;
>> uint32_t pid = GetCurrentThreadId();
>> @@ -123,7 +124,14
Re: [PATCH] opensm: Add a name to IB subnet and include it in syslog messages
Hi Benjamin,
On 14:16 Thu 21 Jan , Arputham Benjamin wrote:
> Added a text based name to an IB subnet to help user in identifying
> an IB subnet or understanding its function in a multi-fabric IB cluster.
> For example, in a dual-fabric (or dual-rail) IB cluster, one subnet
> could be named "mpi" and the other subnet could be named "storage".
>
> Summary of changes:
> o Added the option 'subnet_name' to OpenSM command line and config file.
> o Enhanced OpenSM logging facility to include the subnet name in
>syslog messages.
Looking at the usage below I can see that his is done as adding "free
text" prefix to syslog prints. This is fine and seems could be useful
for any purpose (not only different subnets) when syslog message mark
is desired.
Assuming so, wouldn't it be better to change "subnet_name" to something
more generic, let say "log_prefix"?
>
> Signed-off-by: Arputham Benjamin
> ---
> diff -rup a/include/opensm/osm_log.h b/include/opensm/osm_log.h
> --- a/include/opensm/osm_log.h2010-01-18 21:32:12.195328129 -0800
> +++ b/include/opensm/osm_log.h2010-01-18 21:34:46.573932164 -0800
> @@ -120,6 +120,7 @@ typedef struct osm_log {
> boolean_t accum_log_file;
> boolean_t daemon;
> char *log_file_name;
> + char *subnet_name;
> } osm_log_t;
> /*/
>
> diff -rup a/include/opensm/osm_subnet.h b/include/opensm/osm_subnet.h
> --- a/include/opensm/osm_subnet.h 2010-01-18 21:32:12.195328129 -0800
> +++ b/include/opensm/osm_subnet.h 2010-01-18 21:34:55.782087826 -0800
> @@ -224,6 +224,7 @@ typedef struct osm_subn_opt {
> char *event_plugin_name;
> char *node_name_map_name;
> char *prefix_routes_file;
> + char *subnet_name;
> boolean_t consolidate_ipv6_snm_req;
> struct osm_subn_opt *file_opts; /* used for update */
> uint8_t lash_start_vl; /* starting vl to use in lash */
> diff -rup a/man/opensm.8.in b/man/opensm.8.in
> --- a/man/opensm.8.in 2010-01-19 11:29:03.954832199 -0800
> +++ b/man/opensm.8.in 2010-01-21 10:56:58.901836423 -0800
> @@ -49,6 +49,7 @@ opensm \- InfiniBand subnet manager and
> [\-\-perfmgr_sweep_time_s ]
> [\-\-prefix_routes_file ]
> [\-\-consolidate_ipv6_snm_req]
> +[\-\-subnet_name ]
> [\-v(erbose)] [\-V] [\-D ] [\-d(ebug) ]
> [\-h(elp)] [\-?]
>
> @@ -345,6 +346,9 @@ effect if --enable-perfmgr was specified
> Use shared MLID for IPv6 Solicited Node Multicast groups per MGID scope
> and P_Key.
> .TP
> +\fB\-\-subnet_name\fR
> +This option specifies the text based name of the subnet.
> +.TP
> \fB\-v\fR, \fB\-\-verbose\fR
> This option increases the log verbosity level.
> The -v option may be specified multiple times
> diff -rup a/opensm/main.c b/opensm/main.c
> --- a/opensm/main.c 2010-01-18 21:31:43.318842260 -0800
> +++ b/opensm/main.c 2010-01-19 11:51:45.566967909 -0800
> @@ -324,6 +324,8 @@ static void show_usage(void)
> printf("--consolidate_ipv6_snm_req\n"
> " Use shared MLID for IPv6 Solicited Node Multicast
> groups\n"
> " per MGID scope and P_Key.\n\n");
> + printf("--subnet_name \n"
> +" Text based name of the IB subnet.\n\n");
> printf("--verbose, -v\n"
> " This option increases the log verbosity level.\n"
> " The -v option may be specified multiple times\n"
> @@ -607,6 +609,7 @@ int main(int argc, char *argv[])
> {"lash_start_vl", 1, NULL, 6},
> {"sm_sl", 1, NULL, 7},
> {"retries", 1, NULL, 8},
> + {"subnet_name", 1, NULL, 9},
> {NULL, 0, NULL, 0} /* Required at the end of the array */
> };
>
> @@ -985,6 +988,11 @@ int main(int argc, char *argv[])
> printf(" Transaction retries = %u\n",
> opt.transaction_retries);
> break;
> + case 9:
> + SET_STR_OPT(opt.subnet_name, optarg);
> + printf("IB subnet name = %s\n",
> +opt.subnet_name);
> + break;
> case 'h':
> case '?':
> case ':':
> diff -rup a/opensm/osm_log.c b/opensm/osm_log.c
> --- a/opensm/osm_log.c2010-01-18 21:31:43.318842260 -0800
> +++ b/opensm/osm_log.c2010-01-18 21:33:47.808939648 -0800
> @@ -107,6 +107,7 @@ void osm_log(IN osm_log_t * p_log, IN os
> char buffer[LOG_ENTRY_SIZE_MAX];
> va_list args;
> int ret;
> + uint8_t ind = 0;
> #ifdef __WIN__
> SYSTEMTIME st;
> uint32_t pid = GetCurrentThreadId();
> @@ -123,7 +124,14 @@ void osm_log(IN osm_log_t * p_log, IN os
> return;
>
> va_start(args, p_str);
> - vsprintf(buffer, p_str, args);
> + if(p_log->subnet_name) {
> + snprintf(buffer, LOG_ENTRY_SIZE_MAX, "%s: ",
> p_log->subnet_name);
> + ind = strlen(buffer);
[PATCH] opensm: Add a name to IB subnet and include it in syslog messages
Added a text based name to an IB subnet to help user in identifying
an IB subnet or understanding its function in a multi-fabric IB cluster.
For example, in a dual-fabric (or dual-rail) IB cluster, one subnet
could be named "mpi" and the other subnet could be named "storage".
Summary of changes:
o Added the option 'subnet_name' to OpenSM command line and config file.
o Enhanced OpenSM logging facility to include the subnet name in
syslog messages.
Signed-off-by: Arputham Benjamin
---
diff -rup a/include/opensm/osm_log.h b/include/opensm/osm_log.h
--- a/include/opensm/osm_log.h 2010-01-18 21:32:12.195328129 -0800
+++ b/include/opensm/osm_log.h 2010-01-18 21:34:46.573932164 -0800
@@ -120,6 +120,7 @@ typedef struct osm_log {
boolean_t accum_log_file;
boolean_t daemon;
char *log_file_name;
+ char *subnet_name;
} osm_log_t;
/*/
diff -rup a/include/opensm/osm_subnet.h b/include/opensm/osm_subnet.h
--- a/include/opensm/osm_subnet.h 2010-01-18 21:32:12.195328129 -0800
+++ b/include/opensm/osm_subnet.h 2010-01-18 21:34:55.782087826 -0800
@@ -224,6 +224,7 @@ typedef struct osm_subn_opt {
char *event_plugin_name;
char *node_name_map_name;
char *prefix_routes_file;
+ char *subnet_name;
boolean_t consolidate_ipv6_snm_req;
struct osm_subn_opt *file_opts; /* used for update */
uint8_t lash_start_vl; /* starting vl to use in lash */
diff -rup a/man/opensm.8.in b/man/opensm.8.in
--- a/man/opensm.8.in 2010-01-19 11:29:03.954832199 -0800
+++ b/man/opensm.8.in 2010-01-21 10:56:58.901836423 -0800
@@ -49,6 +49,7 @@ opensm \- InfiniBand subnet manager and
[\-\-perfmgr_sweep_time_s ]
[\-\-prefix_routes_file ]
[\-\-consolidate_ipv6_snm_req]
+[\-\-subnet_name ]
[\-v(erbose)] [\-V] [\-D ] [\-d(ebug) ]
[\-h(elp)] [\-?]
@@ -345,6 +346,9 @@ effect if --enable-perfmgr was specified
Use shared MLID for IPv6 Solicited Node Multicast groups per MGID scope
and P_Key.
.TP
+\fB\-\-subnet_name\fR
+This option specifies the text based name of the subnet.
+.TP
\fB\-v\fR, \fB\-\-verbose\fR
This option increases the log verbosity level.
The -v option may be specified multiple times
diff -rup a/opensm/main.c b/opensm/main.c
--- a/opensm/main.c 2010-01-18 21:31:43.318842260 -0800
+++ b/opensm/main.c 2010-01-19 11:51:45.566967909 -0800
@@ -324,6 +324,8 @@ static void show_usage(void)
printf("--consolidate_ipv6_snm_req\n"
" Use shared MLID for IPv6 Solicited Node Multicast
groups\n"
" per MGID scope and P_Key.\n\n");
+ printf("--subnet_name \n"
+ " Text based name of the IB subnet.\n\n");
printf("--verbose, -v\n"
" This option increases the log verbosity level.\n"
" The -v option may be specified multiple times\n"
@@ -607,6 +609,7 @@ int main(int argc, char *argv[])
{"lash_start_vl", 1, NULL, 6},
{"sm_sl", 1, NULL, 7},
{"retries", 1, NULL, 8},
+ {"subnet_name", 1, NULL, 9},
{NULL, 0, NULL, 0} /* Required at the end of the array */
};
@@ -985,6 +988,11 @@ int main(int argc, char *argv[])
printf(" Transaction retries = %u\n",
opt.transaction_retries);
break;
+ case 9:
+ SET_STR_OPT(opt.subnet_name, optarg);
+ printf("IB subnet name = %s\n",
+ opt.subnet_name);
+ break;
case 'h':
case '?':
case ':':
diff -rup a/opensm/osm_log.c b/opensm/osm_log.c
--- a/opensm/osm_log.c 2010-01-18 21:31:43.318842260 -0800
+++ b/opensm/osm_log.c 2010-01-18 21:33:47.808939648 -0800
@@ -107,6 +107,7 @@ void osm_log(IN osm_log_t * p_log, IN os
char buffer[LOG_ENTRY_SIZE_MAX];
va_list args;
int ret;
+ uint8_t ind = 0;
#ifdef __WIN__
SYSTEMTIME st;
uint32_t pid = GetCurrentThreadId();
@@ -123,7 +124,14 @@ void osm_log(IN osm_log_t * p_log, IN os
return;
va_start(args, p_str);
- vsprintf(buffer, p_str, args);
+ if(p_log->subnet_name) {
+ snprintf(buffer, LOG_ENTRY_SIZE_MAX, "%s: ",
p_log->subnet_name);
+ ind = strlen(buffer);
+ vsprintf(&buffer[ind], p_str, args);
+ }
+ else {
+ vsprintf(buffer, p_str, args);
+ }
va_end(args);
/* this is a call to the syslog */
diff -rup a/opensm/osm_opensm.c b/opensm/osm_opensm.c
--- a/opensm/osm_opensm.c 2010-01-18 21:31:43.318842260 -0800
+++ b/opensm/osm_opensm.c 2010-01-18 21:34:00.125147535 -0800
@@ -337,6 +337,7 @@ ib_api_status_t osm_opensm_init(IN osm_o
p_opt->log_max_size, p_opt->accum_log_file)
