[openib-general] Re: [PATCH] Opensm - osmt_service.c fixes - take #2

Hal Rosenstock halr at voltaire.com
Wed Feb 15 09:59:47 PST 2006


On Wed, 2006-02-15 at 05:35, Yael Kalka wrote:
> Hi Hal,
> 
> The following patch add the following to osmt_service.c:
> 1. Currently, the flow sometimes exits with "TEST PATH" although the test
> actually fails.
> 2. Added cleanup of all the services created at the end of the test.
> 3. Cosmetic cleanups of the code.
> 
> Thanks,
> Yael
> 
> Signed-off-by:  Yael Kalka <yael at mellanox.co.il>

Thanks. I applied this in 2 pieces (osm_sa_service_record.c and then
osmt_service.c). I fixed a couple of minor nits noted below.

> Index: osmtest/osmt_service.c
> ===================================================================
> --- osmtest/osmt_service.c	(revision 5403)
> +++ osmtest/osmt_service.c	(working copy)

[snip...]

> @@ -539,62 +542,73 @@ osmt_get_service_by_id_and_name ( IN osm
>    if( status != IB_SUCCESS )
>    {
>      osm_log( &p_osmt->log, OSM_LOG_ERROR,
> -             "osmt_get_service_by_id_and_name: ERR 0365: "
> -             "ib_query failed (%s).\n", ib_get_err_str( status ) );
> +             "osmt_get_service_by_id_and_name: ERR 4A07: "
> +             "ib_query failed (%s)\n", ib_get_err_str( status ) );
>      goto Exit;
>    }
>  
>    status = context.result.status;
> +  num_recs =  context.result.result_cnt;
>  
>    if( status != IB_SUCCESS )
>    {
> -    num_recs = 0;
> -    if (status != IB_INVALID_PARAMETER)
> -    {
> -      osm_log( &p_osmt->log, OSM_LOG_ERROR,
> -               "osmt_get_service_by_id_and_name: ERR 0370: "
> -               "ib_query failed (%s).\n", ib_get_err_str( status ) );
> -    }
> +    char mad_stat_err[256];
> +    /* If the failure is due to IB_SA_MAD_STATUS_NO_RECORDS and rec_num is 0,
> +        then this is fine. */
>      if( status == IB_REMOTE_ERROR )
> +      strcpy(mad_stat_err, ib_get_mad_status_str( 
> +        osm_madw_get_mad_ptr(context.result.p_result_madw) ) );
> +    else
> +      strcpy(mad_stat_err, ib_get_err_str(status) );
> +    if( status == IB_REMOTE_ERROR && 
> +        !strcmp(mad_stat_err, "IB_SA_MAD_STATUS_NO_RECORDS") &&
> +        rec_num == 0 )
>      {
>        osm_log( &p_osmt->log, OSM_LOG_ERROR,
>                 "osmt_get_service_by_id_and_name: "
> -               "Remote error = %s.\n",
> -               ib_get_mad_status_str( osm_madw_get_mad_ptr
> -                                      ( context.result.
> -                                        p_result_madw ) ) );
> -    }
> -    goto Exit;
> +               "IS EXPECTED ERROR ^^^^\n");
> +      status = IB_SUCCESS;
>    }
>    else
>    {
> -    num_recs =  context.result.result_cnt;
> +      osm_log( &p_osmt->log, OSM_LOG_ERROR,
> +               "osmt_get_service_by_id_and_name: ERR 4A08: "
> +               "Query failed:%s (%s)\n",
> +               ib_get_err_str(status),
> +               mad_stat_err );
> +      goto Exit;
> +    }
>    }
>  
> -  if ( num_recs != rec_num )
> +  if ( rec_num && num_recs != rec_num )
>    {
>      osm_log( &p_osmt->log, OSM_LOG_ERROR,
>               "osmt_get_service_by_id_and_name: "
> -             "Unmatched record number, Expeceted : %d, Got : %d.\n",
> +             "Unmatched number of records: expeceted:%d, received:%d\n",
>               rec_num, num_recs);
>      status = IB_REMOTE_ERROR;
>      goto Exit;
>    }
>  
>    p_rec = osmv_get_query_svc_rec( context.result.p_result_madw, 0 );
> +  *p_out_rec = *p_rec;
> +
> +  if (num_recs)
> +  {
>    osm_log( &p_osmt->log, OSM_LOG_VERBOSE,
>             "osmt_get_service_by_id_and_name: "
> -           "Found Service Record by Name:%s ID:0x%016" PRIx64 ".\n",
> +             "Found service record: name:%s id:0x%016" PRIx64 "\n",
>             p_rec->service_name, cl_ntoh64(p_rec->service_id));
>  
>    osm_dump_service_record(&p_osmt->log, p_rec, OSM_LOG_DEBUG);
> -  *p_out_rec = *p_rec;
> +  }
>  
>   Exit:
>    osm_log( &p_osmt->log, OSM_LOG_VERBOSE,
>             "osmt_get_service_by_id_and_name: "
> -           "Expected num of records is : %d, Found number of records : %d\n",
> -           rec_num,num_recs);
> +           "Expected and found $d records\n",

                                  ^^^
                                  %d
> +           rec_num );
> +
>    if( context.result.p_result_madw != NULL )
>    {
>      osm_mad_pool_put( &p_osmt->mad_pool, context.result.p_result_madw );

[snip...]

> @@ -893,78 +939,90 @@ osmt_get_service_by_name( IN osmtest_t *
>  
>    context.p_osmt = p_osmt;
>  
> +  /* prepare the data used for this query */
>    req.query_type = OSMV_QUERY_SVC_REC_BY_NAME;
>    req.timeout_ms = p_osmt->opt.transaction_timeout;
>    req.retry_cnt = p_osmt->opt.retry_count;
>    req.flags = OSM_SA_FLAGS_SYNC;
>    req.query_context = &context;
>    req.pfn_query_cb = osmtest_query_res_cb;
> +  req.sm_key = 0;
> +
>    cl_memclr(service_name, sizeof(service_name));
>    cl_memcpy(service_name, sr_name, (strlen(sr_name)+1)*sizeof(char));
>    req.p_query_input = service_name;
> -  req.sm_key = 0;
>  
>    status = osmv_query_sa( p_osmt->h_bind, &req );
>    if( status != IB_SUCCESS )
>    {
>      osm_log( &p_osmt->log, OSM_LOG_ERROR,
> -             "osmt_get_service_by_name: ERR 0365: "
> -             "ib_query failed (%s).\n", ib_get_err_str( status ) );
> +             "osmt_get_service_by_name: ERR 4A0E: "
> +             "ib_query failed (%s)\n", ib_get_err_str( status ) );
>      goto Exit;
>    }
>  
>    status = context.result.status;
> +  num_recs =  context.result.result_cnt;
>  
>    if( status != IB_SUCCESS )
>    {
> -    /*  The context struct is not init OR result with illegal number of records */
> -    num_recs = 0;
> -    if (status != IB_INVALID_PARAMETER)
> -    {
> -      osm_log( &p_osmt->log, OSM_LOG_ERROR,
> -               "osmt_get_service_by_name: ERR 0370: "
> -               "ib_query failed (%s).\n", ib_get_err_str( status ) );
> -    }
> +    char mad_stat_err[256];
> +    /*  If the failure is due to IB_SA_MAD_STATUS_NO_RECORDS and rec_num is 0,
> +        then this is fine. */
>      if( status == IB_REMOTE_ERROR )
> +      strcpy(mad_stat_err, ib_get_mad_status_str( 
> +        osm_madw_get_mad_ptr(context.result.p_result_madw) ) );
> +    else
> +      strcpy(mad_stat_err, ib_get_err_str(status) );
> +
> +    if( status == IB_REMOTE_ERROR && 
> +        !strcmp(mad_stat_err, "IB_SA_MAD_STATUS_NO_RECORDS") &&
> +        rec_num == 0 )
>      {
>        osm_log( &p_osmt->log, OSM_LOG_ERROR,
>                 "osmt_get_service_by_name: "
> -               "Remote error = %s.\n",
> -               ib_get_mad_status_str( osm_madw_get_mad_ptr
> -                                      ( context.result.
> -                                        p_result_madw ) ) );
> -    }
> -    goto Exit;
> +               "IS EXPECTED ERROR ^^^^\n");
> +      status = IB_SUCCESS;
>    }
>    else
>    {
> -    num_recs =  context.result.result_cnt;
> +      osm_log( &p_osmt->log, OSM_LOG_ERROR,
> +               "osmt_get_service_by_name: ERR 4A0F: "
> +               "Query failed:%s (%s)\n",
> +               ib_get_err_str(status),
> +               mad_stat_err );
> +      goto Exit;
> +    }
>    }
>  
> -  if ( num_recs != rec_num )
> +  if ( rec_num && num_recs != rec_num )
>    {
>      osm_log( &p_osmt->log, OSM_LOG_ERROR,
> -             "osmt_get_service_by_name: "
> -             "Unmatched record number, Expeceted : %d, Got : %u.\n",
> +             "osmt_get_service_by_name: ERR 4A10: "
> +             "Unmatched number of records: expected:%d, received:%u\n",
>               rec_num, num_recs);
>      status = IB_REMOTE_ERROR;
>      goto Exit;
>    }
>  
>    p_rec = osmv_get_query_svc_rec( context.result.p_result_madw, 0 );
> +  *p_out_rec = *p_rec;
> +
> +  if (num_recs)
> +  {
>    osm_log( &p_osmt->log, OSM_LOG_VERBOSE,
>             "osmt_get_service_by_name: "
> -           "Found Service Record by Name:%s ID:0x%016" PRIx64 ".\n",
> +             "Found service record: name:%s id:0x%016" PRIx64 "\n",
>             sr_name, cl_ntoh64(p_rec->service_id));
>  
>    osm_dump_service_record(&p_osmt->log, p_rec, OSM_LOG_DEBUG);
> -  *p_out_rec = *p_rec;
> +  }
>  
>   Exit:
>    osm_log( &p_osmt->log, OSM_LOG_VERBOSE,
>             "osmt_get_service_by_name: "
> -           "Expected num of records is : %d, Found number of records : %u\n",
                                                                      ^^
                                                                      %d
> -           rec_num,num_recs);
> +           "Expected and found %d records\n",
> +           rec_num );
>  
>    if( context.result.p_result_madw != NULL )
>    {
> @@ -1002,7 +1060,7 @@ osmt_get_all_services_and_check_names( I
>    {
>      osm_log( &p_osmt->log, OSM_LOG_VERBOSE,
>               "osmt_get_all_services_and_check_names: "
> -             "Getting All Service Records\n");
> +             "Getting all service records\n");
>    }
>    /*
>     * Do a blocking query for this record in the subnet.

[snip...]





More information about the general mailing list