On Wed, Dec 20 2006, Jens Axboe wrote:
> On Wed, Dec 20 2006, Jens Axboe wrote:
> > On Tue, Dec 19 2006, Ming Zhang wrote:
> > > On Tue, 2006-12-19 at 17:22 +0100, Jens Axboe wrote:
> > > > On Tue, Dec 19 2006, Ming Zhang wrote:
> > > > > On Tue, 2006-12-19 at 15:24 +0100, Jens Axboe wrote:
> > > > > > On Tue, Dec 19 2006, Ming Zhang wrote:
> > > > > > > On Tue, 2006-12-19 at 15:20 +0100, Jens Axboe wrote:
> > > > > > > > On Tue, Dec 19 2006, Ming Zhang wrote:
> > > > > > > > > On Tue, 2006-12-19 at 08:17 +0100, Jens Axboe wrote:
> > > > > > > > > > On Mon, Dec 18 2006, Ming Zhang wrote:
> > > > > > > > > > > On Mon, 2006-12-18 at 10:54 -0500, Alan D. Brunelle wrote:
> > > > > > > > > > > > (Hopefully this works better than my last patch attempt
> > > > > > > > > > > > - Jens: the
> > > > > > > > > > > > other patches got put on the back burner for some
> > > > > > > > > > > > work-related stuff,
> > > > > > > > > > > > I'll get to those in the New Year.)
> > > > > > > > > > > >
> > > > > > > > > > > > Alan
> > > > > > > > > > > > plain text document attachment (bp-fix-empties)
> > > > > > > > > > > > Fix problem where empty input files cause premature
> > > > > > > > > > > > parsing of files.
> > > > > > > > > > > >
> > > > > > > > > > > > From: Alan D. Brunelle <[EMAIL PROTECTED]>
> > > > > > > > > > > >
> > > > > > > > > > > > Had a problem where CPU 0 opened a file with 0 traces,
> > > > > > > > > > > > and that caused the
> > > > > > > > > > > > run to end before processing other trace files.
> > > > > > > > > > > > ---
> > > > > > > > > > > >
> > > > > > > > > > > > blkparse.c | 4 +++-
> > > > > > > > > > > > 1 files changed, 3 insertions(+), 1 deletions(-)
> > > > > > > > > > > >
> > > > > > > > > > > > diff --git a/blkparse.c b/blkparse.c
> > > > > > > > > > > > index bded2f8..0d2ea12 100644
> > > > > > > > > > > > --- a/blkparse.c
> > > > > > > > > > > > +++ b/blkparse.c
> > > > > > > > > > > > @@ -2249,8 +2249,10 @@ static int setup_file(struct
> > > > > > > > > > > > per_dev_inf
> > > > > > > > > > > >
> > > > > > > > > > > > snprintf(pci->fname + len,
> > > > > > > > > > > > sizeof(pci->fname)-1-len,
> > > > > > > > > > > > "%s.blktrace.%d", pdi->name, pci->cpu);
> > > > > > > > > > > > - if (stat(pci->fname, &st) < 0 || !st.st_size)
> > > > > > > > > > > > + if (stat(pci->fname, &st) < 0)
> > > > > > > > > > > > return 0;
> > > > > > > > > > > > + if (!st.st_size)
> > > > > > > > > > > > + return 1;
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > not picky but what if i do blkparse from a fifo? will
> > > > > > > > > > > this work?
> > > > > > > > > >
> > > > > > > > > > It can't make things worse, as 0 is the failure condition.
> > > > > > > > > > Alans change
> > > > > > > > > > makes sense, we will just read 0 events from the descriptor.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > yes, not worse, just not fixed.
> > > > > > > > >
> > > > > > > > > i know this is rarely happen. but if somebody do blkparse -i
> > > > > > > > > xyz and xyz
> > > > > > > > > is a fifo file (feed data by netcat or any fancy stuff), then
> > > > > > > > > setup_file
> > > > > > > > > will return 1. then blkparse behave incorrectly. and
> > > > > > > > > "blkparse -h" does
> > > > > > > > > not explicitly ban the use of fifo.
> > > > > > > >
> > > > > > > > It's not something I've done, but would be nice to have fixed.
> > > > > > > > Care to
> > > > > > > > send a patch?
> > > > > > >
> > > > > > > my pleasure. which way u prefer? ban the use by revising the
> > > > > > > usage, or
> > > > > > > enable the use of fifo?
> > > > > >
> > > > > > Making fifo work would be nicest, if it's easily doable.
> > > > >
> > > > > sorry that i do not think it is easily doable once i pick up the code.
> > > > > the file names have to follow the patten like foo.blktrace.X. If a box
> > > > > has N cpu, people will not bother to create N fifo and follow that
> > > > > rule
> > > > > at all. So no worthy supporting it. Also add another note about
> > > > > instead
> > > > > of give full name, partial name should be supplied.
> > > >
> > > > Are you sure you read it correctly? blkparse already supports a piped
> > > > input, reading from stdin is how live traces are handled. You should not
> > > > need to add any new io handling code, just basically make sure it passes
> > > > the initial checks and ends up on do_stdin() (perhaps add a do_pipe()
> > > > that do_stdin() calls, after doing a dup() on STDIN_FILENO, reusing
> > > > do_pipe() for your fifo?).
> > > >
> > >
> > > i know the blkparse support a piped input.
> > >
> > > the problem is the naming. blkparse assume a file name to follow
> > > foo.blktrace.X pattern. so if we allow a fifo file to follow foo, or
> > > foo.blktrace pattern, this looks quite inconsistent.
> >
> > I guess cat pipe | blkparse - would solve the problem from that end,
> > then. Or add a piece of code stat'ing the given filename and check for
> > a pipe.
>
> Something like this, totally untested.
>
> diff --git a/blkparse.c b/blkparse.c
> index 0d2ea12..cc4ca94 100644
> --- a/blkparse.c
> +++ b/blkparse.c
> @@ -2428,6 +2428,18 @@ static int find_stopwatch_interval(char *string)
> return 0;
> }
>
> +static int is_pipe(const char *str)
> +{
> + struct stat st;
> +
> + if (!strcmp(str, "-"))
> + return 1;
> + if (!stat(str, &st) && S_ISFIFO(st.st_mode))
> + return 1;
> +
> + return 0;
> +}
> +
> #define S_OPTS "a:A:b:D:d:f:F:hi:o:Oqstw:vV"
> static char usage_str[] = "\n\n" \
> "-i <file> | --input=<file>\n" \
> @@ -2501,7 +2513,7 @@ int main(int argc, char *argv[])
> act_mask_tmp = i;
> break;
> case 'i':
> - if (!strcmp(optarg, "-") && !pipeline)
> + if (is_pipe(optarg) && !pipeline)
> pipeline = 1;
> else if (resize_devices(optarg) != 0)
> return 1;
> @@ -2559,7 +2571,7 @@ int main(int argc, char *argv[])
> }
>
> while (optind < argc) {
> - if (!strcmp(argv[optind], "-") && !pipeline)
> + if (is_pipe(argv[optind]) && !pipeline)
> pipeline = 1;
> else if (resize_devices(argv[optind]) != 0)
> return 1;
>
Irk, do_pipe() needs to know what the input is, obviously. Lemme add
that as well.
--
Jens Axboe
-
To unsubscribe from this list: send the line "unsubscribe linux-btrace" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
