Oh, yep. We don't need an ACL again. I will send new patchset today. Thanks, Wojtek
2014-04-08 12:09 GMT+02:00 Roman Bogorodskiy <bogorods...@gmail.com>: > 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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list