The comment says that the function kills domains and networks. This is obviously not the case. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7d14588409..99db99e23a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1154,7 +1154,7 @@ qemuStateStop(void) /** * qemuStateCleanup: * - * Shutdown the QEMU daemon, it will stop all active domains and networks + * Release resources allocated by QEMU driver (no domain is shut off though) */ static int qemuStateCleanup(void) -- 2.21.0

Some of objects we manage can be autostarted on libvirtd startup (e.g. domains, network, storage pools). The idea was that when the host is started up these objects are started too without need of user intervention. However, with the latest daemon split and switch to socket activated, short lived daemons (we put --timeout 120 onto each daemon's command line) this doesn't do what we want it to. The problem is not new though, we already had the session daemon come and go and we circumvented this problem by documenting it (see v4.10.0-92-g61b4e8aaf1). But now that we meet the same problem at all fronts it's time to deal with it. The solution implemented in this commit is to have a file (one per each driver) that: 1) if doesn't exist, is created and autostart is allowed for given driver, 2) if it does exist, then autostart is suppressed for given driver. All the files live in a location that doesn't survive host reboots (/var/run/ for instance) and thus the file is automatically not there on fresh host boot. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/driver.c | 39 +++++++++++++++++++++++++++++++++++++++ src/driver.h | 3 +++ src/libvirt_private.syms | 1 + 3 files changed, 43 insertions(+) diff --git a/src/driver.c b/src/driver.c index ed2d943ddf..305c4c624b 100644 --- a/src/driver.c +++ b/src/driver.c @@ -29,6 +29,7 @@ #include "virfile.h" #include "virlog.h" #include "virmodule.h" +#include "virstring.h" #include "virthread.h" #include "configmake.h" @@ -69,6 +70,44 @@ virDriverLoadModule(const char *name, /* XXX unload modules, but we can't until we can unregister libvirt drivers */ +/** + * virDriverShouldAutostart: + * @dir: driver's run state directory (usually /var/run/libvirt/$driver) + * @autostart: whether driver should initiate autostart + * + * Automatic starting of libvirt's objects (e.g. domains, networks, storage + * pools, etc.) doesn't play nice with using '--timeout' on daemon's command + * line because the objects are attempted to autostart on every start of + * corresponding driver/daemon. To resolve this problem, a file is created in + * driver's private directory (which doesn't survive host's reboot) and thus + * autostart is attempted only once. + */ +int +virDriverShouldAutostart(const char *dir, + bool *autostart) +{ + VIR_AUTOFREE(char *) path = NULL; + + *autostart = false; + + if (virAsprintf(&path, "%s/autostarted", dir) < 0) + return -1; + + if (virFileExists(path)) { + VIR_DEBUG("Autostart file %s exists, skipping autostart", path); + return 0; + } + + VIR_DEBUG("Autostart file %s does not exist, do autostart", path); + *autostart = true; + + if (virFileTouch(path, 0600) < 0) + return -1; + + return 0; +} + + virThreadLocal connectInterface; virThreadLocal connectNetwork; virThreadLocal connectNWFilter; diff --git a/src/driver.h b/src/driver.h index 68c0004d86..faf4788a4f 100644 --- a/src/driver.h +++ b/src/driver.h @@ -114,6 +114,9 @@ int virDriverLoadModule(const char *name, const char *regfunc, bool required); +int virDriverShouldAutostart(const char *name, + bool *autostart); + virConnectPtr virGetConnectInterface(void); virConnectPtr virGetConnectNetwork(void); virConnectPtr virGetConnectNWFilter(void); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index eeab820eca..c818bc807a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1346,6 +1346,7 @@ virStreamClass; # driver.h virConnectValidateURIPath; +virDriverShouldAutostart; virGetConnectInterface; virGetConnectNetwork; virGetConnectNodeDev; -- 2.21.0

https://bugzilla.redhat.com/show_bug.cgi?id=1755303 With the recent work in daemon split and socket activation daemons can come and go. They can and will be started many times during a session which results in objects being autostarted multiple times. This is not optimal. Use virDriverShouldAutostart() to determine if autostart should be done or not. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/bhyve/bhyve_driver.c | 8 +++++++- src/libxl/libxl_driver.c | 12 +++++++++--- src/lxc/lxc_driver.c | 7 ++++++- src/network/bridge_driver.c | 12 +++++++++--- src/qemu/qemu_driver.c | 7 ++++++- src/storage/storage_driver.c | 7 ++++++- 6 files changed, 43 insertions(+), 10 deletions(-) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index c52def7607..fb0996bd2a 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -1218,6 +1218,8 @@ bhyveStateInitialize(bool privileged, virStateInhibitCallback callback ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { + bool autostart = true; + if (!privileged) { VIR_INFO("Not running privileged, disabling driver"); return VIR_DRV_STATE_INIT_SKIPPED; @@ -1301,7 +1303,11 @@ bhyveStateInitialize(bool privileged, virBhyveProcessReconnectAll(bhyve_driver); - bhyveAutostartDomains(bhyve_driver); + if (virDriverShouldAutostart(BHYVE_STATE_DIR, &autostart) < 0) + goto cleanup; + + if (autostart) + bhyveAutostartDomains(bhyve_driver); return VIR_DRV_STATE_INIT_COMPLETE; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 45de6b24c6..ea643b8a3f 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -655,6 +655,7 @@ libxlStateInitialize(bool privileged, libxlDriverConfigPtr cfg; char *driverConf = NULL; char ebuf[1024]; + bool autostart = true; if (!libxlDriverShouldLoad(privileged)) return VIR_DRV_STATE_INIT_SKIPPED; @@ -800,9 +801,14 @@ libxlStateInitialize(bool privileged, NULL, NULL) < 0) goto error; - virDomainObjListForEach(libxl_driver->domains, false, - libxlAutostartDomain, - libxl_driver); + if (virDriverShouldAutostart(cfg->stateDir, &autostart) < 0) + goto error; + + if (autostart) { + virDomainObjListForEach(libxl_driver->domains, false, + libxlAutostartDomain, + libxl_driver); + } virDomainObjListForEach(libxl_driver->domains, false, libxlDomainManagedSaveLoad, diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index cf1dd1428e..a69589e50c 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1541,6 +1541,7 @@ static int lxcStateInitialize(bool privileged, { virCapsPtr caps = NULL; virLXCDriverConfigPtr cfg = NULL; + bool autostart = true; /* Check that the user is root, silently disable if not */ if (!privileged) { @@ -1630,7 +1631,11 @@ static int lxcStateInitialize(bool privileged, NULL, NULL) < 0) goto cleanup; - virLXCProcessAutostartAll(lxc_driver); + if (virDriverShouldAutostart(cfg->stateDir, &autostart) < 0) + goto cleanup; + + if (autostart) + virLXCProcessAutostartAll(lxc_driver); virObjectUnref(caps); return VIR_DRV_STATE_INIT_COMPLETE; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index ae2e4f09f8..c05157c3ca 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -715,6 +715,7 @@ networkStateInitialize(bool privileged, int ret = VIR_DRV_STATE_INIT_ERROR; char *configdir = NULL; char *rundir = NULL; + bool autostart = true; #ifdef WITH_FIREWALLD DBusConnection *sysbus = NULL; #endif @@ -815,9 +816,14 @@ networkStateInitialize(bool privileged, networkReloadFirewallRules(network_driver, true); networkRefreshDaemons(network_driver); - virNetworkObjListForEach(network_driver->networks, - networkAutostartConfig, - network_driver); + if (virDriverShouldAutostart(network_driver->stateDir, &autostart) < 0) + goto error; + + if (autostart) { + virNetworkObjListForEach(network_driver->networks, + networkAutostartConfig, + network_driver); + } network_driver->networkEventState = virObjectEventStateNew(); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 99db99e23a..bc0ede2fb0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -673,6 +673,7 @@ qemuStateInitialize(bool privileged, gid_t run_gid = -1; char *hugepagePath = NULL; char *memoryBackingPath = NULL; + bool autostart = true; size_t i; if (VIR_ALLOC(qemu_driver) < 0) @@ -1035,7 +1036,11 @@ qemuStateInitialize(bool privileged, qemuProcessReconnectAll(qemu_driver); - qemuAutostartDomains(qemu_driver); + if (virDriverShouldAutostart(cfg->stateDir, &autostart) < 0) + goto error; + + if (autostart) + qemuAutostartDomains(qemu_driver); return VIR_DRV_STATE_INIT_COMPLETE; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index d160ff34fe..c536535c6c 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -258,6 +258,7 @@ storageStateInitialize(bool privileged, { VIR_AUTOFREE(char *) configdir = NULL; VIR_AUTOFREE(char *) rundir = NULL; + bool autostart = true; if (VIR_ALLOC(driver) < 0) return VIR_DRV_STATE_INIT_ERROR; @@ -319,7 +320,11 @@ storageStateInitialize(bool privileged, storagePoolUpdateAllState(); - storageDriverAutostart(); + if (virDriverShouldAutostart(driver->stateDir, &autostart) < 0) + goto error; + + if (autostart) + storageDriverAutostart(); driver->storageEventState = virObjectEventStateNew(); -- 2.21.0

Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/news.xml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 9fd8f65515..d9c402e6bb 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -45,6 +45,21 @@ <section title="Improvements"> </section> <section title="Bug fixes"> + <change> + <summary> + lib: autostart objects exactly once + </summary> + <description> + If libvirtd or any of the sub-daemons is started with socket + activation then objects might be autostarted more than once. + For instance, if a domain under <code> qemu:///session </code> + URI is mark as autostarted and the session daemon is started then the + domain is started with it. If user shuts the domain down and the + session daemon is started again, the user's wish to keep the + domain shut off is ignored and the domain is autostarted again. + This is now fixed. + </description> + </change> </section> </release> <release version="v5.8.0" date="2019-10-05"> -- 2.21.0

This reverts commit 61b4e8aaf1bce07f282c152de556c3d6aa8d65be. After previous commits this is no longer needed. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt-domain.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index e200dcc7d0..65c73be312 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -6696,11 +6696,6 @@ virDomainCreateWithFiles(virDomainPtr domain, unsigned int nfiles, * configured to be automatically started when the host * machine boots. * - * Please note that this might result in unexpected behaviour if - * used for some session URIs. Since the session daemon is started - * with --timeout it comes and goes and as it does so it - * autostarts domains which might have been shut off recently. - * * Returns -1 in case of error, 0 in case of success */ int -- 2.21.0