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@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:
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@xxxxxxxxxx>
>
> 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 :|
