[libvirt] [PATCH] daemon: Run virStateCleanup conditionally

[Daniel P. Berrange]

On Tue, Dec 03, 2013 at 06:41:36AM -0700, Eric Blake wrote:
> On 12/03/2013 03:43 AM, Daniel P. Berrange wrote:
> > On Tue, Dec 03, 2013 at 11:39:15AM +0100, Michal Privoznik wrote:
> >> https://bugzilla.redhat.com/show_bug.cgi?id=1033061
> >>
> 
> >> +    driversInitialized = true;
> >> +
> >>  #ifdef HAVE_DBUS
> >>      /* Tie the non-priviledged libvirtd to the session/shutdown lifecycle */
> >>      if (!virNetServerIsPrivileged(srv)) {
> >> @@ -1546,7 +1550,8 @@ cleanup:
> >>  
> >>      daemonConfigFree(config);
> >>  
> >> -    virStateCleanup();
> >> +    if (driversInitialized)
> >> +        virStateCleanup();
> > 
> > Don't we technically need to use an int and atomic int APIs for these
> > changes ?
> 
> Not as far as I can tell.  Since only one thread is setting the
> variable, and we are not accessing the variable in a signal handler, it
> seems that mere 'volatile' is enough to ensure that the compiler won't
> optimize any out-of-order assignments.  I'm fine giving ACK to this
> patch as-is.

I wasn't thinking of ordering, but rather atomicity of updating
the value. eg what happens if thread 1 tries to read the value
concurrently with thread 2 setting it. Surely this is the case
atomic ops are intended for.

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