Re: [Qemu-devel] [PATCH 2/2] qemu-ga: Add the guest-suspend command

["Daniel P. Berrange" <berrange@redhat.com>]

On Mon, Jan 16, 2012 at 02:02:08PM -0600, Michael Roth wrote:
> On 01/16/2012 11:23 AM, Luiz Capitulino wrote:
> >On Mon, 16 Jan 2012 15:18:37 -0200
> >Luiz Capitulino<lcapitulino@redhat.com>  wrote:
> >
> >>On Mon, 16 Jan 2012 17:13:39 +0000
> >>"Daniel P. Berrange"<berrange@redhat.com>  wrote:
> >>
> >>>On Mon, Jan 16, 2012 at 03:08:53PM -0200, Luiz Capitulino wrote:
> >>>>On Fri, 13 Jan 2012 14:48:04 -0700
> >>>>Eric Blake<eblake@redhat.com>  wrote:
> >>>>
> >>>>>>+
> >>>>>>+        pid = fork();
> >>>>>>+        if (!pid) {
> >>>>>>+            char buf[32];
> >>>>>>+            FILE *sysfile;
> >>>>>>+            const char *arg;
> >>>>>>+            const char *pmutils_bin = "pm-is-supported";
> >>>>>>+
> >>>>>>+            if (strcmp(mode, "hibernate") == 0) {
> >>>>>
> >>>>>Strangely enough, POSIX doesn't include strcmp() in its list of
> >>>>>async-signal-safe functions (which is what you should be restricting
> >>>>>yourself to, if qemu-ga is multi-threaded), but in practice, I think
> >>>>>that is a bug of omission in POSIX, and not something you have to change
> >>>>>in your code.
> >>>>
> >>>>memset() ins't either... sigaction() either, which begins to get
> >>>>annoying.
> >>>>
> >>>>For those familiar with glib: isn't it possible to confirm it's using
> >>>>threads and/or acquire a global mutex or something?
> >
> >Misread, sigaction() is there. The ones that aren't are strcmp(), strstr()
> >and memset(). Interestingly, they are all "string functions".
> >
> 
> There seem to be things beyond that list required to be implemented
> as thread/signal safe:
> 
> http://www.unix.org/whitepapers/reentrant.html
> 
> fread()/fwrite()/f* for instance, more at `man flockfile`:
> 
>        The  stdio  functions are thread-safe.
>        This is achieved by assigning to  each
>        FILE  object  a  lockcount and (if the
>        lockcount  is   nonzero)   an   owning
>        thread.   For each library call, these
>        functions wait until the  FILE  object
>        is  no  longer  locked  by a different
>        thread, then lock it, do the requested
>        I/O, and unlock the object again.

You need to be careful with terminology here.

   Threadsafe != async signal safe

STDIO is one of the major areas of code that is definitely not
async signal safe. Consider Thread A doing something like
fwrite(stderr, "Foo\n"), while another thread forks, and then
its child also does an fwrite(stderr, "Foo\n"). Given that
every stdio function will lock/unlock a mutex, you easily get
this sequence of events:

1.     Thread A: lock(stderr)
2.     Thread A: write(stderr, "foo\n");
3.     Thread B: fork() -> Process B1
4.     Thread A: unlock(stderr)
5.   Process B1: lock(stderr)

When the child process is started at step 3, the FILE *stderr
object will be locked by thread A.  When Thread A does the
unlock in step 4, it has no effect on Process B1. So process
B1 hangs forever in step 5.

> In practice, are these functions really a problem for multi-threaded
> applications (beyond concurrent access to shared storage)? Maybe it
> would be sufficient to just check the glibc sources?

In libvirt we have seen the hang scenarios I describe in the real world.
Causes I rememeber were use of malloc (via asprintf()), or use of stdio
FILE * functions, and use of syslog. The libvirt code still isn't 100%
in compliance with avoiding async signal safe functions, but we have
cleaned up many problems in this area.

Regards,
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 :|