[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] qemu: Ignore libvirt debug messages in qemu log



On Wed, Mar 30, 2011 at 13:46:12 -0600, Eric Blake wrote:
> On 03/30/2011 06:17 AM, Jiri Denemark wrote:
> >  
> > +    if (virAsprintf(&debug, ": %d: debug : ", vm->pid) < 0) {
> 
> That's rather hard-coded to our current output style; should we add
> comments here and to the place that outputs those lines to be careful to
> keep the two places in sync?

Yeah, it makes sense to add these comments.

> > -        got += ret;
> > +        got += bytes;
> >          buf[got] = '\0';
> > +
> > +        /* Filter out debug messages from intermediate libvirt process */
> > +        while ((eol = strchr(filter_next, '\n'))) {
> > +            char *p = strstr(filter_next, debug);
> 
> Suppose you have five normal lines before the first debug line.  Is it
> any more efficient to use use getline() to read a line at a time

Heh, the one which needs FILE *? No, thanks :-)

> Or even if you don't use getline(), can you temporarily set eol to NUL

Sure, that actually even results in a bit nicer code.

> > +            if (p && p < eol) {
> > +                memmove(filter_next, eol + 1, got - (eol - buf));
> 
> Conversely, supposed you have five debug lines followed by five normal
> lines.  Doing the memmove() on every debug line encountered results in
> moving 25 lines around, whereas if you rewrite the loop to only copy one
> line at a time and only if it passed the filter, then you only have to
> move 5 lines around.

And all this after we went through fork() and going to call exec() soon, I
don't think such optimization would make any difference. If this code had been
run on any libvirt API, it would have make sense to optimize the loop, but now
I don't think it's worth it. It looks like a premature optimization to me :-)

Jirka


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]