[ofa-general] Re: [PATCH 2/10] infiniband-diags: Convert ibaddr to "new" ibmad interface

[Ira Weiny]

On Sun, 1 Mar 2009 10:08:04 +0200
Sasha Khapyorsky <sashak at voltaire.com> wrote:

> On 19:05 Thu 19 Feb     , Ira Weiny wrote:

[snip]

> >  
> > -	madrpc_init(ibd_ca, ibd_ca_port, mgmt_classes, 3);
> > +	srcport = mad_rpc_open_port(ibd_ca, ibd_ca_port, mgmt_classes, 3);
> > +	if (!srcport)
> > +		IBERROR("Failed to open '%s' port '%d'", ibd_ca, ibd_ca_port);
> 
> Here and in almost all other infiniband-diags patches:
> 
> madrpc_init() in case of failures calls IBPANIC() which prints error
> message and calls exit(). Unlike this mad_rpc_open_port() sets errno and
> returns NULL, IBERROR() just prints error message - the program will
> continue execution...

Good catch I will update with a proper exit call.

[snip]

> > diff --git a/infiniband-diags/src/ibdiag_common.c b/infiniband-diags/src/ibdiag_common.c
> > index 5f2472d..609df69 100644
> > --- a/infiniband-diags/src/ibdiag_common.c
> > +++ b/infiniband-diags/src/ibdiag_common.c
> > @@ -179,7 +179,8 @@ static int process_opt(int ch, char *optarg)
> >  		ibd_timeout = val;
> >  		break;
> >  	case 's':
> > -		if (ib_resolve_portid_str(&sm_portid, optarg, IB_DEST_LID, 0) < 0)
> > +		if (ib_resolve_portid_str_via(&sm_portid, optarg, IB_DEST_LID,
> > +				0, NULL) < 0)
> 
> This may be not correct (not introduced by those patches) - then host is
> connected to multiple subnets we need to resolve SM LID on requested
> one, eg. using srcport...
> 

Would you accept adding srcport to ibdiag_process_opts?  Or would you prefer
something more generic?

Ira