<div dir="ltr"><div>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... 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. </div><div>What do you mean by ACL check? Do you think the check is unnecessary, or just should be placed earlier? </div><div>W. </div> </div><div class="gmail_extra"> <div class="gmail_quote"> 2014-04-08 9:01 GMT+02:00 Roman Bogorodskiy <<a href="mailto:bogorodskiy@gmail.com" target="_blank">bogorodskiy@gmail.com</a>>: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Wojciech Macek wrote: > Implement bhyveDomainCreteXML function. s/Crete/Create/ <div class=""> > src/bhyve/bhyve_driver.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 80 insertions(+), 1 deletion(-) > > diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c > index f7a8912..2ea2d22 100644 > --- a/src/bhyve/bhyve_driver.c > +++ b/src/bhyve/bhyve_driver.c > @@ -756,8 +756,14 @@ bhyveDomainDestroy(virDomainPtr dom) > > ret = virBhyveProcessStop(privconn, vm, VIR_DOMAIN_SHUTOFF_DESTROYED); > > + if (!vm->persistent) { > + virDomainObjListRemove(privconn->domains, vm); > + vm = NULL; > + } > + </div>This chunk doesn't seem to be directly related to domainCreateXML(), should there be a reasoning and probably it should be a separate commit even? <div class=""> > cleanup: > - virObjectUnlock(vm); > + if (vm) > + virObjectUnlock(vm); </div>It's safe not to check for NULL here. <div><div class="h5"> > return ret; > } > > @@ -1074,6 +1080,78 @@ bhyveDomainResume(virDomainPtr dom) > return ret; > } > > +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) > + 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; </div></div>Do we really need this ACL check here? <div><div class="h5"> > + if (virDomainObjIsActive(vm)) { > + virReportError(VIR_ERR_OPERATION_INVALID, > + "%s", _("Domain is already running")); > + goto cleanup; > + } > + > + ret = virBhyveProcessStart(dom->conn, privconn, vm, > + VIR_DOMAIN_RUNNING_BOOTED, > + start_flags); > + if (ret) { > + virObjectUnref(dom); > + dom = NULL; > + goto cleanup; > + } > + > + cleanup: > + virObjectUnref(caps); > + virDomainDefFree(def); > + virObjectUnlock(vm); > + > + return dom; > +} > + > static virDriver bhyveDriver = { > .no = VIR_DRV_BHYVE, > .name = "bhyve", > @@ -1091,6 +1169,7 @@ static virDriver bhyveDriver = { > .connectNumOfDefinedDomains = bhyveConnectNumOfDefinedDomains, /* 1.2.2 */ > .domainCreate = bhyveDomainCreate, /* 1.2.2 */ > .domainCreateWithFlags = bhyveDomainCreateWithFlags, /* 1.2.3 */ > + .domainCreateXML = bhyveDomainCreateXML, /* 1.2.4 */ > .domainDestroy = bhyveDomainDestroy, /* 1.2.2 */ > .domainLookupByUUID = bhyveDomainLookupByUUID, /* 1.2.2 */ > .domainLookupByName = bhyveDomainLookupByName, /* 1.2.2 */ > -- > 1.9.0 > </div></div>> -- > libvir-list mailing list > <a href="mailto:libvir-list@redhat.com">libvir-list@redhat.com</a> > <a href="https://www.redhat.com/mailman/listinfo/libvir-list" target="_blank">https://www.redhat.com/mailman/listinfo/libvir-list</a> Roman Bogorodskiy </blockquote></div> </div>