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

[Michael Roth]

On 01/16/2012 11:23 AM, Luiz Capitulino wrote:
> On Mon, 16 Jan 2012 15:18:37 -0200
> Luiz Capitulino<address@hidden>  wrote:
>
> > On Mon, 16 Jan 2012 17:13:39 +0000
> > "Daniel P. Berrange"<address@hidden>  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<address@hidden>  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.

glib seems to give itself at least some liberty in confirming whether a 
function beyond the POSIX-confirmed ones are thread safe. 
glib/gthreadpool.c:169 calls g_get_current_time(), for instance, which 
calls gettimeofday(), which isn't on the list (but does happen to be 
thread-safe). This technically renders a substantial number of functions 
glib exposes in it's APIs unsafe, since a large number of those also use 
g_get_current_time()/gettimeofday() and don't do any thread 
synchronization. The situation seems to be even more lax on win32 
(memcpy, memmove, strcmp in their GIO reader thread, for instance), but 
I'm not sure what the story is there WRT to thread safety.

qemu as well, we use memcpy/memmove/memset/fread/printf/etc even though 
it has the same glib dependencies as qemu-ga, and I don't think it's 
realistic to consider removing them.

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?

> > > The most that GLib says is
> > >
> > >    "The GLib threading system used to be initialized with g_thread_init().
> > >     This is no longer necessary. Since version 2.32, the GLib threading
> > >     system is automatically initialized at the start of your program,
> > >     and all thread-creation functions and synchronization primitives
> > >     are available right away.
> > >
> > >     Note that it is not safe to assume that your program has no threads
> > >     even if you don't call g_thread_new() yourself. GLib and GIO can
> > >     and will create threads for their own purposes in some cases, such
> > >     as when using g_unix_signal_source_new() or when using GDBus. "
> > >
> > > The latter paragraph is rather fuzzy, which is probably intentional.
> > > So I think the only safe thing, in order to be future proof wrt later
> > > GLib releases, is to just assume you have threads at all times.
> >
> > Yeah, and we do use GIO in qemu-ga...
> >
> > Thanks Daniel.
> >
> > > Daniel