Heitor Ricardo Alves de Siqueira <hal...@linux.vnet.ibm.com> writes: > Hi Gabriel, > > Thanks again for your comments! I've removed all whitespace errors and also > fixed the other issues. If there are any other issues with the code, please > let > me know. > > Regarding your question about the driver string 'ipr2': it does come from the > dump file itself (see IPR_DUMP_DRIVER_NAME in linux/drivers/scsi/ipr.h). I've > left this string in lower case for now. Do you think we should print it upper > case, like it's defined in the dump file?
Nah, I think it's good this way. The iprdumpfmt looks good to me now, but there is still one issue left in the build system that I missed in the previous iterations. Sorry for missing that, I should have caught it earlier. > -- >8 -- > > This commit implements a new iprdumpfmt command that takes an iprdump.XX file > as > input and strips out driver information. This includes driver version and > type, > driver trace in the correct order and SDT entries. It has a verbose option > that > prints additional information for the driver trace, such as timestamps and > SCSI > commands. > > Signed-off-by: Heitor Ricardo Alves de Siqueira <hal...@linux.vnet.ibm.com> > --- > Makefile.am | 5 +- > iprdumpfmt.8 | 33 +++ > iprdumpfmt.c | 665 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 702 insertions(+), 1 deletion(-) > create mode 100644 iprdumpfmt.8 > create mode 100644 iprdumpfmt.c > > diff --git a/Makefile.am b/Makefile.am > index 4636ecbaf8ce..96b0cd2a5c4c 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -10,7 +10,7 @@ > noinst_LTLIBRARIES = libipr.la > libipr_la_SOURCES = iprlib.c iprlib.h > > -sbin_PROGRAMS = iprconfig iprdump iprupdate iprinit iprdbg > +sbin_PROGRAMS = iprconfig iprdump iprupdate iprinit iprdbg iprdumpfmt By doing this, we're always building and installing the tool by default, which is not very nice, since iprdumpfmt is a very specific tool to debug iprdumps that won't be very useful in the field. Installing by default also messes up with our spec file and would force distro packagers to fix their build systems to explicitly delete it after the build. Not ok. My suggestion is that we make this tool disabled by default in configure.ac and allow users to build it only if necessary. There are two ways to do that. One is creating a flag --enable-iprdumpfmt in configure.ac, like it is done with iprsos script. The other is building the tool only if we build with Autotool's maintainer mode enabled. I think using the maintainer mode could even be more semantic in this case, but it would also be more confusing, making the build system even more complex. On the other hand, creating the new flag should be trivial, just copy-paste from the code already in configure.ac. As long as the default is disabled, we should be ok. Thanks, -- Gabriel Krisman Bertazi ------------------------------------------------------------------------------ _______________________________________________ Iprdd-devel mailing list Iprdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/iprdd-devel
