On Wed, Jan 08, 2025 at 19:42:37 +0000, Daniel P. Berrangé wrote:
> Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
Here.
