[libvirt] [PATCH 2/2] bhyve: domainCreateXML

[Wojciech Macek]

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 <bogorodskiy at 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140409/25c85eb3/attachment-0001.htm>