[libvirt] [PATCH] daemon: Run virStateCleanup conditionally

[Daniel P. Berrange]

On Tue, Dec 03, 2013 at 11:39:15AM +0100, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1033061
> 
> Currently, initialization of drivers is done in a separate thread. This
> is done for several reasons: a driver that is initialized may require
> running event loop, it may take ages to initialize driver (e.g. due to
> autostarting domains). While the thread is spawn and run, the main()
> continues its execution. However, if something goes bad, or the event
> loop is just exited (e.g. due to a --timeout or SIGINT) we try to
> cleanup all the drivers. So we have two threads running Initialize() and
> Cleanup() concurrently. This may result in accessing stale pointers -
> e.g. netcf driver will free() itself in stateCleanup callback, while the
> init thread may come, open a dummy connection in order to autostart some
> domains and voilà: do_open() iterates over interface drivers and
> accesses stale netcf driver.
> 
> The fix consists in not running stateCleanup if the init thread is still
> running.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  daemon/libvirtd.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> index aef1546..49c42ad 100644
> --- a/daemon/libvirtd.c
> +++ b/daemon/libvirtd.c
> @@ -108,6 +108,8 @@ virNetServerProgramPtr remoteProgram = NULL;
>  virNetServerProgramPtr qemuProgram = NULL;
>  virNetServerProgramPtr lxcProgram = NULL;
>  
> +volatile bool driversInitialized = false;
> +
>  enum {
>      VIR_DAEMON_ERR_NONE = 0,
>      VIR_DAEMON_ERR_PIDFILE,
> @@ -912,6 +914,8 @@ static void daemonRunStateInit(void *opaque)
>          goto cleanup;
>      }
>  
> +    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 ?

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