[ofa-general] Re: [PATCH 7/7 V3] osm: QoS - reading policy file

[Sasha Khapyorsky]

On 02:11 Tue 28 Aug     , Yevgeny Kliteynik wrote:
> Reading QoS policy file
> 
> Signed-off-by: Yevgeny Kliteynik <kliteyn at dev.mellanox.co.il>

Applied. Thanks.

Some comments are below.

> ---
>  opensm/include/opensm/osm_subnet.h |   16 ++++++++--------
>  opensm/opensm/osm_state_mgr.c      |    2 +-
>  opensm/opensm/osm_subnet.c         |   17 +++++++++++------
>  3 files changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/opensm/include/opensm/osm_subnet.h b/opensm/include/opensm/osm_subnet.h
> index 5e8b522..7e1a3e7 100644
> --- a/opensm/include/opensm/osm_subnet.h
> +++ b/opensm/include/opensm/osm_subnet.h
> @@ -1118,28 +1118,28 @@ ib_api_status_t osm_subn_parse_conf_file(IN osm_subn_opt_t * const p_opt);
>  *	Subnet object, osm_subn_construct, osm_subn_destroy
>  *********/
> 
> -/****f* OpenSM: Subnet/osm_subn_parse_conf_file
> +/****f* OpenSM: Subnet/osm_subn_parse_conf_files
>  * NAME
> -*	osm_subn_rescan_conf_file
> +*	osm_subn_rescan_conf_files
>  *
>  * DESCRIPTION
> -*	The osm_subn_rescan_conf_file function parses the configuration
> -*	file and update selected subnet options
> +*	The osm_subn_rescan_conf_files function parses the configuration
> +*	files and update selected subnet options
>  *
>  * SYNOPSIS
>  */
> -ib_api_status_t osm_subn_rescan_conf_file(IN osm_subn_opt_t * const p_opts);
> +ib_api_status_t osm_subn_rescan_conf_files(IN osm_subn_t * const p_subn);
>  /*
>  * PARAMETERS
>  *
> -*	p_opt
> -*		[in] Pointer to the subnet options structure.
> +*	p_subn
> +*		[in] Pointer to the subnet structure.
>  *
>  * RETURN VALUES
>  *	IB_SUCCESS, IB_ERROR
>  *
>  * NOTES
> -*  This uses the same file as osm_subn_parse_conf_file()
> +*  This uses the same file as osm_subn_parse_conf_files()
>  *
>  *********/
> 
> diff --git a/opensm/opensm/osm_state_mgr.c b/opensm/opensm/osm_state_mgr.c
> index 030d344..2a94518 100644
> --- a/opensm/opensm/osm_state_mgr.c
> +++ b/opensm/opensm/osm_state_mgr.c
> @@ -1878,7 +1878,7 @@ void osm_state_mgr_process(IN osm_state_mgr_t * const p_mgr,
>  					p_mgr->p_subn->subnet_initialization_error = FALSE;
> 
>  					/* rescan configuration updates */
> -					status = osm_subn_rescan_conf_file(&p_mgr->p_subn->opt);
> +					status = osm_subn_rescan_conf_files(p_mgr->p_subn);
>  					if (status != IB_SUCCESS) {
>  						osm_log(p_mgr->p_log,
>  							OSM_LOG_ERROR,
> diff --git a/opensm/opensm/osm_subnet.c b/opensm/opensm/osm_subnet.c
> index 4162522..c847daf 100644
> --- a/opensm/opensm/osm_subnet.c
> +++ b/opensm/opensm/osm_subnet.c
> @@ -69,6 +69,7 @@
>  #include <opensm/osm_console.h>
>  #include <opensm/osm_perfmgr.h>
>  #include <opensm/osm_event_plugin.h>
> +#include <opensm/osm_qos_policy.h>
> 
>  #if defined(PATH_MAX)
>  #define OSM_PATH_MAX	(PATH_MAX + 1)
> @@ -677,7 +678,7 @@ subn_dump_qos_options(FILE * file,
> 
>  /**********************************************************************
>   **********************************************************************/
> -ib_api_status_t osm_subn_rescan_conf_file(IN osm_subn_opt_t * const p_opts)
> +ib_api_status_t osm_subn_rescan_conf_files(IN osm_subn_t * const p_subn)
>  {
>  	char *p_cache_dir = getenv("OSM_CACHE_DIR");
>  	char file_name[OSM_PATH_MAX];
> @@ -704,28 +705,32 @@ ib_api_status_t osm_subn_rescan_conf_file(IN osm_subn_opt_t * const p_opts)
> 
>  			subn_parse_qos_options("qos",
>  					       p_key, p_val,
> -					       &p_opts->qos_options);
> +					       &p_subn->opt.qos_options);
> 
>  			subn_parse_qos_options("qos_ca",
>  					       p_key, p_val,
> -					       &p_opts->qos_ca_options);
> +					       &p_subn->opt.qos_ca_options);
> 
>  			subn_parse_qos_options("qos_sw0",
>  					       p_key, p_val,
> -					       &p_opts->qos_sw0_options);
> +					       &p_subn->opt.qos_sw0_options);
> 
>  			subn_parse_qos_options("qos_swe",
>  					       p_key, p_val,
> -					       &p_opts->qos_swe_options);
> +					       &p_subn->opt.qos_swe_options);
> 
>  			subn_parse_qos_options("qos_rtr",
>  					       p_key, p_val,
> -					       &p_opts->qos_rtr_options);
> +					       &p_subn->opt.qos_rtr_options);
> 
>  		}
>  	}
>  	fclose(opts_file);
> 
> +	/* read QoS policy config file */
> +	if (!p_subn->opt.no_qos)
> +		osm_qos_parse_policy_file(p_subn);
> +

Should the qos parser return value be checked and an error logged in
case of failure?

Sasha

>  	return IB_SUCCESS;
>  }
> 
> -- 
> 1.5.1.4
>