Wojciech Macek wrote:
> Sure, I will split it into two separate commits. I must have messed up sth
> in commits, since even in head-email I described 3 patches...
That would be good, thanks!
> Regarding NULL, it is a little tricky. After running
> virDomainObjListRemove, the vm pointer has 0 references. Then, running
> virObjectUnlock causes libvirt segfault. To minimize changes, I used the
> solution from qemu: set vm to null and then check it inside cleanup. The
> "if (vm)" is just aesthetic - without it there was an awful warning on the
> console about null-pointer. I'd like to leave it as is, or remove
> "vm=NULL;" and insert virObjectRef/virObjectUnref pair instead.
Yeah, looks like you're right, sorry for the confusion.
> What do you mean by ACL check? Do you think the check is unnecessary, or
> just should be placed earlier?
As for the ACL...
> > > +static virDomainPtr
> > > +bhyveDomainCreateXML(virConnectPtr conn,
> > > + const char *xml,
> > > + unsigned int flags)
> > > +{
> > > + bhyveConnPtr privconn = conn->privateData;
> > > + virDomainPtr dom = NULL;
> > > + virDomainDefPtr def = NULL;
> > > + virDomainObjPtr vm = NULL;
> > > + virCapsPtr caps = NULL;
> > > + int ret;
> > > + unsigned int start_flags = 0;
> > > +
> > > + virCheckFlags(VIR_DOMAIN_START_AUTODESTROY, NULL);
> > > +
> > > + if (flags & VIR_DOMAIN_START_AUTODESTROY)
> > > + start_flags |= VIR_BHYVE_PROCESS_START_AUTODESTROY;
> > > +
> > > + caps = bhyveDriverGetCapabilities(privconn);
> > > + if (!caps)
> > > + return NULL;
> > > +
> > > + if ((def = virDomainDefParseString(xml, caps, privconn->xmlopt,
> > > + 1 << VIR_DOMAIN_VIRT_BHYVE,
> > > + VIR_DOMAIN_XML_INACTIVE)) ==
> > NULL)
> > > + goto cleanup;
> > > +
> > > + if (virDomainCreateXMLEnsureACL(conn, def) < 0)
We check it here...
> > > + goto cleanup;
> > > +
> > > + if (!(vm = virDomainObjListAdd(privconn->domains, def,
> > > + privconn->xmlopt,
> > > + 0, NULL)))
> > > + goto cleanup;
> > > + def = NULL;
> > > + vm->persistent = 0;
> > > +
> > > + dom = virGetDomain(conn, vm->def->name, vm->def->uuid);
> > > + if (!dom)
> > > + goto cleanup;
> > > +
> > > + dom->id = vm->def->id;
> > > +
> > > + if (flags & VIR_DOMAIN_START_AUTODESTROY)
> > > + start_flags |= VIR_BHYVE_PROCESS_START_AUTODESTROY;
> > > +
> > > + if (virDomainCreateWithFlagsEnsureACL(dom->conn, vm->def) < 0)
> > > + goto cleanup;
Do we need it here again?
Roman Bogorodskiy
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list