On Thu, 23 Apr 2009 10:15:42 +0300 Sasha Khapyorsky <[email protected]> wrote:
> On 15:42 Fri 03 Apr , Ira Weiny wrote: > > From a677ae35fe7a5966f05b5859df8f00e9b18df864 Mon Sep 17 00:00:00 2001 > > You know :) > > > From: Ira Weiny <[email protected]> > > Date: Fri, 3 Apr 2009 15:28:18 -0700 > > Subject: [PATCH] Convert iblinkinfo.pl to C and use new ibnetdisc library. > > > > Signed-off-by: Ira Weiny <[email protected]> > > Applied. Thanks. Couple of notes are below. > > > --- > > infiniband-diags/Makefile.am | 7 +- > > infiniband-diags/configure.in | 1 + > > infiniband-diags/scripts/iblinkinfo.pl | 327 ------------------------ > > infiniband-diags/scripts/iblinkinfo.pl.in | 40 +++ > > infiniband-diags/src/iblinkinfo.c | 386 > > +++++++++++++++++++++++++++++ > > 5 files changed, 432 insertions(+), 329 deletions(-) > > delete mode 100755 infiniband-diags/scripts/iblinkinfo.pl > > create mode 100755 infiniband-diags/scripts/iblinkinfo.pl.in > > create mode 100644 infiniband-diags/src/iblinkinfo.c > > > > diff --git a/infiniband-diags/Makefile.am b/infiniband-diags/Makefile.am > > index 7b8523a..b480a4a 100644 > > --- a/infiniband-diags/Makefile.am > > +++ b/infiniband-diags/Makefile.am > > @@ -1,6 +1,7 @@ > > SUBDIRS = libibnetdisc > > > > -INCLUDES = -I$(top_builddir)/include/ -I$(srcdir)/include -I$(includedir) > > -I$(includedir)/infiniband > > +INCLUDES = -I$(top_builddir)/include/ -I$(srcdir)/include -I$(includedir) > > -I$(includedir)/infiniband \ > > + -I$(top_builddir)/libibnetdisc/include > > Here $(top_srcdir) should be used instead of $(top_builddir). > 'top_builddir' points project build directory, not source directory > (where not generated header files are located). And build like: > > mkdir /tmp/ib-diags && cd /tmp/ib-diags > /path/to/management/infiniband-diags/configure && make > > will fail. > > I'm fixing this. yep, thanks. > > > if DEBUG > > DBGFLAGS = -ggdb -D_DEBUG_ > > @@ -11,7 +12,7 @@ endif > > sbin_PROGRAMS = src/ibaddr src/ibnetdiscover src/ibping src/ibportstate \ > > src/ibroute src/ibstat src/ibsysstat src/ibtracert \ > > src/perfquery src/sminfo src/smpdump src/smpquery \ > > - src/saquery src/vendstat > > + src/saquery src/vendstat src/iblinkinfo > > > > if ENABLE_TEST_UTILS > > sbin_PROGRAMS += src/ibsendtrap src/mcm_rereg_test > > @@ -55,6 +56,8 @@ src_saquery_SOURCES = src/saquery.c > > src_ibsendtrap_SOURCES = src/ibsendtrap.c > > src_vendstat_SOURCES = src/vendstat.c > > src_mcm_rereg_test_SOURCES = src/mcm_rereg_test.c > > +src_iblinkinfo_SOURCES = src/iblinkinfo.c > > +src_iblinkinfo_LDADD = -libnetdisc > > I think here should be '-L$(top_builddir)/libibnetdisc -libnetdisc'. > Otherwise we are assuming pre-installed library (we could run > 'make install', but it fails due to 'make' error :)). > > Adding this too. Oops... Sorry, that got put into the "convert ibnetdiscover" patch. But I used $(top_srcdir) I can change it. See below: --- a/infiniband-diags/Makefile.am +++ b/infiniband-diags/Makefile.am @@ -41,7 +41,8 @@ LDADD = libcommon.a libcommon_a_SOURCES = src/ibdiag_common.c src_ibaddr_SOURCES = src/ibaddr.c -src_ibnetdiscover_SOURCES = src/ibnetdiscover.c src/grouping.c +src_ibnetdiscover_SOURCES = src/ibnetdiscover.c +src_ibnetdiscover_LDFLAGS = -L$(top_srcdir)/libibnetdisc -libnetdisc src_ibping_SOURCES = src/ibping.c src_ibportstate_SOURCES = src/ibportstate.c src_ibroute_SOURCES = src/ibroute.c @@ -57,7 +58,7 @@ src_ibsendtrap_SOURCES = src/ibsendtrap.c src_vendstat_SOURCES = src/vendstat.c src_mcm_rereg_test_SOURCES = src/mcm_rereg_test.c src_iblinkinfo_SOURCES = src/iblinkinfo.c -src_iblinkinfo_LDADD = -libnetdisc +src_iblinkinfo_LDFLAGS = -L$(top_srcdir)/libibnetdisc -libnetdisc man_MANS = man/ibaddr.8 man/ibcheckerrors.8 man/ibcheckerrs.8 \ man/ibchecknet.8 man/ibchecknode.8 man/ibcheckport.8 \ [snip] > > + > > + static char const str_opts[] = "S:D:n:C:P:t:sldgphuf:R"; > > + static const struct option long_opts[] = { > > + { "S", 1, 0, 'S'}, > > + { "D", 1, 0, 'D'}, > > + { "num-hops", 1, 0, 'n'}, > > + { "down-links-only", 0, 0, 'd'}, > > + { "line-mode", 0, 0, 'l'}, > > + { "ca-name", 1, 0, 'C'}, > > + { "ca-port", 1, 0, 'P'}, > > + { "timeout", 1, 0, 't'}, > > + { "show", 0, 0, 's'}, > > + { "print-port-guids", 0, 0, 'g'}, > > + { "print-additional", 0, 0, 'p'}, > > + { "help", 0, 0, 'h'}, > > + { "usage", 0, 0, 'u'}, > > + { "node-name-map", 1, 0, 1}, > > + { "debug", 0, 0, 2}, > > + { "compat", 0, 0, 3}, > > + { "from", 1, 0, 'f'}, > > + { "R", 0, 0, 'R'}, > > + { } > > + }; > > + > > + f = stdout; > > + > > + argv0 = argv[0]; > > + > > + while (1) { > > + int ch = getopt_long(argc, argv, str_opts, long_opts, NULL); > > Any reason to not use new ibdiag_process_opts() here? It should simplify > an options processing and unify the usage message. > > Of course this can be done as subsequent patch. > The only reason was that I started this patch before ibdiag_process_opts was created! I did use ibdiag_process_opts for ibqueryerrors. And yes I will be cleaning this up in subsequent patches. Ira _______________________________________________ general mailing list [email protected] http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
