[libvirt] [PATCH 5/6] Add an an internal API for emergency dump of debug buffer

[Daniel P. Berrange]

On Mon, Mar 07, 2011 at 09:18:46PM +0800, Daniel Veillard wrote:
> On Mon, Mar 07, 2011 at 10:18:57AM +0000, Daniel P. Berrange wrote:
> > On Mon, Mar 07, 2011 at 03:06:18PM +0800, Daniel Veillard wrote:
> > > On Fri, Mar 04, 2011 at 08:53:55AM -0700, Eric Blake wrote:
> > > > Is virLogLock async-signal-safe?
> > > 
> > >   I could not find, I'm afraid it's implementation dependant. I would
> > > expect the lock to be done in user space using an atomic test and set
> > > op and hence being safe at least on i386 and x86_64.
> > >   The problem is that if we drop the lock we can crash if used on USR2
> > > while another thread is legitimately running.
> > 
> > Even if the impl was robust there is an architectural problem
> > with mixing of mutexes and signals. If a thread is running
> > virLogMessage(), it will be holding the log mutex. If a signal
> > is processed, and it calls into this new log code, it will
> > likely deadlock.
> 
>   right this is the only thing that really makes the USR2 debug
> dangerous IMHO, and we should try to fix it.
> 
> > Is there a way we can somehow make the global log buffer into
> > something that can be lockless for readers. ie although our
> > main log funcs would still need to use the mutex in general
> > when writing to the buffer, we could read the log buffer
> > without any locks.
> 
>   the problem is that I really want to empty the buffer as a
> result of emitting the logs, i.e. the reader will emit only once
> the content at most.

While emptying the buffer is nice, I don't actually think it is
too important.  Upon SEGV,BUS,FPE etc we're going to abort the
entire process anyway, so clearing the buffer isn't neccessary.
With SIGUSR2, if the user triggers it multiple times, they will
likely have left enough time between invocation that the original
data has been already overwritten. So clearing the buffer is a
minor non-critical optmization for that.

> > It would probably suffice to make sure we use atomic integer
> > arithmetic on virLogLen, virLogStart and virLogEnd, and update
> > them in a specific order to avoid a reader seeing garbage.
> 
> honnestly I would not be chagrined by the probability of emitting
> a little bit of garbage when flushing the logs or one or two items.
> I think we can work lockless there, by registering the initial values
> of virLogStart and virLogEnd when entering the routine, emit whever is
> in those boundaries with the tiny risk that some of it may be
> overwritten (at the beginning hence the less interesting stuff) and
> on exit set back virLogStart = virLogEnd and virLogLen = 0, possibly
> loosing whetver might have been emitted between the beginning of the
> save and the exit from the routine.

Agreed, the only thing we need to avoid is crashing. If we emit a little
bit of garbage data, that's not too bad and outweighed by the benefit of
useful data in most scenarios.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|