On 12/17/24 12:15, Daniel P. Berrangé wrote:
> This initial conversion of the drivers switches them over to use
> the virInhibitor APIs in local daemon only mode. Communication to
> logind is still handled by the virNetDaemon class logic.
>
> This mostly just replaces upto 3 fields in the driver state
> with a single new virInhibitor object, but otherwise should not
> change functionality besides replacing atomics with mutex protected
> APIs.
>
> The exception is the LXC driver which has been trying to inhibit
> shutdown shutdown but silently failing to, since nothing ever
> remembered to set the 'inhibitCallback' pointer in the driver
> state struct.
>
> Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> ---
> src/libxl/libxl_conf.h | 9 +++----
> src/libxl/libxl_domain.c | 6 ++---
> src/libxl/libxl_driver.c | 15 +++++++----
> src/lxc/lxc_conf.h | 9 +++----
> src/lxc/lxc_driver.c | 13 +++++++--
> src/lxc/lxc_process.c | 9 +++----
> src/network/bridge_driver.c | 20 +++++++-------
> src/network/bridge_driver_conf.h | 9 +++----
> src/qemu/qemu_conf.h | 9 +++----
> src/qemu/qemu_driver.c | 12 ++++++---
> src/qemu/qemu_process.c | 9 +++----
> src/secret/secret_driver.c | 46 +++++++++++++-------------------
> 12 files changed, 80 insertions(+), 86 deletions(-)
>
> diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
> index 7087b41079..0edcde079d 100644
> --- a/src/libxl/libxl_conf.h
> +++ b/src/libxl/libxl_conf.h
> @@ -35,6 +35,7 @@
> #include "virfirmware.h"
> #include "libxl_capabilities.h"
> #include "libxl_logger.h"
> +#include "virinhibitor.h"
>
> #define LIBXL_DRIVER_EXTERNAL_NAME "Xen"
> /*
> @@ -117,12 +118,8 @@ struct _libxlDriverPrivate {
> /* pid file FD, ensures two copies of the driver can't use the same root */
> int lockFD;
>
> - /* Atomic inc/dec only */
> - unsigned int nactive;
> -
> - /* Immutable pointers. Caller must provide locking */
> - virStateInhibitCallback inhibitCallback;
> - void *inhibitOpaque;
> + /* Immutable pointer, self-locking APIs */
> + virInhibitor *inhibitor;
>
> /* Immutable pointer, self-locking APIs */
> virDomainObjList *domains;
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index cad6c9ce42..a049cdb30f 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -873,8 +873,7 @@ libxlDomainCleanup(libxlDriverPrivate *driver,
> priv->deathW = NULL;
> }
>
> - if (g_atomic_int_dec_and_test(&driver->nactive) && driver->inhibitCallback)
> - driver->inhibitCallback(false, driver->inhibitOpaque);
> + virInhibitorRelease(driver->inhibitor);
>
> /* Release auto-allocated graphics ports */
> for (i = 0; i < vm->def->ngraphics; i++) {
> @@ -1421,8 +1420,7 @@ libxlDomainStart(libxlDriverPrivate *driver,
> return -1;
> }
>
> - if (g_atomic_int_add(&driver->nactive, 1) == 0 && driver->inhibitCallback)
> - driver->inhibitCallback(true, driver->inhibitOpaque);
> + virInhibitorHold(driver->inhibitor);
>
> event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED,
> restore_fd < 0 ?
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index e72553603d..eb293172f7 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -437,8 +437,7 @@ libxlReconnectDomain(virDomainObj *vm,
> virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,
> VIR_DOMAIN_RUNNING_UNKNOWN);
>
> - if (g_atomic_int_add(&driver->nactive, 1) == 0 && driver->inhibitCallback)
> - driver->inhibitCallback(true, driver->inhibitOpaque);
> + virInhibitorHold(driver->inhibitor);
>
> /* Enable domain death events */
> libxl_evenable_domain_death(cfg->ctx, vm->def->id, 0, &priv->deathW);
> @@ -514,6 +513,7 @@ libxlStateCleanup(void)
>
> virObjectUnref(libxl_driver->domainEventState);
> virSysinfoDefFree(libxl_driver->hostsysinfo);
> + virInhibitorFree(libxl_driver->inhibitor);
>
> if (libxl_driver->lockFD != -1)
> virPidFileRelease(libxl_driver->config->stateDir, "driver", libxl_driver->lockFD);
> @@ -675,9 +675,6 @@ libxlStateInitialize(bool privileged,
> return VIR_DRV_STATE_INIT_ERROR;
> }
>
> - libxl_driver->inhibitCallback = callback;
> - libxl_driver->inhibitOpaque = opaque;
> -
> /* Allocate bitmap for vnc port reservation */
> if (!(libxl_driver->reservedGraphicsPorts =
> virPortAllocatorRangeNew(_("VNC"),
> @@ -709,6 +706,14 @@ libxlStateInitialize(bool privileged,
> if (libxlDriverConfigLoadFile(cfg, driverConf) < 0)
> goto error;
>
> + libxl_driver->inhibitor = virInhibitorNew(
> + VIR_INHIBITOR_WHAT_NONE,
> + _("Libvirt Xen"),
> + _("Xen virtual machines are running"),
> + VIR_INHIBITOR_MODE_DELAY,
> + callback,
> + opaque);
A bit funky formatting here and in the rest of the patch. But I can live
with that.
Michal
