Hi Bernd, On 1/4/2014 5:23 PM, Bernd Schubert wrote: > Hello Hal, > > thanks for updating the patch! I had it on my todo list during my vacation > (still one week left) and didn't have the time to care about it before. > > What do you think about the patch below on top of your updated patch? > (So far I only tested if it compiles at all.) > > > From: Bernd Schubert <[email protected]> > > When the partition config file is parsed, partitions are created > for each line. If one of the config lines has an error a default > partition is created already by a previous commit, but partitions > from previous lines are not cleaned up.
Would a better policy for this be to honor all good partition config lines and ignore/warn about the bad ones rather than revert to default ? So as long as there was at least one good line in partition config it would use that; otherwise the default partition config. Does that make sense ? -- Hal > Avoid this by doing a dry-run parse first and only if this succeeds > actually create new partitions. > This still ignores errors from osm_prtn_make_new(), but these > errors are rather unlikely (i.e. malloc fails) and not really > related to the config file. > > Signed-off-by: Bernd Schubert <[email protected]> > --- > opensm/osm_prtn_config.c | 47 > ++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 41 insertions(+), 6 deletions(-) > > diff --git a/opensm/osm_prtn_config.c b/opensm/osm_prtn_config.c > index 9bad7a7..4a2eaab 100644 > --- a/opensm/osm_prtn_config.c > +++ b/opensm/osm_prtn_config.c > @@ -212,7 +212,8 @@ static void __create_mgrp(struct part_conf *conf, struct > precreate_mgroup *group > } > > static int partition_create(unsigned lineno, struct part_conf *conf, > - char *name, char *id, char *flag, char *flag_val) > + char *name, char *id, char *flag, char *flag_val, > + boolean_t is_dry_run) > { > ib_net16_t pkey; > > @@ -230,6 +231,9 @@ static int partition_create(unsigned lineno, struct > part_conf *conf, > } else > pkey = 0; > > + if (is_dry_run) > + return 0; > + > conf->p_prtn = osm_prtn_make_new(conf->p_log, conf->p_subn, > name, pkey); > if (!conf->p_prtn) > @@ -601,7 +605,11 @@ static int flush_part_conf(struct part_conf *conf) > return 0; > } > > -static int parse_part_conf(struct part_conf *conf, char *str, int lineno) > +/** > + *@is_dry_run only test if this line is valid > + */ > +static int parse_part_conf(struct part_conf *conf, char *str, int lineno, > + boolean_t is_dry_run) > { > int ret, len = 0; > char *name, *id, *flag, *flval; > @@ -659,7 +667,8 @@ static int parse_part_conf(struct part_conf *conf, char > *str, int lineno) > } > > if (p != str || (partition_create(lineno, conf, > - name, id, flag, flval) < 0)) { > + name, id, flag, flval, > + is_dry_run) < 0)) { > OSM_LOG(conf->p_log, OSM_LOG_ERROR, "PARSE ERROR: line %d: " > "bad partition definition\n", lineno); > fprintf(stderr, "\nPARSE ERROR: line %d: " > @@ -697,10 +706,12 @@ done: > } > > /** > + * @is_dry_run test if the config is valid only > * @return -1 on error, 0 on success > */ > -int osm_prtn_config_parse_file(osm_log_t * p_log, osm_subn_t * p_subn, > - const char *file_name) > +static int _osm_prtn_config_parse_file(osm_log_t * p_log, osm_subn_t * > p_subn, > + const char *file_name, > + boolean_t is_dry_run) > { > char line[4096]; > struct part_conf *conf = NULL; > @@ -757,7 +768,7 @@ int osm_prtn_config_parse_file(osm_log_t * p_log, > osm_subn_t * p_subn, > if (q) > *q = '\0'; > > - len = parse_part_conf(conf, p, lineno); > + len = parse_part_conf(conf, p, lineno, is_dry_run); > if (len < 0) { > is_parse_success = FALSE; > break; > @@ -779,3 +790,27 @@ int osm_prtn_config_parse_file(osm_log_t * p_log, > osm_subn_t * p_subn, > else > return -1; > } > + > + > +/** > + * @return -1 on error, 0 on success > + */ > +int osm_prtn_config_parse_file(osm_log_t * p_log, osm_subn_t * p_subn, > + const char *file_name) > +{ > + boolean_t is_dry_run = TRUE; // dry-run first > + int parse_res; > + > + while (1) { > + parse_res =_osm_prtn_config_parse_file(p_log, p_subn, file_name, > + is_dry_run); > + if (parse_res != 0); > + return -1; > + > + if (!is_dry_run) > + return 0; // parse / partition create success > + > + is_dry_run = FALSE; > + } > + > +} > > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
