Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
src/bhyve/bhyve_driver.c | 53 ++++++++++++---------------------------
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(+), 123 deletions(-)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index 8f97ac032c..ec997a38f5 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -71,41 +71,19 @@ 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(data->conn, 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)
-{
- /* 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);
+ ret = virBhyveProcessStart(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());
+ }
- virObjectUnref(conn);
+ return ret;
}
/**
@@ -1181,7 +1159,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",
@@ -1266,11 +1244,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 494b1ad9bc..b6e17b47de 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 c2982244f0..4e152c193c 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 d2eddbd9ae..45bfbd3727 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.1On Wed, Jan 08, 2025 at 19:42:37 +0000, Daniel P. Berrangé wrote:
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> src/bhyve/bhyve_driver.c | 53 ++++++++++++---------------------------
> 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(+), 123 deletions(-)
>
> diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
> index 8f97ac032c..ec997a38f5 100644
> --- a/src/bhyve/bhyve_driver.c
> +++ b/src/bhyve/bhyve_driver.c
> @@ -71,41 +71,19 @@ 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(data->conn, 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)
> -{
> - /* 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);
> + ret = virBhyveProcessStart(NULL, vm,
> + VIR_DOMAIN_RUNNING_BOOTED, 0);
int
virBhyveProcessStart(virConnectPtr conn,
virDomainObj *vm,
virDomainRunningReason reason,
unsigned int flags)
{
struct _bhyveConn *driver = conn->privateData;
The first thing that virBhyveProcessStart() does is to dereference
'conn'. So firstly the note about NULL con being "mostly harmless" is
not true and we also can't do this here in the current state.
[...]
The rest looks good. If you plan to fix the bhyve stuff separately (e.g.
drop 'conn') and what remains in this patch will be a trivial change,
you can add:
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Here.
On Thu, Jan 30, 2025 at 01:08:00PM +0100, Peter Krempa wrote:
> On Wed, Jan 08, 2025 at 19:42:37 +0000, Daniel P. Berrangé wrote:
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> > src/bhyve/bhyve_driver.c | 53 ++++++++++++---------------------------
> > 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(+), 123 deletions(-)
> >
> > diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
> > index 8f97ac032c..ec997a38f5 100644
> > --- a/src/bhyve/bhyve_driver.c
> > +++ b/src/bhyve/bhyve_driver.c
> > @@ -71,41 +71,19 @@ 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(data->conn, 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)
> > -{
> > - /* 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);
> > + ret = virBhyveProcessStart(NULL, vm,
> > + VIR_DOMAIN_RUNNING_BOOTED, 0);
>
>
> int
> virBhyveProcessStart(virConnectPtr conn,
> virDomainObj *vm,
> virDomainRunningReason reason,
> unsigned int flags)
> {
> struct _bhyveConn *driver = conn->privateData;
>
>
> The first thing that virBhyveProcessStart() does is to dereference
> 'conn'. So firstly the note about NULL con being "mostly harmless" is
> not true and we also can't do this here in the current state.
>
>
> [...]
>
> The rest looks good. If you plan to fix the bhyve stuff separately (e.g.
> drop 'conn') and what remains in this patch will be a trivial change,
> you can add:
Urgh, don't know why I didn't check that more closely.
We should just pass 'driver' into virBhyveProcessStart directly, since
all the callers have access to it already.
>
> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
>
> Here.
>
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 :|
© 2016 - 2026 Red Hat, Inc.