[PATCH v2 0/8] Refactor autostart for VM daemons
These patches are a spin off from my bigger auto-shutdown series: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/EIBL5... Since they're self contained, they can be merged sooner while I'm still working on Peter's feedback from latter parts of the above patch series. Daniel P. Berrangé (8): bhyve: add 'driver' to bhyveProcessStart method bhyve: stop opening & passing connection to bhyveProcessStart util: add APIs for more systemd notifications remote: notify systemd when reloading config hypervisor: introduce helper for autostart src: convert drivers over to use new autostart helper hypervisor: add support for delay interval during autostart qemu: add 'auto_start_delay' configuration parameter src/bhyve/bhyve_driver.c | 60 ++++++++++-------------------- src/bhyve/bhyve_process.c | 5 +-- src/bhyve/bhyve_process.h | 3 +- src/hypervisor/domain_driver.c | 55 +++++++++++++++++++++++++++ src/hypervisor/domain_driver.h | 19 ++++++++++ src/libvirt_private.syms | 7 +++- src/libxl/libxl_driver.c | 36 +++++------------- src/lxc/lxc_driver.c | 13 ++++--- src/lxc/lxc_process.c | 18 +-------- src/lxc/lxc_process.h | 2 + src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf.in | 5 +++ src/qemu/qemu_conf.c | 2 + src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 55 +++++++++------------------ src/qemu/test_libvirtd_qemu.aug.in | 1 + src/remote/libvirtd.service.in | 2 +- src/remote/remote_daemon.c | 2 + src/rpc/virnetdaemon.c | 2 +- src/util/virsystemd.c | 44 ++++++++++++++++++++-- src/util/virsystemd.h | 6 ++- src/virtd.service.in | 2 +- 22 files changed, 202 insertions(+), 139 deletions(-) -- 2.47.1
This allows for passinga NULL connection object in cases where domain autodestroy is not required. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/bhyve/bhyve_driver.c | 6 +++--- src/bhyve/bhyve_process.c | 5 ++--- src/bhyve/bhyve_process.h | 3 ++- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 8f97ac032c..ea5a9be4fb 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -79,7 +79,7 @@ bhyveAutostartDomain(virDomainObj *vm, void *opaque) if (vm->autostart && !virDomainObjIsActive(vm)) { virResetLastError(); - ret = virBhyveProcessStart(data->conn, vm, + ret = virBhyveProcessStart(bhyve_driver, data->conn, vm, VIR_DOMAIN_RUNNING_BOOTED, 0); if (ret < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -834,7 +834,7 @@ bhyveDomainCreateWithFlags(virDomainPtr dom, goto cleanup; } - ret = virBhyveProcessStart(dom->conn, vm, + ret = virBhyveProcessStart(privconn, dom->conn, vm, VIR_DOMAIN_RUNNING_BOOTED, start_flags); @@ -892,7 +892,7 @@ bhyveDomainCreateXML(virConnectPtr conn, VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, NULL))) goto cleanup; - if (virBhyveProcessStart(conn, vm, + if (virBhyveProcessStart(privconn, conn, vm, VIR_DOMAIN_RUNNING_BOOTED, start_flags) < 0) { /* If domain is not persistent, remove its data */ diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c index 41feabcbe7..3e6f678cf5 100644 --- a/src/bhyve/bhyve_process.c +++ b/src/bhyve/bhyve_process.c @@ -270,13 +270,12 @@ bhyveProcessPrepareDomain(bhyveConn *driver, } int -virBhyveProcessStart(virConnectPtr conn, +virBhyveProcessStart(bhyveConn *driver, + virConnectPtr conn, virDomainObj *vm, virDomainRunningReason reason, unsigned int flags) { - struct _bhyveConn *driver = conn->privateData; - /* Run an early hook to setup missing devices. */ if (bhyveProcessStartHook(driver, vm, VIR_HOOK_BHYVE_OP_PREPARE) < 0) return -1; diff --git a/src/bhyve/bhyve_process.h b/src/bhyve/bhyve_process.h index 668411db57..e69db41fc2 100644 --- a/src/bhyve/bhyve_process.h +++ b/src/bhyve/bhyve_process.h @@ -28,7 +28,8 @@ bhyveProcessPrepareDomain(bhyveConn *driver, virDomainObj *vm, unsigned int flags); -int virBhyveProcessStart(virConnect *conn, +int virBhyveProcessStart(bhyveConn *driver, + virConnectPtr conn, virDomainObj *vm, virDomainRunningReason reason, unsigned int flags); -- 2.47.1
On Wed, Feb 12, 2025 at 15:46:39 +0000, Daniel P. Berrangé wrote:
This allows for passinga NULL connection object in cases where domain autodestroy is not required.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/bhyve/bhyve_driver.c | 6 +++--- src/bhyve/bhyve_process.c | 5 ++--- src/bhyve/bhyve_process.h | 3 ++- 3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 8f97ac032c..ea5a9be4fb 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -79,7 +79,7 @@ bhyveAutostartDomain(virDomainObj *vm, void *opaque)
if (vm->autostart && !virDomainObjIsActive(vm)) { virResetLastError(); - ret = virBhyveProcessStart(data->conn, vm, + ret = virBhyveProcessStart(bhyve_driver, data->conn, vm,
This function already gets the driver passed in data->driver so you don't need to access the global variable. Reviewed-by: Peter Krempa <pkrempa@redhat.com>
On Wed, Feb 12, 2025 at 05:41:41PM +0100, Peter Krempa wrote:
On Wed, Feb 12, 2025 at 15:46:39 +0000, Daniel P. Berrangé wrote:
This allows for passinga NULL connection object in cases where domain autodestroy is not required.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/bhyve/bhyve_driver.c | 6 +++--- src/bhyve/bhyve_process.c | 5 ++--- src/bhyve/bhyve_process.h | 3 ++- 3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 8f97ac032c..ea5a9be4fb 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -79,7 +79,7 @@ bhyveAutostartDomain(virDomainObj *vm, void *opaque)
if (vm->autostart && !virDomainObjIsActive(vm)) { virResetLastError(); - ret = virBhyveProcessStart(data->conn, vm, + ret = virBhyveProcessStart(bhyve_driver, data->conn, vm,
This function already gets the driver passed in data->driver so you don't need to access the global variable.
This changes in the next patch, but yeah, makes sense to use data->driver meanwhile
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
A connection object is not required because autostarted domains are never marked for autodestroy. The comment about needing a connection for the network driver is obsolete since we can auto-open a connection on demand. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/bhyve/bhyve_driver.c | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index ea5a9be4fb..f6b0417014 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -71,15 +71,14 @@ VIR_LOG_INIT("bhyve.bhyve_driver"); struct _bhyveConn *bhyve_driver = NULL; static int -bhyveAutostartDomain(virDomainObj *vm, void *opaque) +bhyveAutostartDomain(virDomainObj *vm, void *opaque G_GNUC_UNUSED) { - const struct bhyveAutostartData *data = opaque; int ret = 0; VIR_LOCK_GUARD lock = virObjectLockGuard(vm); if (vm->autostart && !virDomainObjIsActive(vm)) { virResetLastError(); - ret = virBhyveProcessStart(bhyve_driver, data->conn, vm, + ret = virBhyveProcessStart(bhyve_driver, NULL, vm, VIR_DOMAIN_RUNNING_BOOTED, 0); if (ret < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -93,19 +92,7 @@ bhyveAutostartDomain(virDomainObj *vm, void *opaque) static void bhyveAutostartDomains(struct _bhyveConn *driver) { - /* XXX: Figure out a better way todo this. The domain - * startup code needs a connection handle in order - * to lookup the bridge associated with a virtual - * network - */ - virConnectPtr conn = virConnectOpen("bhyve:///system"); - /* Ignoring NULL conn which is mostly harmless here */ - - struct bhyveAutostartData data = { driver, conn }; - - virDomainObjListForEach(driver->domains, false, bhyveAutostartDomain, &data); - - virObjectUnref(conn); + virDomainObjListForEach(driver->domains, false, bhyveAutostartDomain, NULL); } /** -- 2.47.1
On Wed, Feb 12, 2025 at 15:46:40 +0000, Daniel P. Berrangé wrote:
A connection object is not required because autostarted domains are never marked for autodestroy.
The comment about needing a connection for the network driver is obsolete since we can auto-open a connection on demand.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/bhyve/bhyve_driver.c | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index ea5a9be4fb..f6b0417014 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -71,15 +71,14 @@ VIR_LOG_INIT("bhyve.bhyve_driver"); struct _bhyveConn *bhyve_driver = NULL;
static int -bhyveAutostartDomain(virDomainObj *vm, void *opaque) +bhyveAutostartDomain(virDomainObj *vm, void *opaque G_GNUC_UNUSED) { - const struct bhyveAutostartData *data = opaque;
Re 1/8: Ah I see.
int ret = 0; VIR_LOCK_GUARD lock = virObjectLockGuard(vm);
if (vm->autostart && !virDomainObjIsActive(vm)) { virResetLastError(); - ret = virBhyveProcessStart(bhyve_driver, data->conn, vm, + ret = virBhyveProcessStart(bhyve_driver, NULL, vm, VIR_DOMAIN_RUNNING_BOOTED, 0); if (ret < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -93,19 +92,7 @@ bhyveAutostartDomain(virDomainObj *vm, void *opaque) static void bhyveAutostartDomains(struct _bhyveConn *driver)
This argument will be unused now.
{ - /* XXX: Figure out a better way todo this. The domain - * startup code needs a connection handle in order - * to lookup the bridge associated with a virtual - * network - */ - virConnectPtr conn = virConnectOpen("bhyve:///system"); - /* Ignoring NULL conn which is mostly harmless here */ - - struct bhyveAutostartData data = { driver, conn }; - - virDomainObjListForEach(driver->domains, false, bhyveAutostartDomain, &data); - - virObjectUnref(conn); + virDomainObjListForEach(driver->domains, false, bhyveAutostartDomain, NULL);
I'm not sure what the idea is but you could pass the driver as private data still. Reviewed-by: Peter Krempa <pkrempa@redhat.com>
On Wed, Feb 12, 2025 at 17:43:50 +0100, Peter Krempa wrote:
On Wed, Feb 12, 2025 at 15:46:40 +0000, Daniel P. Berrangé wrote:
A connection object is not required because autostarted domains are never marked for autodestroy.
The comment about needing a connection for the network driver is obsolete since we can auto-open a connection on demand.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/bhyve/bhyve_driver.c | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index ea5a9be4fb..f6b0417014 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -71,15 +71,14 @@ VIR_LOG_INIT("bhyve.bhyve_driver"); struct _bhyveConn *bhyve_driver = NULL;
static int -bhyveAutostartDomain(virDomainObj *vm, void *opaque) +bhyveAutostartDomain(virDomainObj *vm, void *opaque G_GNUC_UNUSED) { - const struct bhyveAutostartData *data = opaque;
Re 1/8: Ah I see.
int ret = 0; VIR_LOCK_GUARD lock = virObjectLockGuard(vm);
if (vm->autostart && !virDomainObjIsActive(vm)) { virResetLastError(); - ret = virBhyveProcessStart(bhyve_driver, data->conn, vm, + ret = virBhyveProcessStart(bhyve_driver, NULL, vm, VIR_DOMAIN_RUNNING_BOOTED, 0); if (ret < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -93,19 +92,7 @@ bhyveAutostartDomain(virDomainObj *vm, void *opaque) static void bhyveAutostartDomains(struct _bhyveConn *driver)
This argument will be unused now.
Never-mind this ..
{ - /* XXX: Figure out a better way todo this. The domain - * startup code needs a connection handle in order - * to lookup the bridge associated with a virtual - * network - */ - virConnectPtr conn = virConnectOpen("bhyve:///system"); - /* Ignoring NULL conn which is mostly harmless here */ - - struct bhyveAutostartData data = { driver, conn }; - - virDomainObjListForEach(driver->domains, false, bhyveAutostartDomain, &data); - - virObjectUnref(conn); + virDomainObjListForEach(driver->domains, false, bhyveAutostartDomain, NULL);
... as it is used here.
I'm not sure what the idea is but you could pass the driver as private data still.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
On Wed, Feb 12, 2025 at 05:43:50PM +0100, Peter Krempa wrote:
On Wed, Feb 12, 2025 at 15:46:40 +0000, Daniel P. Berrangé wrote:
A connection object is not required because autostarted domains are never marked for autodestroy.
The comment about needing a connection for the network driver is obsolete since we can auto-open a connection on demand.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/bhyve/bhyve_driver.c | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index ea5a9be4fb..f6b0417014 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -71,15 +71,14 @@ VIR_LOG_INIT("bhyve.bhyve_driver"); struct _bhyveConn *bhyve_driver = NULL;
static int -bhyveAutostartDomain(virDomainObj *vm, void *opaque) +bhyveAutostartDomain(virDomainObj *vm, void *opaque G_GNUC_UNUSED) { - const struct bhyveAutostartData *data = opaque;
Re 1/8: Ah I see.
int ret = 0; VIR_LOCK_GUARD lock = virObjectLockGuard(vm);
if (vm->autostart && !virDomainObjIsActive(vm)) { virResetLastError(); - ret = virBhyveProcessStart(bhyve_driver, data->conn, vm, + ret = virBhyveProcessStart(bhyve_driver, NULL, vm, VIR_DOMAIN_RUNNING_BOOTED, 0); if (ret < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -93,19 +92,7 @@ bhyveAutostartDomain(virDomainObj *vm, void *opaque) static void bhyveAutostartDomains(struct _bhyveConn *driver)
This argument will be unused now.
{ - /* XXX: Figure out a better way todo this. The domain - * startup code needs a connection handle in order - * to lookup the bridge associated with a virtual - * network - */ - virConnectPtr conn = virConnectOpen("bhyve:///system"); - /* Ignoring NULL conn which is mostly harmless here */ - - struct bhyveAutostartData data = { driver, conn }; - - virDomainObjListForEach(driver->domains, false, bhyveAutostartDomain, &data); - - virObjectUnref(conn); + virDomainObjListForEach(driver->domains, false, bhyveAutostartDomain, NULL);
I'm not sure what the idea is but you could pass the driver as private data still.
That was an oversight from retrofitting this at the start of the series before this code gets changed again by later patch
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
We have a way to notify systemd when we're done starting the daemon. Systemd supports many more notifications, however, and many of them are quite relevant to our needs: https://www.freedesktop.org/software/systemd/man/latest/sd_notify.html This renames the existing notification API to better reflect its semantics, and adds new APIs for reporting * Initiation of config file reload * Initiation of daemon shutdown process * Adhoc progress status messages * Request to extend service shutdown timeout Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/libvirt_private.syms | 6 +++++- src/rpc/virnetdaemon.c | 2 +- src/util/virsystemd.c | 44 +++++++++++++++++++++++++++++++++++++--- src/util/virsystemd.h | 6 +++++- 4 files changed, 52 insertions(+), 6 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6888f6b599..4eca252b2a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3531,7 +3531,11 @@ virSystemdHasResolved; virSystemdHasResolvedResetCachedValue; virSystemdMakeScopeName; virSystemdMakeSliceName; -virSystemdNotifyStartup; +virSystemdNotifyExtendTimeout; +virSystemdNotifyReady; +virSystemdNotifyReload; +virSystemdNotifyStatus; +virSystemdNotifyStopping; virSystemdResolvedRegisterNameServer; virSystemdTerminateMachine; diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index f647cf60ba..e6bdfe0f88 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -753,7 +753,7 @@ virNetDaemonRun(virNetDaemon *dmn) /* We are accepting connections now. Notify systemd * so it can start dependent services. */ - virSystemdNotifyStartup(); + virSystemdNotifyReady(); VIR_DEBUG("dmn=%p quit=%d", dmn, dmn->quit); while (!dmn->finished) { diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index d46e5f74fc..bbcf77a41f 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -624,12 +624,11 @@ int virSystemdTerminateMachine(const char *name) return 0; } -void -virSystemdNotifyStartup(void) +static void +virSystemdNotify(const char *msg) { #ifndef WIN32 const char *path; - const char *msg = "READY=1"; int fd; struct sockaddr_un un = { .sun_family = AF_UNIX, @@ -644,6 +643,8 @@ virSystemdNotifyStartup(void) .msg_iovlen = 1, }; + VIR_DEBUG("Notify '%s'", msg); + if (!(path = getenv("NOTIFY_SOCKET"))) { VIR_DEBUG("Skipping systemd notify, not requested"); return; @@ -674,6 +675,43 @@ virSystemdNotifyStartup(void) #endif /* !WIN32 */ } +void virSystemdNotifyReady(void) +{ + virSystemdNotify("READY=1"); +} + +void virSystemdNotifyReload(void) +{ + gint64 now = g_get_monotonic_time(); + g_autofree char *msg = g_strdup_printf( + "RELOADING=1\nMONOTONIC_USEC=%lld", (long long int)now); + virSystemdNotify(msg); +} + +void virSystemdNotifyStopping(void) +{ + virSystemdNotify("STOPPING=1"); +} + +void virSystemdNotifyExtendTimeout(int secs) +{ + g_autofree char *msg = g_strdup_printf("EXTEND_TIMEOUT_USEC=%llu", + secs * 1000ull * 1000ul); + virSystemdNotify(msg); +} + +void virSystemdNotifyStatus(const char *fmt, ...) +{ + g_autofree char *msg1 = NULL; + g_autofree char *msg2 = NULL; + va_list ap; + va_start(ap, fmt); + msg1 = g_strdup_vprintf(fmt, ap); + va_end(ap); + msg2 = g_strdup_printf("STATUS=%s", msg1); + virSystemdNotify(msg2); +} + static int virSystemdPMSupportTarget(const char *methodName, bool *result) { diff --git a/src/util/virsystemd.h b/src/util/virsystemd.h index b7fdaf67df..98460dbc3a 100644 --- a/src/util/virsystemd.h +++ b/src/util/virsystemd.h @@ -44,7 +44,11 @@ int virSystemdCreateMachine(const char *name, int virSystemdTerminateMachine(const char *name); -void virSystemdNotifyStartup(void); +void virSystemdNotifyReady(void); +void virSystemdNotifyReload(void); +void virSystemdNotifyStopping(void); +void virSystemdNotifyExtendTimeout(int secs); +void virSystemdNotifyStatus(const char *fmt, ...) G_GNUC_PRINTF(1, 2); int virSystemdHasMachined(void); -- 2.47.1
On Wed, Feb 12, 2025 at 15:46:41 +0000, Daniel P. Berrangé wrote:
We have a way to notify systemd when we're done starting the daemon.
Systemd supports many more notifications, however, and many of them are quite relevant to our needs:
https://www.freedesktop.org/software/systemd/man/latest/sd_notify.html
This renames the existing notification API to better reflect its semantics, and adds new APIs for reporting
* Initiation of config file reload * Initiation of daemon shutdown process * Adhoc progress status messages * Request to extend service shutdown timeout
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/libvirt_private.syms | 6 +++++- src/rpc/virnetdaemon.c | 2 +- src/util/virsystemd.c | 44 +++++++++++++++++++++++++++++++++++++--- src/util/virsystemd.h | 6 +++++- 4 files changed, 52 insertions(+), 6 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6888f6b599..4eca252b2a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3531,7 +3531,11 @@ virSystemdHasResolved; virSystemdHasResolvedResetCachedValue; virSystemdMakeScopeName; virSystemdMakeSliceName; -virSystemdNotifyStartup; +virSystemdNotifyExtendTimeout; +virSystemdNotifyReady; +virSystemdNotifyReload; +virSystemdNotifyStatus; +virSystemdNotifyStopping; virSystemdResolvedRegisterNameServer; virSystemdTerminateMachine;
diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index f647cf60ba..e6bdfe0f88 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -753,7 +753,7 @@ virNetDaemonRun(virNetDaemon *dmn)
/* We are accepting connections now. Notify systemd * so it can start dependent services. */ - virSystemdNotifyStartup(); + virSystemdNotifyReady();
VIR_DEBUG("dmn=%p quit=%d", dmn, dmn->quit); while (!dmn->finished) { diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index d46e5f74fc..bbcf77a41f 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -624,12 +624,11 @@ int virSystemdTerminateMachine(const char *name) return 0; }
-void -virSystemdNotifyStartup(void) +static void +virSystemdNotify(const char *msg) { #ifndef WIN32 const char *path; - const char *msg = "READY=1"; int fd; struct sockaddr_un un = { .sun_family = AF_UNIX, @@ -644,6 +643,8 @@ virSystemdNotifyStartup(void) .msg_iovlen = 1, };
+ VIR_DEBUG("Notify '%s'", msg); + if (!(path = getenv("NOTIFY_SOCKET"))) { VIR_DEBUG("Skipping systemd notify, not requested"); return; @@ -674,6 +675,43 @@ virSystemdNotifyStartup(void) #endif /* !WIN32 */ }
+void virSystemdNotifyReady(void) +{ + virSystemdNotify("READY=1"); +} + +void virSystemdNotifyReload(void) +{ + gint64 now = g_get_monotonic_time(); + g_autofree char *msg = g_strdup_printf( + "RELOADING=1\nMONOTONIC_USEC=%lld", (long long int)now); + virSystemdNotify(msg); +} + +void virSystemdNotifyStopping(void) +{ + virSystemdNotify("STOPPING=1"); +} + +void virSystemdNotifyExtendTimeout(int secs) +{ + g_autofree char *msg = g_strdup_printf("EXTEND_TIMEOUT_USEC=%llu", + secs * 1000ull * 1000ul);
'ull' in the second 1000 Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Switch to the 'notify-reload' service type and send notifications to systemd when reloading configuration. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/remote/libvirtd.service.in | 2 +- src/remote/remote_daemon.c | 2 ++ src/virtd.service.in | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/remote/libvirtd.service.in b/src/remote/libvirtd.service.in index 250b4a6fc3..b0a062e885 100644 --- a/src/remote/libvirtd.service.in +++ b/src/remote/libvirtd.service.in @@ -26,7 +26,7 @@ After=xencommons.service Conflicts=xendomains.service [Service] -Type=notify +Type=notify-reload Environment=LIBVIRTD_ARGS="--timeout 120" EnvironmentFile=-@initconfdir@/libvirtd ExecStart=@sbindir@/libvirtd $LIBVIRTD_ARGS diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index 1d079c7e4b..d44a365000 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -452,9 +452,11 @@ static void daemonReloadHandlerThread(void *opaque G_GNUC_UNUSED) virHookCall(VIR_HOOK_DRIVER_DAEMON, "-", VIR_HOOK_DAEMON_OP_RELOAD, SIGHUP, "SIGHUP", NULL, NULL); + virSystemdNotifyReload(); if (virStateReload() < 0) { VIR_WARN("Error while reloading drivers"); } + virSystemdNotifyReady(); /* Drivers are initialized again. */ g_atomic_int_set(&driversInitialized, 1); diff --git a/src/virtd.service.in b/src/virtd.service.in index 651a8d82d7..7ffb77e339 100644 --- a/src/virtd.service.in +++ b/src/virtd.service.in @@ -15,7 +15,7 @@ After=dbus.service After=apparmor.service [Service] -Type=notify +Type=notify-reload Environment=@SERVICE@_ARGS="--timeout 120" EnvironmentFile=-@initconfdir@/@service@ ExecStart=@sbindir@/@service@ $@SERVICE@_ARGS -- 2.47.1
On Wed, Feb 12, 2025 at 15:46:42 +0000, Daniel P. Berrangé wrote:
Switch to the 'notify-reload' service type and send notifications to systemd when reloading configuration.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/remote/libvirtd.service.in | 2 +- src/remote/remote_daemon.c | 2 ++ src/virtd.service.in | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
There's a common pattern for autostart of iterating over VMs, acquiring a lock and ref count, then checking the autostart & is-active flags. Wrap this all up into a helper method. Reviewed-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/hypervisor/domain_driver.c | 41 ++++++++++++++++++++++++++++++++++ src/hypervisor/domain_driver.h | 17 ++++++++++++++ src/libvirt_private.syms | 1 + 3 files changed, 59 insertions(+) diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index 85d68b056c..596157545b 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -29,9 +29,12 @@ #include "viraccessapicheck.h" #include "datatypes.h" #include "driver.h" +#include "virlog.h" #define VIR_FROM_THIS VIR_FROM_DOMAIN +VIR_LOG_INIT("hypervisor.domain_driver"); + char * virDomainDriverGenerateRootHash(const char *drivername, const char *root) @@ -652,3 +655,41 @@ virDomainDriverGetIOThreadsConfig(virDomainDef *targetDef, return ret; } + +static int +virDomainDriverAutoStartOne(virDomainObj *vm, + void *opaque) +{ + virDomainDriverAutoStartConfig *cfg = opaque; + + virObjectLock(vm); + virObjectRef(vm); + + VIR_DEBUG("Autostart %s: autostart=%d", + vm->def->name, vm->autostart); + + if (vm->autostart && !virDomainObjIsActive(vm)) { + virResetLastError(); + cfg->callback(vm, cfg->opaque); + } + + virDomainObjEndAPI(&vm); + virResetLastError(); + + return 0; +} + +void +virDomainDriverAutoStart(virDomainObjList *domains, + virDomainDriverAutoStartConfig *cfg) +{ + bool autostart; + VIR_DEBUG("Run autostart stateDir=%s", cfg->stateDir); + if (virDriverShouldAutostart(cfg->stateDir, &autostart) < 0 || + !autostart) { + VIR_DEBUG("Autostart already processed"); + return; + } + + virDomainObjListForEach(domains, false, virDomainDriverAutoStartOne, cfg); +} diff --git a/src/hypervisor/domain_driver.h b/src/hypervisor/domain_driver.h index 9942f58fda..2ad4e96f3e 100644 --- a/src/hypervisor/domain_driver.h +++ b/src/hypervisor/domain_driver.h @@ -23,6 +23,7 @@ #include "node_device_conf.h" #include "virhostdev.h" #include "virpci.h" +#include "virdomainobjlist.h" char * virDomainDriverGenerateRootHash(const char *drivername, @@ -71,3 +72,19 @@ int virDomainDriverDelIOThreadCheck(virDomainDef *def, int virDomainDriverGetIOThreadsConfig(virDomainDef *targetDef, virDomainIOThreadInfoPtr **info, unsigned int bitmap_size); + +/* + * Will be called with 'vm' locked and ref count held, + * which will be released when this returns. + */ +typedef void (*virDomainDriverAutoStartCallback)(virDomainObj *vm, + void *opaque); + +typedef struct _virDomainDriverAutoStartConfig { + const char *stateDir; + virDomainDriverAutoStartCallback callback; + void *opaque; +} virDomainDriverAutoStartConfig; + +void virDomainDriverAutoStart(virDomainObjList *domains, + virDomainDriverAutoStartConfig *cfg); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 4eca252b2a..bd9564f2a7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1647,6 +1647,7 @@ virDomainCgroupSetupVcpuBW; # hypervisor/domain_driver.h virDomainDriverAddIOThreadCheck; +virDomainDriverAutoStart; virDomainDriverDelIOThreadCheck; virDomainDriverGenerateMachineName; virDomainDriverGenerateRootHash; -- 2.47.1
This eliminates some duplicated code patterns aross drivers. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/bhyve/bhyve_driver.c | 41 ++++++++++++------------------ src/libxl/libxl_driver.c | 36 ++++++++------------------- src/lxc/lxc_driver.c | 13 +++++----- src/lxc/lxc_process.c | 18 ++------------ src/lxc/lxc_process.h | 2 ++ src/qemu/qemu_driver.c | 54 ++++++++++++---------------------------- 6 files changed, 53 insertions(+), 111 deletions(-) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index f6b0417014..4d83a0dd8a 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -54,6 +54,7 @@ #include "virportallocator.h" #include "conf/domain_capabilities.h" #include "virutil.h" +#include "domain_driver.h" #include "bhyve_conf.h" #include "bhyve_device.h" @@ -70,29 +71,18 @@ VIR_LOG_INIT("bhyve.bhyve_driver"); struct _bhyveConn *bhyve_driver = NULL; -static int +static void bhyveAutostartDomain(virDomainObj *vm, void *opaque G_GNUC_UNUSED) { int ret = 0; - VIR_LOCK_GUARD lock = virObjectLockGuard(vm); - - if (vm->autostart && !virDomainObjIsActive(vm)) { - virResetLastError(); - ret = virBhyveProcessStart(bhyve_driver, NULL, vm, - VIR_DOMAIN_RUNNING_BOOTED, 0); - if (ret < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to autostart VM '%1$s': %2$s"), - vm->def->name, virGetLastErrorMessage()); - } - } - return ret; -} -static void -bhyveAutostartDomains(struct _bhyveConn *driver) -{ - virDomainObjListForEach(driver->domains, false, bhyveAutostartDomain, NULL); + ret = virBhyveProcessStart(bhyve_driver, NULL, vm, + VIR_DOMAIN_RUNNING_BOOTED, 0); + if (ret < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to autostart VM '%1$s': %2$s"), + vm->def->name, virGetLastErrorMessage()); + } } /** @@ -1168,7 +1158,7 @@ bhyveStateInitialize(bool privileged, virStateInhibitCallback callback G_GNUC_UNUSED, void *opaque G_GNUC_UNUSED) { - bool autostart = true; + virDomainDriverAutoStartConfig autostartCfg; if (root != NULL) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -1253,11 +1243,12 @@ bhyveStateInitialize(bool privileged, virBhyveProcessReconnectAll(bhyve_driver); - if (virDriverShouldAutostart(BHYVE_STATE_DIR, &autostart) < 0) - goto cleanup; - - if (autostart) - bhyveAutostartDomains(bhyve_driver); + autostartCfg = (virDomainDriverAutoStartConfig) { + .stateDir = BHYVE_STATE_DIR, + .callback = bhyveAutostartDomain, + .opaque = bhyve_driver, + }; + virDomainDriverAutoStart(bhyve_driver->domains, &autostartCfg); return VIR_DRV_STATE_INIT_COMPLETE; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index a76545c9ff..bd858d8127 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -315,36 +315,22 @@ libxlDomObjFromDomain(virDomainPtr dom) return vm; } -static int +static void libxlAutostartDomain(virDomainObj *vm, void *opaque) { libxlDriverPrivate *driver = opaque; - int ret = -1; - - virObjectRef(vm); - virObjectLock(vm); - virResetLastError(); if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0) - goto cleanup; + return; - if (vm->autostart && !virDomainObjIsActive(vm) && - libxlDomainStartNew(driver, vm, false) < 0) { + if (libxlDomainStartNew(driver, vm, false) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to autostart VM '%1$s': %2$s"), vm->def->name, virGetLastErrorMessage()); - goto endjob; } - ret = 0; - - endjob: virDomainObjEndJob(vm); - cleanup: - virDomainObjEndAPI(&vm); - - return ret; } @@ -654,7 +640,7 @@ libxlStateInitialize(bool privileged, { libxlDriverConfig *cfg; g_autofree char *driverConf = NULL; - bool autostart = true; + virDomainDriverAutoStartConfig autostartCfg; if (root != NULL) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -807,14 +793,12 @@ libxlStateInitialize(bool privileged, NULL, NULL) < 0) goto error; - if (virDriverShouldAutostart(cfg->stateDir, &autostart) < 0) - goto error; - - if (autostart) { - virDomainObjListForEach(libxl_driver->domains, false, - libxlAutostartDomain, - libxl_driver); - } + autostartCfg = (virDomainDriverAutoStartConfig) { + .stateDir = cfg->stateDir, + .callback = libxlAutostartDomain, + .opaque = libxl_driver, + }; + virDomainDriverAutoStart(libxl_driver->domains, &autostartCfg); virDomainObjListForEach(libxl_driver->domains, false, libxlDomainManagedSaveLoad, diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 4740aeed52..e63732dbea 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1402,8 +1402,8 @@ lxcStateInitialize(bool privileged, void *opaque) { virLXCDriverConfig *cfg = NULL; - bool autostart = true; const char *defsecmodel; + virDomainDriverAutoStartConfig autostartCfg; if (root != NULL) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -1499,11 +1499,12 @@ lxcStateInitialize(bool privileged, NULL, NULL) < 0) goto cleanup; - if (virDriverShouldAutostart(cfg->stateDir, &autostart) < 0) - goto cleanup; - - if (autostart) - virLXCProcessAutostartAll(lxc_driver); + autostartCfg = (virDomainDriverAutoStartConfig) { + .stateDir = cfg->stateDir, + .callback = virLXCProcessAutostartDomain, + .opaque = NULL, + }; + virDomainDriverAutoStart(lxc_driver->domains, &autostartCfg); return VIR_DRV_STATE_INIT_COMPLETE; diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index d785244dde..1bca9e8dae 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -1562,19 +1562,14 @@ int virLXCProcessStart(virLXCDriver * driver, } -static int +void virLXCProcessAutostartDomain(virDomainObj *vm, void *opaque G_GNUC_UNUSED) { - VIR_LOCK_GUARD lock = virObjectLockGuard(vm); virLXCDomainObjPrivate *priv = vm->privateData; virObjectEvent *event; int rc = 0; - if (!vm->autostart || - virDomainObjIsActive(vm)) - return 0; - rc = virLXCProcessStart(priv->driver, vm, 0, NULL, NULL, VIR_DOMAIN_RUNNING_BOOTED); virDomainAuditStart(vm, "booted", rc >= 0); @@ -1582,22 +1577,13 @@ virLXCProcessAutostartDomain(virDomainObj *vm, VIR_ERROR(_("Failed to autostart VM '%1$s': %2$s"), vm->def->name, virGetLastErrorMessage()); - return -1; + return; } event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED, VIR_DOMAIN_EVENT_STARTED_BOOTED); virObjectEventStateQueue(priv->driver->domainEventState, event); - - return 0; -} - - -void -virLXCProcessAutostartAll(virLXCDriver *driver) -{ - virDomainObjListForEach(driver->domains, false, virLXCProcessAutostartDomain, NULL); } diff --git a/src/lxc/lxc_process.h b/src/lxc/lxc_process.h index 95eacdd1e5..fc464690f8 100644 --- a/src/lxc/lxc_process.h +++ b/src/lxc/lxc_process.h @@ -34,6 +34,8 @@ int virLXCProcessStop(virLXCDriver *driver, unsigned int cleanupFlags); void virLXCProcessAutostartAll(virLXCDriver *driver); +void virLXCProcessAutostartDomain(virDomainObj *vm, + void *opaque); int virLXCProcessReconnectAll(virLXCDriver *driver, virDomainObjList *doms); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 78bfaa5b3a..0a4e423e70 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -167,52 +167,29 @@ qemuDomObjFromSnapshot(virDomainSnapshotPtr snapshot) -static int +static void qemuAutostartDomain(virDomainObj *vm, void *opaque) { virQEMUDriver *driver = opaque; int flags = 0; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - int ret = -1; if (cfg->autoStartBypassCache) flags |= VIR_DOMAIN_START_BYPASS_CACHE; - virObjectLock(vm); - virObjectRef(vm); - virResetLastError(); - if (vm->autostart && - !virDomainObjIsActive(vm)) { - if (qemuProcessBeginJob(vm, VIR_DOMAIN_JOB_OPERATION_START, - flags) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to start job on VM '%1$s': %2$s"), - vm->def->name, virGetLastErrorMessage()); - goto cleanup; - } + if (qemuProcessBeginJob(vm, VIR_DOMAIN_JOB_OPERATION_START, + flags) < 0) + return; - if (qemuDomainObjStart(NULL, driver, vm, flags, - VIR_ASYNC_JOB_START) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to autostart VM '%1$s': %2$s"), + if (qemuDomainObjStart(NULL, driver, vm, flags, + VIR_ASYNC_JOB_START) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to autostart VM '%1$s': %2$s"), vm->def->name, virGetLastErrorMessage()); - } - - qemuProcessEndJob(vm); } - ret = 0; - cleanup: - virDomainObjEndAPI(&vm); - return ret; -} - - -static void -qemuAutostartDomains(virQEMUDriver *driver) -{ - virDomainObjListForEach(driver->domains, false, qemuAutostartDomain, driver); + qemuProcessEndJob(vm); } @@ -557,10 +534,10 @@ qemuStateInitialize(bool privileged, virQEMUDriverConfig *cfg; uid_t run_uid = -1; gid_t run_gid = -1; - bool autostart = true; size_t i; const char *defsecmodel = NULL; g_autoptr(virIdentity) identity = virIdentityGetCurrent(); + virDomainDriverAutoStartConfig autostartCfg; qemu_driver = g_new0(virQEMUDriver, 1); @@ -906,11 +883,12 @@ qemuStateInitialize(bool privileged, qemuProcessReconnectAll(qemu_driver); - if (virDriverShouldAutostart(cfg->stateDir, &autostart) < 0) - goto error; - - if (autostart) - qemuAutostartDomains(qemu_driver); + autostartCfg = (virDomainDriverAutoStartConfig) { + .stateDir = cfg->stateDir, + .callback = qemuAutostartDomain, + .opaque = qemu_driver, + }; + virDomainDriverAutoStart(qemu_driver->domains, &autostartCfg); return VIR_DRV_STATE_INIT_COMPLETE; -- 2.47.1
On Wed, Feb 12, 2025 at 03:46:44PM +0000, Daniel P. Berrangé wrote:
This eliminates some duplicated code patterns aross drivers.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/bhyve/bhyve_driver.c | 41 ++++++++++++------------------ src/libxl/libxl_driver.c | 36 ++++++++------------------- src/lxc/lxc_driver.c | 13 +++++----- src/lxc/lxc_process.c | 18 ++------------ src/lxc/lxc_process.h | 2 ++ src/qemu/qemu_driver.c | 54 ++++++++++++---------------------------- 6 files changed, 53 insertions(+), 111 deletions(-)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index f6b0417014..4d83a0dd8a 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -54,6 +54,7 @@ #include "virportallocator.h" #include "conf/domain_capabilities.h" #include "virutil.h" +#include "domain_driver.h"
#include "bhyve_conf.h" #include "bhyve_device.h" @@ -70,29 +71,18 @@ VIR_LOG_INIT("bhyve.bhyve_driver");
struct _bhyveConn *bhyve_driver = NULL;
-static int +static void bhyveAutostartDomain(virDomainObj *vm, void *opaque G_GNUC_UNUSED) { int ret = 0; - VIR_LOCK_GUARD lock = virObjectLockGuard(vm); - - if (vm->autostart && !virDomainObjIsActive(vm)) { - virResetLastError(); - ret = virBhyveProcessStart(bhyve_driver, NULL, vm, - VIR_DOMAIN_RUNNING_BOOTED, 0); - if (ret < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to autostart VM '%1$s': %2$s"), - vm->def->name, virGetLastErrorMessage()); - } - } - return ret; -}
-static void -bhyveAutostartDomains(struct _bhyveConn *driver) -{ - virDomainObjListForEach(driver->domains, false, bhyveAutostartDomain, NULL); + ret = virBhyveProcessStart(bhyve_driver, NULL, vm, + VIR_DOMAIN_RUNNING_BOOTED, 0); + if (ret < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to autostart VM '%1$s': %2$s"), + vm->def->name, virGetLastErrorMessage()); + } }
/** @@ -1168,7 +1158,7 @@ bhyveStateInitialize(bool privileged, virStateInhibitCallback callback G_GNUC_UNUSED, void *opaque G_GNUC_UNUSED) { - bool autostart = true; + virDomainDriverAutoStartConfig autostartCfg;
if (root != NULL) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -1253,11 +1243,12 @@ bhyveStateInitialize(bool privileged,
virBhyveProcessReconnectAll(bhyve_driver);
- if (virDriverShouldAutostart(BHYVE_STATE_DIR, &autostart) < 0) - goto cleanup; - - if (autostart) - bhyveAutostartDomains(bhyve_driver); + autostartCfg = (virDomainDriverAutoStartConfig) { + .stateDir = BHYVE_STATE_DIR, + .callback = bhyveAutostartDomain, + .opaque = bhyve_driver, + };
Tieing into your earlier patch comment, I pass bhyve_driver here, but then don't bother to use 'opaque' in the actual method, accessing bhyve_driver again. Guess I should really use opaque to be consistent with the other drivers
+ virDomainDriverAutoStart(bhyve_driver->domains, &autostartCfg);
return VIR_DRV_STATE_INIT_COMPLETE;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index a76545c9ff..bd858d8127 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -315,36 +315,22 @@ libxlDomObjFromDomain(virDomainPtr dom) return vm; }
-static int +static void libxlAutostartDomain(virDomainObj *vm, void *opaque) { libxlDriverPrivate *driver = opaque; - int ret = -1; - - virObjectRef(vm); - virObjectLock(vm); - virResetLastError();
if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0) - goto cleanup; + return;
- if (vm->autostart && !virDomainObjIsActive(vm) && - libxlDomainStartNew(driver, vm, false) < 0) { + if (libxlDomainStartNew(driver, vm, false) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to autostart VM '%1$s': %2$s"), vm->def->name, virGetLastErrorMessage()); - goto endjob; }
- ret = 0; - - endjob: virDomainObjEndJob(vm); - cleanup: - virDomainObjEndAPI(&vm); - - return ret; }
@@ -654,7 +640,7 @@ libxlStateInitialize(bool privileged, { libxlDriverConfig *cfg; g_autofree char *driverConf = NULL; - bool autostart = true; + virDomainDriverAutoStartConfig autostartCfg;
if (root != NULL) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -807,14 +793,12 @@ libxlStateInitialize(bool privileged, NULL, NULL) < 0) goto error;
- if (virDriverShouldAutostart(cfg->stateDir, &autostart) < 0) - goto error; - - if (autostart) { - virDomainObjListForEach(libxl_driver->domains, false, - libxlAutostartDomain, - libxl_driver); - } + autostartCfg = (virDomainDriverAutoStartConfig) { + .stateDir = cfg->stateDir, + .callback = libxlAutostartDomain, + .opaque = libxl_driver, + }; + virDomainDriverAutoStart(libxl_driver->domains, &autostartCfg);
virDomainObjListForEach(libxl_driver->domains, false, libxlDomainManagedSaveLoad, diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 4740aeed52..e63732dbea 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1402,8 +1402,8 @@ lxcStateInitialize(bool privileged, void *opaque) { virLXCDriverConfig *cfg = NULL; - bool autostart = true; const char *defsecmodel; + virDomainDriverAutoStartConfig autostartCfg;
if (root != NULL) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -1499,11 +1499,12 @@ lxcStateInitialize(bool privileged, NULL, NULL) < 0) goto cleanup;
- if (virDriverShouldAutostart(cfg->stateDir, &autostart) < 0) - goto cleanup; - - if (autostart) - virLXCProcessAutostartAll(lxc_driver); + autostartCfg = (virDomainDriverAutoStartConfig) { + .stateDir = cfg->stateDir, + .callback = virLXCProcessAutostartDomain, + .opaque = NULL, + }; + virDomainDriverAutoStart(lxc_driver->domains, &autostartCfg);
return VIR_DRV_STATE_INIT_COMPLETE;
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index d785244dde..1bca9e8dae 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -1562,19 +1562,14 @@ int virLXCProcessStart(virLXCDriver * driver, }
-static int +void virLXCProcessAutostartDomain(virDomainObj *vm, void *opaque G_GNUC_UNUSED) { - VIR_LOCK_GUARD lock = virObjectLockGuard(vm); virLXCDomainObjPrivate *priv = vm->privateData; virObjectEvent *event; int rc = 0;
- if (!vm->autostart || - virDomainObjIsActive(vm)) - return 0; - rc = virLXCProcessStart(priv->driver, vm, 0, NULL, NULL, VIR_DOMAIN_RUNNING_BOOTED); virDomainAuditStart(vm, "booted", rc >= 0);
@@ -1582,22 +1577,13 @@ virLXCProcessAutostartDomain(virDomainObj *vm, VIR_ERROR(_("Failed to autostart VM '%1$s': %2$s"), vm->def->name, virGetLastErrorMessage()); - return -1; + return; }
event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED, VIR_DOMAIN_EVENT_STARTED_BOOTED); virObjectEventStateQueue(priv->driver->domainEventState, event); - - return 0; -} - - -void -virLXCProcessAutostartAll(virLXCDriver *driver) -{ - virDomainObjListForEach(driver->domains, false, virLXCProcessAutostartDomain, NULL); }
diff --git a/src/lxc/lxc_process.h b/src/lxc/lxc_process.h index 95eacdd1e5..fc464690f8 100644 --- a/src/lxc/lxc_process.h +++ b/src/lxc/lxc_process.h @@ -34,6 +34,8 @@ int virLXCProcessStop(virLXCDriver *driver, unsigned int cleanupFlags);
void virLXCProcessAutostartAll(virLXCDriver *driver); +void virLXCProcessAutostartDomain(virDomainObj *vm, + void *opaque); int virLXCProcessReconnectAll(virLXCDriver *driver, virDomainObjList *doms);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 78bfaa5b3a..0a4e423e70 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -167,52 +167,29 @@ qemuDomObjFromSnapshot(virDomainSnapshotPtr snapshot)
-static int +static void qemuAutostartDomain(virDomainObj *vm, void *opaque) { virQEMUDriver *driver = opaque; int flags = 0; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - int ret = -1;
if (cfg->autoStartBypassCache) flags |= VIR_DOMAIN_START_BYPASS_CACHE;
- virObjectLock(vm); - virObjectRef(vm); - virResetLastError(); - if (vm->autostart && - !virDomainObjIsActive(vm)) { - if (qemuProcessBeginJob(vm, VIR_DOMAIN_JOB_OPERATION_START, - flags) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to start job on VM '%1$s': %2$s"), - vm->def->name, virGetLastErrorMessage()); - goto cleanup; - } + if (qemuProcessBeginJob(vm, VIR_DOMAIN_JOB_OPERATION_START, + flags) < 0) + return;
- if (qemuDomainObjStart(NULL, driver, vm, flags, - VIR_ASYNC_JOB_START) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to autostart VM '%1$s': %2$s"), + if (qemuDomainObjStart(NULL, driver, vm, flags, + VIR_ASYNC_JOB_START) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to autostart VM '%1$s': %2$s"), vm->def->name, virGetLastErrorMessage()); - } - - qemuProcessEndJob(vm); }
- ret = 0; - cleanup: - virDomainObjEndAPI(&vm); - return ret; -} - - -static void -qemuAutostartDomains(virQEMUDriver *driver) -{ - virDomainObjListForEach(driver->domains, false, qemuAutostartDomain, driver); + qemuProcessEndJob(vm); }
@@ -557,10 +534,10 @@ qemuStateInitialize(bool privileged, virQEMUDriverConfig *cfg; uid_t run_uid = -1; gid_t run_gid = -1; - bool autostart = true; size_t i; const char *defsecmodel = NULL; g_autoptr(virIdentity) identity = virIdentityGetCurrent(); + virDomainDriverAutoStartConfig autostartCfg;
qemu_driver = g_new0(virQEMUDriver, 1);
@@ -906,11 +883,12 @@ qemuStateInitialize(bool privileged,
qemuProcessReconnectAll(qemu_driver);
- if (virDriverShouldAutostart(cfg->stateDir, &autostart) < 0) - goto error; - - if (autostart) - qemuAutostartDomains(qemu_driver); + autostartCfg = (virDomainDriverAutoStartConfig) { + .stateDir = cfg->stateDir, + .callback = qemuAutostartDomain, + .opaque = qemu_driver, + }; + virDomainDriverAutoStart(qemu_driver->domains, &autostartCfg);
return VIR_DRV_STATE_INIT_COMPLETE;
-- 2.47.1
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Wed, Feb 12, 2025 at 15:46:44 +0000, Daniel P. Berrangé wrote:
This eliminates some duplicated code patterns aross drivers.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/bhyve/bhyve_driver.c | 41 ++++++++++++------------------ src/libxl/libxl_driver.c | 36 ++++++++------------------- src/lxc/lxc_driver.c | 13 +++++----- src/lxc/lxc_process.c | 18 ++------------ src/lxc/lxc_process.h | 2 ++ src/qemu/qemu_driver.c | 54 ++++++++++++---------------------------- 6 files changed, 53 insertions(+), 111 deletions(-)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index f6b0417014..4d83a0dd8a 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -54,6 +54,7 @@ #include "virportallocator.h" #include "conf/domain_capabilities.h" #include "virutil.h" +#include "domain_driver.h"
#include "bhyve_conf.h" #include "bhyve_device.h" @@ -70,29 +71,18 @@ VIR_LOG_INIT("bhyve.bhyve_driver");
struct _bhyveConn *bhyve_driver = NULL;
-static int +static void bhyveAutostartDomain(virDomainObj *vm, void *opaque G_GNUC_UNUSED) {
So ultimately opaque will be set to 'bhyve_driver'.
int ret = 0; - VIR_LOCK_GUARD lock = virObjectLockGuard(vm); - - if (vm->autostart && !virDomainObjIsActive(vm)) { - virResetLastError(); - ret = virBhyveProcessStart(bhyve_driver, NULL, vm, - VIR_DOMAIN_RUNNING_BOOTED, 0); - if (ret < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to autostart VM '%1$s': %2$s"), - vm->def->name, virGetLastErrorMessage()); - } - } - return ret; -}
-static void -bhyveAutostartDomains(struct _bhyveConn *driver) -{ - virDomainObjListForEach(driver->domains, false, bhyveAutostartDomain, NULL); + ret = virBhyveProcessStart(bhyve_driver, NULL, vm,
... so you could use that instead ...
+ VIR_DOMAIN_RUNNING_BOOTED, 0); + if (ret < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to autostart VM '%1$s': %2$s"), + vm->def->name, virGetLastErrorMessage()); + } }
/** @@ -1168,7 +1158,7 @@ bhyveStateInitialize(bool privileged, virStateInhibitCallback callback G_GNUC_UNUSED, void *opaque G_GNUC_UNUSED) { - bool autostart = true; + virDomainDriverAutoStartConfig autostartCfg;
if (root != NULL) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -1253,11 +1243,12 @@ bhyveStateInitialize(bool privileged,
virBhyveProcessReconnectAll(bhyve_driver);
- if (virDriverShouldAutostart(BHYVE_STATE_DIR, &autostart) < 0) - goto cleanup; - - if (autostart) - bhyveAutostartDomains(bhyve_driver); + autostartCfg = (virDomainDriverAutoStartConfig) { + .stateDir = BHYVE_STATE_DIR, + .callback = bhyveAutostartDomain, + .opaque = bhyve_driver,
... so that this is not useless.
+ }; + virDomainDriverAutoStart(bhyve_driver->domains, &autostartCfg);
return VIR_DRV_STATE_INIT_COMPLETE;
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This delay can reduce the CPU/IO load storm when autostarting many guests. Reviewed-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/hypervisor/domain_driver.c | 20 +++++++++++++++++--- src/hypervisor/domain_driver.h | 2 ++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index 596157545b..bea84cd09d 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -656,11 +656,16 @@ virDomainDriverGetIOThreadsConfig(virDomainDef *targetDef, return ret; } +typedef struct _virDomainDriverAutoStartState { + virDomainDriverAutoStartConfig *cfg; + bool first; +} virDomainDriverAutoStartState; + static int virDomainDriverAutoStartOne(virDomainObj *vm, void *opaque) { - virDomainDriverAutoStartConfig *cfg = opaque; + virDomainDriverAutoStartState *state = opaque; virObjectLock(vm); virObjectRef(vm); @@ -670,7 +675,15 @@ virDomainDriverAutoStartOne(virDomainObj *vm, if (vm->autostart && !virDomainObjIsActive(vm)) { virResetLastError(); - cfg->callback(vm, cfg->opaque); + if (state->cfg->delayMS) { + if (!state->first) { + g_usleep(state->cfg->delayMS * 1000ull); + } else { + state->first = false; + } + } + + state->cfg->callback(vm, state->cfg->opaque); } virDomainObjEndAPI(&vm); @@ -683,6 +696,7 @@ void virDomainDriverAutoStart(virDomainObjList *domains, virDomainDriverAutoStartConfig *cfg) { + virDomainDriverAutoStartState state = { .cfg = cfg, .first = true }; bool autostart; VIR_DEBUG("Run autostart stateDir=%s", cfg->stateDir); if (virDriverShouldAutostart(cfg->stateDir, &autostart) < 0 || @@ -691,5 +705,5 @@ virDomainDriverAutoStart(virDomainObjList *domains, return; } - virDomainObjListForEach(domains, false, virDomainDriverAutoStartOne, cfg); + virDomainObjListForEach(domains, false, virDomainDriverAutoStartOne, &state); } diff --git a/src/hypervisor/domain_driver.h b/src/hypervisor/domain_driver.h index 2ad4e96f3e..f81d436c2c 100644 --- a/src/hypervisor/domain_driver.h +++ b/src/hypervisor/domain_driver.h @@ -84,6 +84,8 @@ typedef struct _virDomainDriverAutoStartConfig { const char *stateDir; virDomainDriverAutoStartCallback callback; void *opaque; + unsigned int delayMS; /* milliseconds to wait between initiating the + * startup of each guest */ } virDomainDriverAutoStartConfig; void virDomainDriverAutoStart(virDomainObjList *domains, -- 2.47.1
This allows a user specified delay between autostart of each VM, giving parity with the equivalent feature of libvirt-guests. Reviewed-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf.in | 5 +++++ src/qemu/qemu_conf.c | 2 ++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 1 + src/qemu/test_libvirtd_qemu.aug.in | 1 + 6 files changed, 11 insertions(+) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index 1377fd89cc..642093c40b 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -97,6 +97,7 @@ module Libvirtd_qemu = | str_entry "auto_dump_path" | bool_entry "auto_dump_bypass_cache" | bool_entry "auto_start_bypass_cache" + | int_entry "auto_start_delay" let process_entry = str_entry "hugetlbfs_mount" | str_entry "bridge_helper" diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in index d853136f10..31172303dc 100644 --- a/src/qemu/qemu.conf.in +++ b/src/qemu/qemu.conf.in @@ -634,6 +634,11 @@ # #auto_start_bypass_cache = 0 +# Delay in milliseconds between initiating the startup for +# each VM, during autostart +# +#auto_start_delay = 0 + # If provided by the host and a hugetlbfs mount point is configured, # a guest may request huge page backing. When this mount point is # unspecified here, determination of a host mount point in /proc/mounts diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 8b9fe4e381..b73dda7e10 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -638,6 +638,8 @@ virQEMUDriverConfigLoadSaveEntry(virQEMUDriverConfig *cfg, return -1; if (virConfGetValueBool(conf, "auto_start_bypass_cache", &cfg->autoStartBypassCache) < 0) return -1; + if (virConfGetValueUInt(conf, "auto_start_delay", &cfg->autoStartDelayMS) < 0) + return -1; return 0; } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 42cdb6f883..97214f72d0 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -200,6 +200,7 @@ struct _virQEMUDriverConfig { char *autoDumpPath; bool autoDumpBypassCache; bool autoStartBypassCache; + unsigned int autoStartDelayMS; char *lockManagerName; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0a4e423e70..1bdf3c1622 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -887,6 +887,7 @@ qemuStateInitialize(bool privileged, .stateDir = cfg->stateDir, .callback = qemuAutostartDomain, .opaque = qemu_driver, + .delayMS = cfg->autoStartDelayMS, }; virDomainDriverAutoStart(qemu_driver->domains, &autostartCfg); diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index 69fdae215a..c2a1d7d829 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -75,6 +75,7 @@ module Test_libvirtd_qemu = { "auto_dump_path" = "/var/lib/libvirt/qemu/dump" } { "auto_dump_bypass_cache" = "0" } { "auto_start_bypass_cache" = "0" } +{ "auto_start_delay" = "0" } { "hugetlbfs_mount" = "/dev/hugepages" } { "bridge_helper" = "qemu-bridge-helper" } { "set_process_name" = "1" } -- 2.47.1
participants (2)
-
Daniel P. Berrangé -
Peter Krempa