[libvirt] [PATCHv2] bhyve: domainCreateXML
Review-fixes, changes since last time: - persistent flag is not reset to 0 anymore - dom->id =vm->def->id is moved after domain startup - duplicate of start_flags |= VIR_BHYVE_PROCESS_START_AUTODESTROY removed - flag VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE is used instead of zero - virDomainObjListRemove is called for not persistent domains in case of failure - unnecessary checkActive removed ======== DIFF BETWEEN PATCHv1 and PATCHv2 ================================= diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 1357e07..4ac89c1 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -1139,39 +1133,36 @@ bhyveDomainCreateXML(virConnectPtr conn, if (!(vm = virDomainObjListAdd(privconn->domains, def, privconn->xmlopt, - 0, NULL))) + VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, 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 (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; + + /* If domain is not persistent, remove its data */ + if (!vm->persistent) { + virDomainObjListRemove(privconn->domains, vm); + vm = NULL; + } goto cleanup; } + dom->id = vm->def->id; + cleanup: virObjectUnref(caps); virDomainDefFree(def); - virObjectUnlock(vm); + if (vm) + virObjectUnlock(vm); return dom; } ======== END OF DIFF BETWEEN PATCHv1 and PATCHv2 ========================= Wojciech Macek (1): bhyve: domainCreateXML src/bhyve/bhyve_driver.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) -- 1.9.0
Implement bhyveDomainCreteXML function. --- src/bhyve/bhyve_driver.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 7187202..4ac89c1 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -1101,6 +1101,72 @@ bhyveConnectCompareCPU(virConnectPtr conn, 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, + VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, NULL))) + goto cleanup; + def = NULL; + + dom = virGetDomain(conn, vm->def->name, vm->def->uuid); + if (!dom) + goto cleanup; + + ret = virBhyveProcessStart(dom->conn, privconn, vm, + VIR_DOMAIN_RUNNING_BOOTED, + start_flags); + if (ret) { + virObjectUnref(dom); + dom = NULL; + + /* If domain is not persistent, remove its data */ + if (!vm->persistent) { + virDomainObjListRemove(privconn->domains, vm); + vm = NULL; + } + goto cleanup; + } + + dom->id = vm->def->id; + + cleanup: + virObjectUnref(caps); + virDomainDefFree(def); + if (vm) + virObjectUnlock(vm); + + return dom; +} + static virDriver bhyveDriver = { .no = VIR_DRV_BHYVE, .name = "bhyve", @@ -1118,6 +1184,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
Wojciech Macek wrote:
Implement bhyveDomainCreteXML function.
s/Crete/Create/
--- src/bhyve/bhyve_driver.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 7187202..4ac89c1 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -1101,6 +1101,72 @@ bhyveConnectCompareCPU(virConnectPtr conn, 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, + VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, NULL))) + goto cleanup; + def = NULL; + + dom = virGetDomain(conn, vm->def->name, vm->def->uuid); + if (!dom) + goto cleanup; + + ret = virBhyveProcessStart(dom->conn, privconn, vm, + VIR_DOMAIN_RUNNING_BOOTED, + start_flags); + if (ret) { + virObjectUnref(dom); + dom = NULL; I think we could simplify the logic a little by calling virBhyveProcessStart() after this block, right before
dom->id = vm->def->id; line. In that case we don't have to jump to cleanup and do that cleanup in if (ret) block as well. I'm going to push that with my comments squashed in, if no objections.
+ /* If domain is not persistent, remove its data */ + if (!vm->persistent) { + virDomainObjListRemove(privconn->domains, vm); + vm = NULL; + } + goto cleanup; + } + + dom->id = vm->def->id; + + cleanup: + virObjectUnref(caps); + virDomainDefFree(def); + if (vm) + virObjectUnlock(vm); + + return dom; +} + static virDriver bhyveDriver = { .no = VIR_DRV_BHYVE, .name = "bhyve", @@ -1118,6 +1184,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
Roman Bogorodskiy
Roman Bogorodskiy wrote:
+ dom = virGetDomain(conn, vm->def->name, vm->def->uuid); + if (!dom) + goto cleanup; + + ret = virBhyveProcessStart(dom->conn, privconn, vm, + VIR_DOMAIN_RUNNING_BOOTED, + start_flags); + if (ret) { + virObjectUnref(dom); + dom = NULL; I think we could simplify the logic a little by calling virBhyveProcessStart() after this block, right before
Err, virGetDomain(), not virBhyveProcessStart().
dom->id = vm->def->id; line. In that case we don't have to jump to cleanup and do that cleanup in if (ret) block as well.
I'm going to push that with my comments squashed in, if no objections.
Roman Bogorodskiy
participants (2)
-
Roman Bogorodskiy -
Wojciech Macek