<div dir="ltr"><div>Oh, yep. We don't need an ACL again. I will send new patchset today.<br><br>Thanks,<br></div>Wojtek<br></div><div class="gmail_extra"><br><br><div class="gmail_quote">2014-04-08 12:09 GMT+02:00 Roman Bogorodskiy <span dir="ltr"><<a href="mailto:bogorodskiy@gmail.com" target="_blank">bogorodskiy@gmail.com</a>></span>:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">  Wojciech Macek wrote:<br>
<br>
> Sure, I will split it into two separate commits. I must have messed up sth<br>
> in commits, since even in head-email I described 3 patches...<br>
<br>
</div>That would be good, thanks!<br>
<div class=""><br>
> Regarding NULL, it is a little tricky. After running<br>
> virDomainObjListRemove, the vm pointer has 0 references. Then, running<br>
> virObjectUnlock causes libvirt segfault. To minimize changes, I used the<br>
> solution from qemu: set vm to null and then check it inside cleanup. The<br>
> "if (vm)" is just aesthetic - without it there was an awful warning on the<br>
> console about null-pointer. I'd like to leave it as is, or remove<br>
> "vm=NULL;" and insert virObjectRef/virObjectUnref pair instead.<br>
<br>
</div>Yeah, looks like you're right, sorry for the confusion.<br>
<div class=""><br>
> What do you mean by ACL check? Do you think the check is unnecessary, or<br>
> just should be placed earlier?<br>
<br>
</div>As for the ACL...<br>
<div><div class="h5"><br>
> > > +static virDomainPtr<br>
> > > +bhyveDomainCreateXML(virConnectPtr conn,<br>
> > > +                     const char *xml,<br>
> > > +                     unsigned int flags)<br>
> > > +{<br>
> > > +    bhyveConnPtr privconn = conn->privateData;<br>
> > > +    virDomainPtr dom = NULL;<br>
> > > +    virDomainDefPtr def = NULL;<br>
> > > +    virDomainObjPtr vm = NULL;<br>
> > > +    virCapsPtr caps = NULL;<br>
> > > +    int ret;<br>
> > > +    unsigned int start_flags = 0;<br>
> > > +<br>
> > > +    virCheckFlags(VIR_DOMAIN_START_AUTODESTROY, NULL);<br>
> > > +<br>
> > > +    if (flags & VIR_DOMAIN_START_AUTODESTROY)<br>
> > > +        start_flags |= VIR_BHYVE_PROCESS_START_AUTODESTROY;<br>
> > > +<br>
> > > +    caps = bhyveDriverGetCapabilities(privconn);<br>
> > > +    if (!caps)<br>
> > > +        return NULL;<br>
> > > +<br>
> > > +    if ((def = virDomainDefParseString(xml, caps, privconn->xmlopt,<br>
> > > +                                       1 << VIR_DOMAIN_VIRT_BHYVE,<br>
> > > +                                       VIR_DOMAIN_XML_INACTIVE)) ==<br>
> > NULL)<br>
> > > +        goto cleanup;<br>
> > > +<br>
> > > +    if (virDomainCreateXMLEnsureACL(conn, def) < 0)<br>
<br>
</div></div>We check it here...<br>
<div class=""><br>
> > > +        goto cleanup;<br>
> > > +<br>
> > > +    if (!(vm = virDomainObjListAdd(privconn->domains, def,<br>
> > > +                                   privconn->xmlopt,<br>
> > > +                                   0, NULL)))<br>
> > > +        goto cleanup;<br>
> > > +    def = NULL;<br>
> > > +    vm->persistent = 0;<br>
> > > +<br>
> > > +    dom = virGetDomain(conn, vm->def->name, vm->def->uuid);<br>
> > > +    if (!dom)<br>
> > > +        goto cleanup;<br>
> > > +<br>
> > > +    dom->id = vm->def->id;<br>
> > > +<br>
> > > +    if (flags & VIR_DOMAIN_START_AUTODESTROY)<br>
> > > +        start_flags |= VIR_BHYVE_PROCESS_START_AUTODESTROY;<br>
> > > +<br>
> > > +    if (virDomainCreateWithFlagsEnsureACL(dom->conn, vm->def) < 0)<br>
> > > +            goto cleanup;<br>
<br>
</div>Do we need it here again?<br>
<span class="HOEnZb"><font color="#888888"><br>
Roman Bogorodskiy<br>
</font></span></blockquote></div><br></div>