3:56 a.m.
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.
What do you mean by ACL check? Do you think the check is unnecessary, or
just should be placed earlier?
W.
2014-04-08 9:01 GMT+02:00 Roman Bogorodskiy <bogorodskiy(a)gmail.com>:
Wojciech Macek wrote:
> Implement bhyveDomainCreteXML function.
s/Crete/Create/
> 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;
> + }
> +
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?
> cleanup:
> - virObjectUnlock(vm);
> + if (vm)
> + virObjectUnlock(vm);
It's safe not to check for NULL here.
> 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;
Do we really need this ACL check here?
> + 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
>
> --
> libvir-list mailing list
> libvir-list(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
Roman Bogorodskiy