[libvirt] [PATCH 08/12] libxl: Use atomic ops for driver->nactive
Michal Privoznik
mprivozn at redhat.com
Mon Sep 2 11:08:07 UTC 2013
On 30.08.2013 23:46, Jim Fehlig wrote:
>
> Signed-off-by: Jim Fehlig <jfehlig at suse.com>
> ---
> src/libxl/libxl_conf.h | 2 +-
> src/libxl/libxl_driver.c | 10 ++++------
> 2 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
> index e3875ba..83bb6b9 100644
> --- a/src/libxl/libxl_conf.h
> +++ b/src/libxl/libxl_conf.h
> @@ -90,7 +90,7 @@ struct _libxlDriverPrivate {
> * then lockless thereafter */
> libxlDriverConfigPtr config;
>
> - size_t nactive;
> + unsigned int nactive;
>
> virStateInhibitCallback inhibitCallback;
> void *inhibitOpaque;
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index e604899..7615cdd 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -50,6 +50,7 @@
> #include "virstring.h"
> #include "virsysinfo.h"
> #include "viraccessapicheck.h"
> +#include "viratomic.h"
>
> #define VIR_FROM_THIS VIR_FROM_LIBXL
>
> @@ -265,8 +266,7 @@ libxlVmCleanup(libxlDriverPrivatePtr driver,
> virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason);
> }
>
> - driver->nactive--;
> - if (!driver->nactive && driver->inhibitCallback)
> + if (virAtomicIntDecAndTest(&driver->nactive) && driver->inhibitCallback)
> driver->inhibitCallback(false, driver->inhibitOpaque);
>
> if ((vm->def->ngraphics == 1) &&
> @@ -655,9 +655,8 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
> if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
> goto error;
>
> - if (!driver->nactive && driver->inhibitCallback)
> + if (virAtomicIntInc(&driver->nactive) && driver->inhibitCallback)
> driver->inhibitCallback(true, driver->inhibitOpaque);
Not exactly the same as previous code. Previously, the callback was
called iff nactive == 0; However, with your change the callback is
invoked each time the control gets to the 'if' (as virAtomicIntInc()
returns the *new* value after the increment). I think we want this to be:
if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback)
> - driver->nactive++;
>
> event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED,
> restore_fd < 0 ?
> @@ -727,9 +726,8 @@ libxlReconnectDomain(virDomainObjPtr vm,
> vm->def->id = d_info.domid;
> virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_UNKNOWN);
>
> - if (!driver->nactive && driver->inhibitCallback)
> + if (virAtomicIntInc(&driver->nactive) && driver->inhibitCallback)
Same applies here.
> driver->inhibitCallback(true, driver->inhibitOpaque);
> - driver->nactive++;
>
> /* Recreate domain death et. al. events */
> libxlCreateDomEvents(vm);
>
ACK if you fix the issue.
Michal
More information about the libvir-list
mailing list