[libvirt] [PATCH 0/2] bhyve: enhance domain management
Two small changes: 1. Inside domainDestroy, forget about domain if it is not persistent. 2. Add domainCreateXML call Wojciech Macek (2): bhyve: fix domain management bhyve: domainCreateXML src/bhyve/bhyve_driver.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 78 insertions(+), 2 deletions(-) -- 1.9.0
When domain is not persistent, it should be forgotten upon destroying. --- src/bhyve/bhyve_driver.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 7187202..23e7112 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -753,8 +753,14 @@ bhyveDomainDestroy(virDomainPtr dom) ret = virBhyveProcessStop(privconn, vm, VIR_DOMAIN_SHUTOFF_DESTROYED); - cleanup: - virObjectUnlock(vm); + if (!vm->persistent) { + virDomainObjListRemove(privconn->domains, vm); + vm = NULL; + } + +cleanup: + if (vm) + virObjectUnlock(vm); return ret; } -- 1.9.0
Wojciech Macek wrote:
When domain is not persistent, it should be forgotten upon destroying. --- src/bhyve/bhyve_driver.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 7187202..23e7112 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -753,8 +753,14 @@ bhyveDomainDestroy(virDomainPtr dom)
ret = virBhyveProcessStop(privconn, vm, VIR_DOMAIN_SHUTOFF_DESTROYED);
- cleanup: - virObjectUnlock(vm); + if (!vm->persistent) { + virDomainObjListRemove(privconn->domains, vm); + vm = NULL; + } + +cleanup: + if (vm) + virObjectUnlock(vm); return ret; }
ACK, I'm going to push that tomorrow if there'll be no other notes. Roman Bogorodskiy
Roman Bogorodskiy wrote:
Wojciech Macek wrote:
When domain is not persistent, it should be forgotten upon destroying. --- src/bhyve/bhyve_driver.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 7187202..23e7112 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -753,8 +753,14 @@ bhyveDomainDestroy(virDomainPtr dom)
ret = virBhyveProcessStop(privconn, vm, VIR_DOMAIN_SHUTOFF_DESTROYED);
- cleanup: - virObjectUnlock(vm); + if (!vm->persistent) { + virDomainObjListRemove(privconn->domains, vm); + vm = NULL; + } + +cleanup: + if (vm) + virObjectUnlock(vm); return ret; }
ACK, I'm going to push that tomorrow if there'll be no other notes.
Squashed in a label formatting style fix (now labels should have one space offset) and pushed, thanks! Roman Bogorodskiy
Implement bhyveDomainCreteXML function. --- src/bhyve/bhyve_driver.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 23e7112..1357e07 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -1107,6 +1107,75 @@ 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, + 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 (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", @@ -1124,6 +1193,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
On Wed, Apr 09, 2014 at 07:52:12AM +0200, Wojciech Macek wrote:
Implement bhyveDomainCreteXML function. --- src/bhyve/bhyve_driver.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 23e7112..1357e07 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -1107,6 +1107,75 @@ 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, + 0, NULL)))
You should pass VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, rather than 0. This ensures you don't overwrite the config of an existing runing guest
+ goto cleanup; + def = NULL; + vm->persistent = 0;
Don't reset the persistent flag. The guest may already exist as an inactive persistent guest, and you're causing that to be lost.
+ + dom = virGetDomain(conn, vm->def->name, vm->def->uuid); + if (!dom) + goto cleanup; + + dom->id = vm->def->id;
Err, the 'id' won't be set until you've started the guest, so this fnuction call & set needs to be the last thing you do
+ + if (flags & VIR_DOMAIN_START_AUTODESTROY) + start_flags |= VIR_BHYVE_PROCESS_START_AUTODESTROY;
You've already done this above.
+ + if (virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("Domain is already running")); + goto cleanup; + }
This is not required if you pass the right flags to the virDomainObjListAdd method
+ + 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; +}
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Daniel P. Berrange wrote:
On Wed, Apr 09, 2014 at 07:52:12AM +0200, Wojciech Macek wrote:
Implement bhyveDomainCreteXML function. --- src/bhyve/bhyve_driver.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 23e7112..1357e07 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -1107,6 +1107,75 @@ 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, + 0, NULL)))
You should pass VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, rather than 0. This ensures you don't overwrite the config of an existing runing guest
+ goto cleanup; + def = NULL; + vm->persistent = 0;
Don't reset the persistent flag. The guest may already exist as an inactive persistent guest, and you're causing that to be lost.
+ + dom = virGetDomain(conn, vm->def->name, vm->def->uuid); + if (!dom) + goto cleanup; + + dom->id = vm->def->id;
Err, the 'id' won't be set until you've started the guest, so this fnuction call & set needs to be the last thing you do
+ + if (flags & VIR_DOMAIN_START_AUTODESTROY) + start_flags |= VIR_BHYVE_PROCESS_START_AUTODESTROY;
You've already done this above.
+ + if (virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("Domain is already running")); + goto cleanup; + }
This is not required if you pass the right flags to the virDomainObjListAdd method
+ + ret = virBhyveProcessStart(dom->conn, privconn, vm, + VIR_DOMAIN_RUNNING_BOOTED, + start_flags); + if (ret) {
As discussed on IRC now, I think we need to call virDomainObjListRemove() if the domain is not persistent in case if virBhyveProcessStart() fails. Daniel, please correct me if I'm wrong.
+ virObjectUnref(dom); + dom = NULL; + goto cleanup; + }
Roman Bogorodskiy
On Wed, Apr 09, 2014 at 03:11:48PM +0400, Roman Bogorodskiy wrote:
Daniel P. Berrange wrote:
On Wed, Apr 09, 2014 at 07:52:12AM +0200, Wojciech Macek wrote:
Implement bhyveDomainCreteXML function. --- src/bhyve/bhyve_driver.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 23e7112..1357e07 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -1107,6 +1107,75 @@ 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, + 0, NULL)))
You should pass VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, rather than 0. This ensures you don't overwrite the config of an existing runing guest
+ goto cleanup; + def = NULL; + vm->persistent = 0;
Don't reset the persistent flag. The guest may already exist as an inactive persistent guest, and you're causing that to be lost.
+ + dom = virGetDomain(conn, vm->def->name, vm->def->uuid); + if (!dom) + goto cleanup; + + dom->id = vm->def->id;
Err, the 'id' won't be set until you've started the guest, so this fnuction call & set needs to be the last thing you do
+ + if (flags & VIR_DOMAIN_START_AUTODESTROY) + start_flags |= VIR_BHYVE_PROCESS_START_AUTODESTROY;
You've already done this above.
+ + if (virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("Domain is already running")); + goto cleanup; + }
This is not required if you pass the right flags to the virDomainObjListAdd method
+ + ret = virBhyveProcessStart(dom->conn, privconn, vm, + VIR_DOMAIN_RUNNING_BOOTED, + start_flags); + if (ret) {
As discussed on IRC now, I think we need to call virDomainObjListRemove() if the domain is not persistent in case if virBhyveProcessStart() fails.
Daniel, please correct me if I'm wrong.
You are right. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (3)
-
Daniel P. Berrange -
Roman Bogorodskiy -
Wojciech Macek