When we allow multiple instances of the driver for the same user account, using a separate root directory, we need to ensure mutual exclusion. Use a pidfile to guarantee this. In privileged libvirtd this ends up locking /var/run/libvirt/secrets/driver.pid In unprivileged libvirtd this ends up locking /run/user/$UID/libvirt/secrets/run/driver.pid NB, the latter can vary depending on $XDG_RUNTIME_DIR Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/secret/secret_driver.c | 44 +++++++++++++++++++++++++++++++------- 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c index ac85f5d195..9344948db4 100644 --- a/src/secret/secret_driver.c +++ b/src/secret/secret_driver.c @@ -37,6 +37,7 @@ #include "viruuid.h" #include "virerror.h" #include "virfile.h" +#include "virpidfile.h" #include "configmake.h" #include "virstring.h" #include "viraccessapicheck.h" @@ -56,8 +57,12 @@ struct _virSecretDriverState { virMutex lock; bool privileged; /* readonly */ virSecretObjListPtr secrets; + char *stateDir; char *configDir; + /* pid file FD, ensures two copies of the driver can't use the same root */ + int lockFD; + /* Immutable pointer, self-locking APIs */ virObjectEventStatePtr secretEventState; }; @@ -434,6 +439,10 @@ secretStateCleanup(void) virObjectUnref(driver->secretEventState); + if (driver->lockFD != -1) + virPidFileRelease(driver->stateDir, "driver", driver->lockFD); + + VIR_FREE(driver->stateDir); secretDriverUnlock(); virMutexDestroy(&driver->lock); VIR_FREE(driver); @@ -447,11 +456,10 @@ secretStateInitialize(bool privileged, virStateInhibitCallback callback ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { - char *base = NULL; - if (VIR_ALLOC(driver) < 0) return -1; + driver->lockFD = -1; if (virMutexInit(&driver->lock) < 0) { VIR_FREE(driver); return -1; @@ -462,15 +470,26 @@ secretStateInitialize(bool privileged, driver->privileged = privileged; if (privileged) { - if (VIR_STRDUP(base, SYSCONFDIR "/libvirt") < 0) + if (virAsprintf(&driver->configDir, + "%s/libvirt/secrets", SYSCONFDIR) < 0) + goto error; + if (virAsprintf(&driver->stateDir, + "%s/run/libvirt/secrets", LOCALSTATEDIR) < 0) goto error; } else { - if (!(base = virGetUserConfigDirectory())) + VIR_AUTOFREE(char *) rundir = NULL; + VIR_AUTOFREE(char *) cfgdir = NULL; + + if (!(cfgdir = virGetUserConfigDirectory())) + goto error; + if (virAsprintf(&driver->configDir, "%s/secrets/", cfgdir) < 0) + goto error; + + if (!(rundir = virGetUserRuntimeDirectory())) + goto error; + if (virAsprintf(&driver->stateDir, "%s/secrets/run", rundir) < 0) goto error; } - if (virAsprintf(&driver->configDir, "%s/secrets", base) < 0) - goto error; - VIR_FREE(base); if (virFileMakePathWithMode(driver->configDir, S_IRWXU) < 0) { virReportSystemError(errno, _("cannot create config directory '%s'"), @@ -478,6 +497,16 @@ secretStateInitialize(bool privileged, goto error; } + if (virFileMakePathWithMode(driver->stateDir, S_IRWXU) < 0) { + virReportSystemError(errno, _("cannot create state directory '%s'"), + driver->stateDir); + goto error; + } + + if ((driver->lockFD = + virPidFileAcquire(driver->stateDir, "driver", true, getpid())) < 0) + goto error; + if (!(driver->secrets = virSecretObjListNew())) goto error; @@ -488,7 +517,6 @@ secretStateInitialize(bool privileged, return 0; error: - VIR_FREE(base); secretDriverUnlock(); secretStateCleanup(); return -1; -- 2.21.0

When we allow multiple instances of the driver for the same user account, using a separate root directory, we need to ensure mutual exclusion. Use a pidfile to guarantee this. In privileged libvirtd this ends up locking /var/run/libvirt/storage/driver.pid In unprivileged libvirtd this ends up locking /run/user/$UID/libvirt/storage/run/driver.pid NB, the latter can vary depending on $XDG_RUNTIME_DIR Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/conf/virstorageobj.h | 3 +++ src/storage/storage_driver.c | 11 +++++++++++ 2 files changed, 14 insertions(+) diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h index 92d229f9b4..4547a0df9b 100644 --- a/src/conf/virstorageobj.h +++ b/src/conf/virstorageobj.h @@ -37,6 +37,9 @@ typedef virStorageDriverState *virStorageDriverStatePtr; struct _virStorageDriverState { virMutex lock; + /* pid file FD, ensures two copies of the driver can't use the same root */ + int lockFD; + virStoragePoolObjListPtr pools; char *configDir; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 496d51b1e0..03ac6a6845 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -43,6 +43,7 @@ #include "virlog.h" #include "virfile.h" #include "virfdstream.h" +#include "virpidfile.h" #include "configmake.h" #include "virsecret.h" #include "virstring.h" @@ -256,6 +257,7 @@ storageStateInitialize(bool privileged, if (VIR_ALLOC(driver) < 0) return -1; + driver->lockFD = -1; if (virMutexInit(&driver->lock) < 0) { VIR_FREE(driver); return -1; @@ -296,6 +298,11 @@ storageStateInitialize(bool privileged, goto error; } + if ((driver->lockFD = + virPidFileAcquire(driver->stateDir, "driver", + true, getpid())) < 0) + goto error; + if (virStoragePoolObjLoadAllState(driver->pools, driver->stateDir) < 0) goto error; @@ -371,6 +378,10 @@ storageStateCleanup(void) /* free inactive pools */ virObjectUnref(driver->pools); + if (driver->lockFD != -1) + virPidFileRelease(driver->stateDir, "driver", + driver->lockFD); + VIR_FREE(driver->configDir); VIR_FREE(driver->autostartDir); VIR_FREE(driver->stateDir); -- 2.21.0

When we allow multiple instances of the driver for the same user account, using a separate root directory, we need to ensure mutual exclusion. Use a pidfile to guarantee this. In privileged libvirtd this ends up locking /var/run/libvirt/interface/driver.pid In unprivileged libvirtd this ends up locking /run/user/$UID/libvirt/interface/run/driver.pid NB, the latter can vary depending on $XDG_RUNTIME_DIR Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/interface/interface_backend_netcf.c | 45 +++++++++++++++++++++++-- src/interface/interface_backend_udev.c | 44 +++++++++++++++++++++++- 2 files changed, 85 insertions(+), 4 deletions(-) diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c index cf8eb9488d..868e49c56e 100644 --- a/src/interface/interface_backend_netcf.c +++ b/src/interface/interface_backend_netcf.c @@ -29,10 +29,14 @@ #include "interface_conf.h" #include "viralloc.h" #include "virlog.h" +#include "virfile.h" +#include "virpidfile.h" #include "virstring.h" #include "viraccessapicheck.h" #include "virinterfaceobj.h" +#include "configmake.h" + #define VIR_FROM_THIS VIR_FROM_INTERFACE VIR_LOG_INIT("interface.interface_backend_netcf"); @@ -43,6 +47,10 @@ VIR_LOG_INIT("interface.interface_backend_netcf"); typedef struct { virObjectLockable parent; + /* pid file FD, ensures two copies of the driver can't use the same root */ + int lockFD; + + char *stateDir; struct netcf *netcf; bool privileged; } virNetcfDriverState, *virNetcfDriverStatePtr; @@ -71,6 +79,11 @@ virNetcfDriverStateDispose(void *obj) if (_driver->netcf) ncf_close(_driver->netcf); + + if (_driver->lockFD != -1) + virPidFileRelease(_driver->stateDir, "driver", _driver->lockFD); + + VIR_FREE(_driver->stateDir); } @@ -87,15 +100,41 @@ netcfStateInitialize(bool privileged, driver->privileged = privileged; + if (privileged) { + if (virAsprintf(&driver->stateDir, + "%s/run/libvirt/nodedev", LOCALSTATEDIR) < 0) + goto error; + } else { + VIR_AUTOFREE(char *) rundir = NULL; + + if (!(rundir = virGetUserRuntimeDirectory())) + goto error; + if (virAsprintf(&driver->stateDir, "%s/nodedev/run", rundir) < 0) + goto error; + } + + if (virFileMakePathWithMode(driver->stateDir, S_IRWXU) < 0) { + virReportSystemError(errno, _("cannot create state directory '%s'"), + driver->stateDir); + goto error; + } + + if ((driver->lockFD = + virPidFileAcquire(driver->stateDir, "driver", true, getpid())) < 0) + goto error; + /* open netcf */ if (ncf_init(&driver->netcf, NULL) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to initialize netcf")); - virObjectUnref(driver); - driver = NULL; - return -1; + goto error; } return 0; + + error: + virObjectUnref(driver); + driver = NULL; + return -1; } diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c index 1373356246..fcd7f1c04a 100644 --- a/src/interface/interface_backend_udev.c +++ b/src/interface/interface_backend_udev.c @@ -32,14 +32,21 @@ #include "interface_conf.h" #include "viralloc.h" #include "virstring.h" +#include "virpidfile.h" #include "viraccessapicheck.h" #include "virinterfaceobj.h" #include "virnetdev.h" +#include "configmake.h" + #define VIR_FROM_THIS VIR_FROM_INTERFACE struct udev_iface_driver { struct udev *udev; + /* pid file FD, ensures two copies of the driver can't use the same root */ + int lockFD; + + char *stateDir; bool privileged; }; @@ -1157,6 +1164,9 @@ udevInterfaceIsActive(virInterfacePtr ifinfo) } +static int +udevStateCleanup(void); + static int udevStateInitialize(bool privileged, virStateInhibitCallback callback ATTRIBUTE_UNUSED, @@ -1167,6 +1177,31 @@ udevStateInitialize(bool privileged, if (VIR_ALLOC(driver) < 0) goto cleanup; + driver->lockFD = -1; + + if (privileged) { + if (virAsprintf(&driver->stateDir, + "%s/run/libvirt/nodedev", LOCALSTATEDIR) < 0) + goto cleanup; + } else { + VIR_AUTOFREE(char *) rundir = NULL; + + if (!(rundir = virGetUserRuntimeDirectory())) + goto cleanup; + if (virAsprintf(&driver->stateDir, "%s/nodedev/run", rundir) < 0) + goto cleanup; + } + + if (virFileMakePathWithMode(driver->stateDir, S_IRWXU) < 0) { + virReportSystemError(errno, _("cannot create state directory '%s'"), + driver->stateDir); + goto cleanup; + } + + if ((driver->lockFD = + virPidFileAcquire(driver->stateDir, "driver", true, getpid())) < 0) + goto cleanup; + driver->udev = udev_new(); if (!driver->udev) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -1178,6 +1213,8 @@ udevStateInitialize(bool privileged, ret = 0; cleanup: + if (ret < 0) + udevStateCleanup(); return ret; } @@ -1187,8 +1224,13 @@ udevStateCleanup(void) if (!driver) return -1; - udev_unref(driver->udev); + if (driver->udev) + udev_unref(driver->udev); + + if (driver->lockFD != -1) + virPidFileRelease(driver->stateDir, "driver", driver->lockFD); + VIR_FREE(driver->stateDir); VIR_FREE(driver); return 0; } -- 2.21.0

No supported build targets for libvirt still ship xend, so there is no need for the libxl driver to check for it anymore. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/libxl/libxl_driver.c | 26 ++++---------------------- 1 file changed, 4 insertions(+), 22 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 731700ded6..ac10fb6dbc 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -525,12 +525,10 @@ libxlStateCleanup(void) static bool libxlDriverShouldLoad(bool privileged) { - bool ret = false; - /* Don't load if non-root */ if (!privileged) { VIR_INFO("Not running privileged, disabling libxenlight driver"); - return ret; + return false; } if (virFileExists(HYPERVISOR_CAPABILITIES)) { @@ -549,31 +547,15 @@ libxlDriverShouldLoad(bool privileged) VIR_INFO("No Xen capabilities detected, probably not running " "in a Xen Dom0. Disabling libxenlight driver"); - return ret; + return false; } } else if (!virFileExists(HYPERVISOR_XENSTORED)) { VIR_INFO("Disabling driver as neither " HYPERVISOR_CAPABILITIES " nor " HYPERVISOR_XENSTORED " exist"); - return ret; + return false; } - /* Don't load if legacy xen toolstack (xend) is in use */ - if (virFileExists("/usr/sbin/xend")) { - virCommandPtr cmd; - - cmd = virCommandNewArgList("/usr/sbin/xend", "status", NULL); - if (virCommandRun(cmd, NULL) == 0) { - VIR_INFO("Legacy xen tool stack seems to be in use, disabling " - "libxenlight driver."); - } else { - ret = true; - } - virCommandFree(cmd); - } else { - ret = true; - } - - return ret; + return true; } /* Callbacks wrapping libvirt's event loop interface */ -- 2.21.0

When we allow multiple instances of the driver for the same user account, using a separate root directory, we need to ensure mutual exclusion. Use a pidfile to guarantee this. In privileged libvirtd this ends up locking /var/run/libvirt/lxc/driver.pid In unprivileged libvirtd this ends up locking /run/user/$UID/libvirt/lxc/run/driver.pid NB, the latter can vary depending on $XDG_RUNTIME_DIR Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/lxc/lxc_conf.h | 3 +++ src/lxc/lxc_driver.c | 9 +++++++++ 2 files changed, 12 insertions(+) diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h index dc5531ebf9..e26ca22d3c 100644 --- a/src/lxc/lxc_conf.h +++ b/src/lxc/lxc_conf.h @@ -70,6 +70,9 @@ struct _virLXCDriver { * then lockless thereafter */ virLXCDriverConfigPtr config; + /* pid file FD, ensures two copies of the driver can't use the same root */ + int lockFD; + /* Require lock to get a reference on the object, * lockless access thereafter */ virCapsPtr caps; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 9db2a02dee..3982c24f34 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1559,6 +1559,7 @@ static int lxcStateInitialize(bool privileged, if (VIR_ALLOC(lxc_driver) < 0) return -1; + lxc_driver->lockFD = -1; if (virMutexInit(&lxc_driver->lock) < 0) { VIR_FREE(lxc_driver); return -1; @@ -1605,6 +1606,10 @@ static int lxcStateInitialize(bool privileged, goto cleanup; } + if ((lxc_driver->lockFD = + virPidFileAcquire(cfg->stateDir, "driver", true, getpid())) < 0) + goto cleanup; + /* Get all the running persistent or transient configs first */ if (virDomainObjListLoadAllConfigs(lxc_driver->domains, cfg->stateDir, @@ -1696,6 +1701,10 @@ static int lxcStateCleanup(void) virObjectUnref(lxc_driver->caps); virObjectUnref(lxc_driver->securityManager); virObjectUnref(lxc_driver->xmlopt); + + if (lxc_driver->lockFD != -1) + virPidFileRelease(lxc_driver->config->stateDir, "driver", lxc_driver->lockFD); + virObjectUnref(lxc_driver->config); virMutexDestroy(&lxc_driver->lock); VIR_FREE(lxc_driver); -- 2.21.0

When we allow multiple instances of the driver for the same user account, using a separate root directory, we need to ensure mutual exclusion. Use a pidfile to guarantee this. In privileged libvirtd this ends up locking /var/run/libvirt/bhyve/driver.pid In unprivileged libvirtd this ends up locking /run/user/$UID/libvirt/bhyve/run/driver.pid NB, the latter can vary depending on $XDG_RUNTIME_DIR Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/bhyve/bhyve_driver.c | 9 +++++++++ src/bhyve/bhyve_utils.h | 3 +++ 2 files changed, 12 insertions(+) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 4ce9ef0b95..cfcf4e1fba 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -43,6 +43,7 @@ #include "virthread.h" #include "virlog.h" #include "virfile.h" +#include "virpidfile.h" #include "virtypedparam.h" #include "virrandom.h" #include "virstring.h" @@ -1203,6 +1204,9 @@ bhyveStateCleanup(void) virObjectUnref(bhyve_driver->config); virPortAllocatorRangeFree(bhyve_driver->remotePorts); + if (bhyve_driver->lockFD != -1) + virPidFileRelease(BHYVE_STATE_DIR, "driver", bhyve_driver->lockFD); + virMutexDestroy(&bhyve_driver->lock); VIR_FREE(bhyve_driver); @@ -1222,6 +1226,7 @@ bhyveStateInitialize(bool privileged, if (VIR_ALLOC(bhyve_driver) < 0) return -1; + bhyve_driver->lockFD = -1; if (virMutexInit(&bhyve_driver->lock) < 0) { VIR_FREE(bhyve_driver); return -1; @@ -1274,6 +1279,10 @@ bhyveStateInitialize(bool privileged, goto cleanup; } + if ((bhyve_driver->lockFD = + virPidFileAcquire(BHYVE_STATE_DIR, "driver", true, getpid())) < 0) + goto cleanup; + if (virDomainObjListLoadAllConfigs(bhyve_driver->domains, BHYVE_STATE_DIR, NULL, true, diff --git a/src/bhyve/bhyve_utils.h b/src/bhyve/bhyve_utils.h index 26956d7d21..3d212e3ccf 100644 --- a/src/bhyve/bhyve_utils.h +++ b/src/bhyve/bhyve_utils.h @@ -48,6 +48,9 @@ struct _bhyveConn { virBhyveDriverConfigPtr config; + /* pid file FD, ensures two copies of the driver can't use the same root */ + int lockFD; + virDomainObjListPtr domains; virCapsPtr caps; virDomainXMLOptionPtr xmlopt; -- 2.21.0