Re: [libvirt] [PATCH 2/2] bhyve: domainCreateXML
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