src: convert drivers over to new virInhibitor APIs

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.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This commit is contained in:
Daniel P. Berrangé 2024-12-16 16:28:48 +00:00
parent d2e5aa4f4e
commit 48f0b6dfa1
12 changed files with 80 additions and 86 deletions

View File

@ -35,6 +35,7 @@
#include "virfirmware.h" #include "virfirmware.h"
#include "libxl_capabilities.h" #include "libxl_capabilities.h"
#include "libxl_logger.h" #include "libxl_logger.h"
#include "virinhibitor.h"
#define LIBXL_DRIVER_EXTERNAL_NAME "Xen" #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 */ /* pid file FD, ensures two copies of the driver can't use the same root */
int lockFD; int lockFD;
/* Atomic inc/dec only */ /* Immutable pointer, self-locking APIs */
unsigned int nactive; virInhibitor *inhibitor;
/* Immutable pointers. Caller must provide locking */
virStateInhibitCallback inhibitCallback;
void *inhibitOpaque;
/* Immutable pointer, self-locking APIs */ /* Immutable pointer, self-locking APIs */
virDomainObjList *domains; virDomainObjList *domains;

View File

@ -873,8 +873,7 @@ libxlDomainCleanup(libxlDriverPrivate *driver,
priv->deathW = NULL; priv->deathW = NULL;
} }
if (g_atomic_int_dec_and_test(&driver->nactive) && driver->inhibitCallback) virInhibitorRelease(driver->inhibitor);
driver->inhibitCallback(false, driver->inhibitOpaque);
/* Release auto-allocated graphics ports */ /* Release auto-allocated graphics ports */
for (i = 0; i < vm->def->ngraphics; i++) { for (i = 0; i < vm->def->ngraphics; i++) {
@ -1421,8 +1420,7 @@ libxlDomainStart(libxlDriverPrivate *driver,
return -1; return -1;
} }
if (g_atomic_int_add(&driver->nactive, 1) == 0 && driver->inhibitCallback) virInhibitorHold(driver->inhibitor);
driver->inhibitCallback(true, driver->inhibitOpaque);
event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED, event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED,
restore_fd < 0 ? restore_fd < 0 ?

View File

@ -437,8 +437,7 @@ libxlReconnectDomain(virDomainObj *vm,
virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,
VIR_DOMAIN_RUNNING_UNKNOWN); VIR_DOMAIN_RUNNING_UNKNOWN);
if (g_atomic_int_add(&driver->nactive, 1) == 0 && driver->inhibitCallback) virInhibitorHold(driver->inhibitor);
driver->inhibitCallback(true, driver->inhibitOpaque);
/* Enable domain death events */ /* Enable domain death events */
libxl_evenable_domain_death(cfg->ctx, vm->def->id, 0, &priv->deathW); libxl_evenable_domain_death(cfg->ctx, vm->def->id, 0, &priv->deathW);
@ -514,6 +513,7 @@ libxlStateCleanup(void)
virObjectUnref(libxl_driver->domainEventState); virObjectUnref(libxl_driver->domainEventState);
virSysinfoDefFree(libxl_driver->hostsysinfo); virSysinfoDefFree(libxl_driver->hostsysinfo);
virInhibitorFree(libxl_driver->inhibitor);
if (libxl_driver->lockFD != -1) if (libxl_driver->lockFD != -1)
virPidFileRelease(libxl_driver->config->stateDir, "driver", libxl_driver->lockFD); virPidFileRelease(libxl_driver->config->stateDir, "driver", libxl_driver->lockFD);
@ -675,9 +675,6 @@ libxlStateInitialize(bool privileged,
return VIR_DRV_STATE_INIT_ERROR; return VIR_DRV_STATE_INIT_ERROR;
} }
libxl_driver->inhibitCallback = callback;
libxl_driver->inhibitOpaque = opaque;
/* Allocate bitmap for vnc port reservation */ /* Allocate bitmap for vnc port reservation */
if (!(libxl_driver->reservedGraphicsPorts = if (!(libxl_driver->reservedGraphicsPorts =
virPortAllocatorRangeNew(_("VNC"), virPortAllocatorRangeNew(_("VNC"),
@ -709,6 +706,14 @@ libxlStateInitialize(bool privileged,
if (libxlDriverConfigLoadFile(cfg, driverConf) < 0) if (libxlDriverConfigLoadFile(cfg, driverConf) < 0)
goto error; goto error;
libxl_driver->inhibitor = virInhibitorNew(
VIR_INHIBITOR_WHAT_NONE,
_("Libvirt Xen"),
_("Xen virtual machines are running"),
VIR_INHIBITOR_MODE_DELAY,
callback,
opaque);
/* Register the callbacks providing access to libvirt's event loop */ /* Register the callbacks providing access to libvirt's event loop */
libxl_osevent_register_hooks(cfg->ctx, &libxl_osevent_callbacks, cfg->ctx); libxl_osevent_register_hooks(cfg->ctx, &libxl_osevent_callbacks, cfg->ctx);

View File

@ -30,6 +30,7 @@
#include "virsysinfo.h" #include "virsysinfo.h"
#include "virclosecallbacks.h" #include "virclosecallbacks.h"
#include "virhostdev.h" #include "virhostdev.h"
#include "virinhibitor.h"
#define LXC_DRIVER_NAME "LXC" #define LXC_DRIVER_NAME "LXC"
@ -75,12 +76,8 @@ struct _virLXCDriver {
/* Immutable pointer, lockless APIs */ /* Immutable pointer, lockless APIs */
virSysinfoDef *hostsysinfo; virSysinfoDef *hostsysinfo;
/* Atomic inc/dec only */ /* Immutable pointer, self-locking APIs */
unsigned int nactive; virInhibitor *inhibitor;
/* Immutable pointers. Caller must provide locking */
virStateInhibitCallback inhibitCallback;
void *inhibitOpaque;
/* Immutable pointer, self-locking APIs */ /* Immutable pointer, self-locking APIs */
virDomainObjList *domains; virDomainObjList *domains;

View File

@ -1398,8 +1398,8 @@ static virDrvStateInitResult
lxcStateInitialize(bool privileged, lxcStateInitialize(bool privileged,
const char *root, const char *root,
bool monolithic G_GNUC_UNUSED, bool monolithic G_GNUC_UNUSED,
virStateInhibitCallback callback G_GNUC_UNUSED, virStateInhibitCallback callback,
void *opaque G_GNUC_UNUSED) void *opaque)
{ {
virLXCDriverConfig *cfg = NULL; virLXCDriverConfig *cfg = NULL;
bool autostart = true; bool autostart = true;
@ -1451,6 +1451,14 @@ lxcStateInitialize(bool privileged,
if (virLXCLoadDriverConfig(cfg, SYSCONFDIR "/libvirt/lxc.conf") < 0) if (virLXCLoadDriverConfig(cfg, SYSCONFDIR "/libvirt/lxc.conf") < 0)
goto cleanup; goto cleanup;
lxc_driver->inhibitor = virInhibitorNew(
VIR_INHIBITOR_WHAT_NONE,
_("Libvirt LXC"),
_("LXC containers are running"),
VIR_INHIBITOR_MODE_DELAY,
callback,
opaque);
if (!(lxc_driver->securityManager = lxcSecurityInit(cfg))) if (!(lxc_driver->securityManager = lxcSecurityInit(cfg)))
goto cleanup; goto cleanup;
@ -1555,6 +1563,7 @@ static int lxcStateCleanup(void)
virObjectUnref(lxc_driver->caps); virObjectUnref(lxc_driver->caps);
virObjectUnref(lxc_driver->securityManager); virObjectUnref(lxc_driver->securityManager);
virObjectUnref(lxc_driver->xmlopt); virObjectUnref(lxc_driver->xmlopt);
virInhibitorFree(lxc_driver->inhibitor);
if (lxc_driver->lockFD != -1) if (lxc_driver->lockFD != -1)
virPidFileRelease(lxc_driver->config->stateDir, "driver", lxc_driver->lockFD); virPidFileRelease(lxc_driver->config->stateDir, "driver", lxc_driver->lockFD);

View File

@ -203,8 +203,7 @@ static void virLXCProcessCleanup(virLXCDriver *driver,
vm->pid = 0; vm->pid = 0;
vm->def->id = -1; vm->def->id = -1;
if (g_atomic_int_dec_and_test(&driver->nactive) && driver->inhibitCallback) virInhibitorRelease(driver->inhibitor);
driver->inhibitCallback(false, driver->inhibitOpaque);
virLXCDomainReAttachHostDevices(driver, vm->def); virLXCDomainReAttachHostDevices(driver, vm->def);
@ -1466,8 +1465,7 @@ int virLXCProcessStart(virLXCDriver * driver,
if (virCommandHandshakeNotify(cmd) < 0) if (virCommandHandshakeNotify(cmd) < 0)
goto cleanup; goto cleanup;
if (g_atomic_int_add(&driver->nactive, 1) == 0 && driver->inhibitCallback) virInhibitorHold(driver->inhibitor);
driver->inhibitCallback(true, driver->inhibitOpaque);
/* The first synchronization point is when the controller creates CGroups. */ /* The first synchronization point is when the controller creates CGroups. */
if (lxcContainerWaitForContinue(handshakefds[0]) < 0) { if (lxcContainerWaitForContinue(handshakefds[0]) < 0) {
@ -1665,8 +1663,7 @@ virLXCProcessReconnectDomain(virDomainObj *vm,
virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,
VIR_DOMAIN_RUNNING_UNKNOWN); VIR_DOMAIN_RUNNING_UNKNOWN);
if (g_atomic_int_add(&driver->nactive, 1) == 0 && driver->inhibitCallback) virInhibitorHold(driver->inhibitor);
driver->inhibitCallback(true, driver->inhibitOpaque);
if (!(priv->monitor = virLXCProcessConnectMonitor(driver, vm))) if (!(priv->monitor = virLXCProcessConnectMonitor(driver, vm)))
goto error; goto error;

View File

@ -504,8 +504,7 @@ networkUpdateState(virNetworkObj *obj,
if (virNetworkObjIsActive(obj)) { if (virNetworkObjIsActive(obj)) {
virNetworkObjPortForEach(obj, networkUpdatePort, obj); virNetworkObjPortForEach(obj, networkUpdatePort, obj);
if (g_atomic_int_add(&driver->nactive, 1) == 0 && driver->inhibitCallback) virInhibitorHold(driver->inhibitor);
driver->inhibitCallback(true, driver->inhibitOpaque);
} }
/* Try and read dnsmasq pids of both active and inactive networks, just in /* Try and read dnsmasq pids of both active and inactive networks, just in
@ -644,9 +643,6 @@ networkStateInitialize(bool privileged,
goto error; goto error;
} }
network_driver->inhibitCallback = callback;
network_driver->inhibitOpaque = opaque;
network_driver->privileged = privileged; network_driver->privileged = privileged;
if (!(network_driver->xmlopt = networkDnsmasqCreateXMLConf())) if (!(network_driver->xmlopt = networkDnsmasqCreateXMLConf()))
@ -655,6 +651,14 @@ networkStateInitialize(bool privileged,
if (!(network_driver->config = cfg = virNetworkDriverConfigNew(privileged))) if (!(network_driver->config = cfg = virNetworkDriverConfigNew(privileged)))
goto error; goto error;
network_driver->inhibitor = virInhibitorNew(
VIR_INHIBITOR_WHAT_NONE,
_("Libvirt Network"),
_("Virtual networks are active"),
VIR_INHIBITOR_MODE_DELAY,
callback,
opaque);
if ((network_driver->lockFD = if ((network_driver->lockFD =
virPidFileAcquire(cfg->stateDir, "driver", getpid())) < 0) virPidFileAcquire(cfg->stateDir, "driver", getpid())) < 0)
goto error; goto error;
@ -2432,8 +2436,7 @@ networkStartNetwork(virNetworkDriverState *driver,
obj, network_driver->xmlopt) < 0) obj, network_driver->xmlopt) < 0)
goto cleanup; goto cleanup;
if (g_atomic_int_add(&driver->nactive, 1) == 0 && driver->inhibitCallback) virInhibitorHold(driver->inhibitor);
driver->inhibitCallback(true, driver->inhibitOpaque);
virNetworkObjSetActive(obj, true); virNetworkObjSetActive(obj, true);
VIR_INFO("Network '%s' started up", def->name); VIR_INFO("Network '%s' started up", def->name);
@ -2509,8 +2512,7 @@ networkShutdownNetwork(virNetworkDriverState *driver,
virNetworkObjSetActive(obj, false); virNetworkObjSetActive(obj, false);
if (g_atomic_int_dec_and_test(&driver->nactive) && driver->inhibitCallback) virInhibitorRelease(driver->inhibitor);
driver->inhibitCallback(false, driver->inhibitOpaque);
virNetworkObjUnsetDefTransient(obj); virNetworkObjUnsetDefTransient(obj);
return ret; return ret;

View File

@ -27,6 +27,7 @@
#include "virnetworkobj.h" #include "virnetworkobj.h"
#include "object_event.h" #include "object_event.h"
#include "virfirewall.h" #include "virfirewall.h"
#include "virinhibitor.h"
typedef struct _virNetworkDriverConfig virNetworkDriverConfig; typedef struct _virNetworkDriverConfig virNetworkDriverConfig;
struct _virNetworkDriverConfig { struct _virNetworkDriverConfig {
@ -49,12 +50,8 @@ typedef struct _virNetworkDriverState virNetworkDriverState;
struct _virNetworkDriverState { struct _virNetworkDriverState {
virMutex lock; virMutex lock;
/* Atomic inc/dec only */ /* Immutable pointer, self-locking APIs */
unsigned int nactive; virInhibitor *inhibitor;
/* Immutable pointers. Caller must provide locking */
virStateInhibitCallback inhibitCallback;
void *inhibitOpaque;
/* Read-only */ /* Read-only */
bool privileged; bool privileged;

View File

@ -42,6 +42,7 @@
#include "virfile.h" #include "virfile.h"
#include "virfilecache.h" #include "virfilecache.h"
#include "virfirmware.h" #include "virfirmware.h"
#include "virinhibitor.h"
#define QEMU_DRIVER_NAME "QEMU" #define QEMU_DRIVER_NAME "QEMU"
@ -257,16 +258,12 @@ struct _virQEMUDriver {
/* Atomic increment only */ /* Atomic increment only */
int lastvmid; int lastvmid;
/* Atomic inc/dec only */
unsigned int nactive;
/* Immutable values */ /* Immutable values */
bool privileged; bool privileged;
char *embeddedRoot; char *embeddedRoot;
/* Immutable pointers. Caller must provide locking */ /* Immutable pointer, self-locking APIs */
virStateInhibitCallback inhibitCallback; virInhibitor *inhibitor;
void *inhibitOpaque;
/* Immutable pointer, self-locking APIs */ /* Immutable pointer, self-locking APIs */
virDomainObjList *domains; virDomainObjList *domains;

View File

@ -573,9 +573,6 @@ qemuStateInitialize(bool privileged,
return VIR_DRV_STATE_INIT_ERROR; return VIR_DRV_STATE_INIT_ERROR;
} }
qemu_driver->inhibitCallback = callback;
qemu_driver->inhibitOpaque = opaque;
qemu_driver->privileged = privileged; qemu_driver->privileged = privileged;
qemu_driver->hostarch = virArchFromHost(); qemu_driver->hostarch = virArchFromHost();
if (root != NULL) if (root != NULL)
@ -675,6 +672,14 @@ qemuStateInitialize(bool privileged,
goto error; goto error;
} }
qemu_driver->inhibitor = virInhibitorNew(
VIR_INHIBITOR_WHAT_NONE,
_("Libvirt QEMU"),
_("QEMU/KVM virtual machines are running"),
VIR_INHIBITOR_MODE_DELAY,
callback,
opaque);
if ((qemu_driver->lockFD = if ((qemu_driver->lockFD =
virPidFileAcquire(cfg->stateDir, "driver", getpid())) < 0) virPidFileAcquire(cfg->stateDir, "driver", getpid())) < 0)
goto error; goto error;
@ -1065,6 +1070,7 @@ qemuStateCleanup(void)
ebtablesContextFree(qemu_driver->ebtables); ebtablesContextFree(qemu_driver->ebtables);
virObjectUnref(qemu_driver->domains); virObjectUnref(qemu_driver->domains);
virObjectUnref(qemu_driver->nbdkitCapsCache); virObjectUnref(qemu_driver->nbdkitCapsCache);
virInhibitorFree(qemu_driver->inhibitor);
if (qemu_driver->lockFD != -1) if (qemu_driver->lockFD != -1)
virPidFileRelease(qemu_driver->config->stateDir, "driver", qemu_driver->lockFD); virPidFileRelease(qemu_driver->config->stateDir, "driver", qemu_driver->lockFD);

View File

@ -5809,8 +5809,7 @@ qemuProcessInit(virQEMUDriver *driver,
qemuDomainSetFakeReboot(vm, false); qemuDomainSetFakeReboot(vm, false);
virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_STARTING_UP); virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_STARTING_UP);
if (g_atomic_int_add(&driver->nactive, 1) == 0 && driver->inhibitCallback) virInhibitorHold(driver->inhibitor);
driver->inhibitCallback(true, driver->inhibitOpaque);
/* Run an early hook to set-up missing devices */ /* Run an early hook to set-up missing devices */
if (qemuProcessStartHook(driver, vm, if (qemuProcessStartHook(driver, vm,
@ -8823,8 +8822,7 @@ void qemuProcessStop(virQEMUDriver *driver,
if (priv->eventThread) if (priv->eventThread)
g_object_unref(g_steal_pointer(&priv->eventThread)); g_object_unref(g_steal_pointer(&priv->eventThread));
if (g_atomic_int_dec_and_test(&driver->nactive) && driver->inhibitCallback) virInhibitorRelease(driver->inhibitor);
driver->inhibitCallback(false, driver->inhibitOpaque);
/* Clear network bandwidth */ /* Clear network bandwidth */
virDomainClearNetBandwidth(vm->def); virDomainClearNetBandwidth(vm->def);
@ -9582,8 +9580,7 @@ qemuProcessReconnect(void *opaque)
goto error; goto error;
} }
if (g_atomic_int_add(&driver->nactive, 1) == 0 && driver->inhibitCallback) virInhibitorHold(driver->inhibitor);
driver->inhibitCallback(true, driver->inhibitOpaque);
cleanup: cleanup:
if (jobStarted) if (jobStarted)

View File

@ -41,6 +41,7 @@
#include "viraccessapicheck.h" #include "viraccessapicheck.h"
#include "secret_event.h" #include "secret_event.h"
#include "virutil.h" #include "virutil.h"
#include "virinhibitor.h"
#define VIR_FROM_THIS VIR_FROM_SECRET #define VIR_FROM_THIS VIR_FROM_SECRET
@ -67,9 +68,8 @@ struct _virSecretDriverState {
/* Immutable pointer, self-locking APIs */ /* Immutable pointer, self-locking APIs */
virObjectEventState *secretEventState; virObjectEventState *secretEventState;
/* Immutable pointers. Caller must provide locking */ /* Immutable pointer, self-locking APIs */
virStateInhibitCallback inhibitCallback; virInhibitor *inhibitor;
void *inhibitOpaque;
}; };
static virSecretDriverState *driver; static virSecretDriverState *driver;
@ -90,23 +90,6 @@ secretObjFromSecret(virSecretPtr secret)
} }
static bool
secretNumOfEphemeralSecretsHelper(virConnectPtr conn G_GNUC_UNUSED,
virSecretDef *def)
{
return def->isephemeral;
}
static int
secretNumOfEphemeralSecrets(void)
{
return virSecretObjListNumOfSecrets(driver->secrets,
secretNumOfEphemeralSecretsHelper,
NULL);
}
/* Driver functions */ /* Driver functions */
static int static int
@ -271,6 +254,10 @@ secretDefineXML(virConnectPtr conn,
objDef->uuid, objDef->uuid,
objDef->usage_type, objDef->usage_type,
objDef->usage_id); objDef->usage_id);
if (objDef->isephemeral)
virInhibitorHold(driver->inhibitor);
goto cleanup; goto cleanup;
restore_backup: restore_backup:
@ -288,8 +275,6 @@ secretDefineXML(virConnectPtr conn,
virSecretDefFree(def); virSecretDefFree(def);
virSecretObjEndAPI(&obj); virSecretObjEndAPI(&obj);
if (secretNumOfEphemeralSecrets() > 0)
driver->inhibitCallback(true, driver->inhibitOpaque);
virObjectEventStateQueue(driver->secretEventState, event); virObjectEventStateQueue(driver->secretEventState, event);
@ -440,6 +425,9 @@ secretUndefine(virSecretPtr secret)
VIR_SECRET_EVENT_UNDEFINED, VIR_SECRET_EVENT_UNDEFINED,
0); 0);
if (def->isephemeral)
virInhibitorRelease(driver->inhibitor);
virSecretObjDeleteData(obj); virSecretObjDeleteData(obj);
virSecretObjListRemove(driver->secrets, obj); virSecretObjListRemove(driver->secrets, obj);
@ -450,9 +438,6 @@ secretUndefine(virSecretPtr secret)
cleanup: cleanup:
virSecretObjEndAPI(&obj); virSecretObjEndAPI(&obj);
if (secretNumOfEphemeralSecrets() == 0)
driver->inhibitCallback(false, driver->inhibitOpaque);
virObjectEventStateQueue(driver->secretEventState, event); virObjectEventStateQueue(driver->secretEventState, event);
return ret; return ret;
@ -469,6 +454,7 @@ secretStateCleanupLocked(void)
VIR_FREE(driver->configDir); VIR_FREE(driver->configDir);
virObjectUnref(driver->secretEventState); virObjectUnref(driver->secretEventState);
virInhibitorFree(driver->inhibitor);
if (driver->lockFD != -1) if (driver->lockFD != -1)
virPidFileRelease(driver->stateDir, "driver", driver->lockFD); virPidFileRelease(driver->stateDir, "driver", driver->lockFD);
@ -502,8 +488,6 @@ secretStateInitialize(bool privileged,
driver->lockFD = -1; driver->lockFD = -1;
driver->secretEventState = virObjectEventStateNew(); driver->secretEventState = virObjectEventStateNew();
driver->privileged = privileged; driver->privileged = privileged;
driver->inhibitCallback = callback;
driver->inhibitOpaque = opaque;
if (root) { if (root) {
driver->embeddedRoot = g_strdup(root); driver->embeddedRoot = g_strdup(root);
@ -535,6 +519,14 @@ secretStateInitialize(bool privileged,
goto error; goto error;
} }
driver->inhibitor = virInhibitorNew(
VIR_INHIBITOR_WHAT_NONE,
_("Libvirt Secret"),
_("Ephemeral secrets are loaded"),
VIR_INHIBITOR_MODE_DELAY,
callback,
opaque);
if ((driver->lockFD = if ((driver->lockFD =
virPidFileAcquire(driver->stateDir, "driver", getpid())) < 0) virPidFileAcquire(driver->stateDir, "driver", getpid())) < 0)
goto error; goto error;