[libvirt] [PATCH] bhyve: add domainCreateWithFlags support
The only supported flag for now is 'autodestroy'. In order to support 'autodestroy', add support for close callbacks. --- src/bhyve/bhyve_driver.c | 28 +++++++++++++++++++++++++--- src/bhyve/bhyve_process.c | 29 ++++++++++++++++++++++++++++- src/bhyve/bhyve_process.h | 7 ++++++- src/bhyve/bhyve_utils.h | 3 +++ 4 files changed, 62 insertions(+), 5 deletions(-) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index ff9ac0d..3d7b7ce 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -176,6 +176,9 @@ bhyveConnectOpen(virConnectPtr conn, static int bhyveConnectClose(virConnectPtr conn) { + bhyveConnPtr privconn = conn->privateData; + + virCloseCallbacksRun(privconn->closeCallbacks, conn, privconn->domains, privconn); conn->privateData = NULL; return 0; @@ -528,16 +531,23 @@ cleanup: } static int -bhyveDomainCreate(virDomainPtr dom) +bhyveDomainCreateWithFlags(virDomainPtr dom, + unsigned int flags) { bhyveConnPtr privconn = dom->conn->privateData; virDomainObjPtr vm; + unsigned int start_flags = 0; int ret = -1; + virCheckFlags(VIR_DOMAIN_START_AUTODESTROY, -1); + + if (flags & VIR_DOMAIN_START_AUTODESTROY) + start_flags |= VIR_BHYVE_PROCESS_START_AUTODESTROY; + if (!(vm = bhyveDomObjFromDomain(dom))) goto cleanup; - if (virDomainCreateEnsureACL(dom->conn, vm->def) < 0) + if (virDomainCreateWithFlagsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; if (virDomainObjIsActive(vm)) { @@ -547,7 +557,8 @@ bhyveDomainCreate(virDomainPtr dom) } ret = virBhyveProcessStart(dom->conn, privconn, vm, - VIR_DOMAIN_RUNNING_BOOTED); + VIR_DOMAIN_RUNNING_BOOTED, + start_flags); cleanup: virObjectUnlock(vm); @@ -555,6 +566,12 @@ cleanup: } static int +bhyveDomainCreate(virDomainPtr dom) +{ + return bhyveDomainCreateWithFlags(dom, 0); +} + +static int bhyveDomainDestroy(virDomainPtr dom) { bhyveConnPtr privconn = dom->conn->privateData; @@ -627,6 +644,7 @@ bhyveStateCleanup(void) virObjectUnref(bhyve_driver->domains); virObjectUnref(bhyve_driver->caps); virObjectUnref(bhyve_driver->xmlopt); + virObjectUnref(bhyve_driver->closeCallbacks); virMutexDestroy(&bhyve_driver->lock); VIR_FREE(bhyve_driver); @@ -653,6 +671,9 @@ bhyveStateInitialize(bool priveleged ATTRIBUTE_UNUSED, return -1; } + if (!(bhyve_driver->closeCallbacks = virCloseCallbacksNew())) + goto cleanup; + if (!(bhyve_driver->caps = bhyveBuildCapabilities())) goto cleanup; @@ -709,6 +730,7 @@ static virDriver bhyveDriver = { .connectListDefinedDomains = bhyveConnectListDefinedDomains, /* 1.2.2 */ .connectNumOfDefinedDomains = bhyveConnectNumOfDefinedDomains, /* 1.2.2 */ .domainCreate = bhyveDomainCreate, /* 1.2.2 */ + .domainCreateWithFlags = bhyveDomainCreateWithFlags, /* 1.2.3 */ .domainDestroy = bhyveDomainDestroy, /* 1.2.2 */ .domainLookupByUUID = bhyveDomainLookupByUUID, /* 1.2.2 */ .domainLookupByName = bhyveDomainLookupByName, /* 1.2.2 */ diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c index ee85680..05bfe15 100644 --- a/src/bhyve/bhyve_process.c +++ b/src/bhyve/bhyve_process.c @@ -44,11 +44,29 @@ #define VIR_FROM_THIS VIR_FROM_BHYVE +static virDomainObjPtr +bhyveProcessAutoDestroy(virDomainObjPtr vm, + virConnectPtr conn ATTRIBUTE_UNUSED, + void *opaque) +{ + bhyveConnPtr driver = opaque; + + virBhyveProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED); + + if (!vm->persistent) { + virDomainObjListRemove(driver->domains, vm); + vm = NULL; + } + + return vm; +} + int virBhyveProcessStart(virConnectPtr conn, bhyveConnPtr driver, virDomainObjPtr vm, - virDomainRunningReason reason) + virDomainRunningReason reason, + unsigned int flags) { char *logfile = NULL; int logfd = -1; @@ -130,6 +148,12 @@ virBhyveProcessStart(virConnectPtr conn, vm->def->id = vm->pid; virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, reason); + + if (flags & VIR_BHYVE_PROCESS_START_AUTODESTROY && + virCloseCallbacksSet(driver->closeCallbacks, vm, + conn, bhyveProcessAutoDestroy) < 0) + goto cleanup; + } else { goto cleanup; } @@ -201,6 +225,9 @@ virBhyveProcessStop(bhyveConnPtr driver, ret = 0; + virCloseCallbacksUnset(driver->closeCallbacks, vm, + bhyveProcessAutoDestroy); + virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason); vm->pid = -1; vm->def->id = -1; diff --git a/src/bhyve/bhyve_process.h b/src/bhyve/bhyve_process.h index 66548ae..f91504e 100644 --- a/src/bhyve/bhyve_process.h +++ b/src/bhyve/bhyve_process.h @@ -27,10 +27,15 @@ int virBhyveProcessStart(virConnectPtr conn, bhyveConnPtr driver, virDomainObjPtr vm, - virDomainRunningReason reason); + virDomainRunningReason reason, + unsigned int flags); int virBhyveProcessStop(bhyveConnPtr driver, virDomainObjPtr vm, virDomainShutoffReason reason); +typedef enum { + VIR_BHYVE_PROCESS_START_AUTODESTROY = 1 << 0, +} bhyveProcessStartFlags; + #endif /* __BHYVE_PROCESS_H__ */ diff --git a/src/bhyve/bhyve_utils.h b/src/bhyve/bhyve_utils.h index 0810caa..6c76770 100644 --- a/src/bhyve/bhyve_utils.h +++ b/src/bhyve/bhyve_utils.h @@ -26,6 +26,7 @@ # include "domain_conf.h" # include "configmake.h" # include "virthread.h" +# include "virclosecallbacks.h" # define BHYVE_AUTOSTART_DIR SYSCONFDIR "/libvirt/bhyve/autostart" # define BHYVE_CONFIG_DIR SYSCONFDIR "/libvirt/bhyve" @@ -38,6 +39,8 @@ struct _bhyveConn { virCapsPtr caps; virDomainXMLOptionPtr xmlopt; char *pidfile; + + virCloseCallbacksPtr closeCallbacks; }; typedef struct _bhyveConn bhyveConn; -- 1.8.4.3
On 18.03.2014 10:31, Roman Bogorodskiy wrote:
The only supported flag for now is 'autodestroy'. In order to support 'autodestroy', add support for close callbacks. --- src/bhyve/bhyve_driver.c | 28 +++++++++++++++++++++++++--- src/bhyve/bhyve_process.c | 29 ++++++++++++++++++++++++++++- src/bhyve/bhyve_process.h | 7 ++++++- src/bhyve/bhyve_utils.h | 3 +++ 4 files changed, 62 insertions(+), 5 deletions(-)
diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c index ee85680..05bfe15 100644 --- a/src/bhyve/bhyve_process.c +++ b/src/bhyve/bhyve_process.c @@ -44,11 +44,29 @@
#define VIR_FROM_THIS VIR_FROM_BHYVE
+static virDomainObjPtr +bhyveProcessAutoDestroy(virDomainObjPtr vm, + virConnectPtr conn ATTRIBUTE_UNUSED, + void *opaque) +{ + bhyveConnPtr driver = opaque; + + virBhyveProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED); + + if (!vm->persistent) { + virDomainObjListRemove(driver->domains, vm); + vm = NULL; + } + + return vm; +} + int virBhyveProcessStart(virConnectPtr conn, bhyveConnPtr driver, virDomainObjPtr vm, - virDomainRunningReason reason) + virDomainRunningReason reason, + unsigned int flags) { char *logfile = NULL; int logfd = -1; @@ -130,6 +148,12 @@ virBhyveProcessStart(virConnectPtr conn,
vm->def->id = vm->pid; virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, reason); + + if (flags & VIR_BHYVE_PROCESS_START_AUTODESTROY && + virCloseCallbacksSet(driver->closeCallbacks, vm, + conn, bhyveProcessAutoDestroy) < 0) + goto cleanup;
If this fails, we should kill the domain and return an error. It won't happen currently, as ret is equal to zero in this area of code (it's not visible in the context, but this whole block starts with 'if (ret == 0) {'). Moreover, the same bug is present a few line above, just at the beginning of the block. So the code I'm aiming at looks like this: diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c index 272cf4a..423185a 100644 --- a/src/bhyve/bhyve_process.c +++ b/src/bhyve/bhyve_process.c @@ -139,27 +139,25 @@ virBhyveProcessStart(virConnectPtr conn, /* Now we can start the domain */ VIR_DEBUG("Starting domain '%s'", vm->def->name); - ret = virCommandRun(cmd, NULL); + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; - if (ret == 0) { - if (virPidFileReadPath(privconn->pidfile, &vm->pid) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Domain %s didn't show up"), vm->def->name); - goto cleanup; - } - - vm->def->id = vm->pid; - virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, reason); - - if (flags & VIR_BHYVE_PROCESS_START_AUTODESTROY && - virCloseCallbacksSet(driver->closeCallbacks, vm, - conn, bhyveProcessAutoDestroy) < 0) - goto cleanup; - - } else { + if (virPidFileReadPath(privconn->pidfile, &vm->pid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Domain %s didn't show up"), vm->def->name); goto cleanup; } + if (flags & VIR_BHYVE_PROCESS_START_AUTODESTROY && + virCloseCallbacksSet(driver->closeCallbacks, vm, + conn, bhyveProcessAutoDestroy) < 0) + goto cleanup; + + vm->def->id = vm->pid; + virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, reason); + + ret = 0; + cleanup: if (ret < 0) { virCommandPtr destroy_cmd; ACK with that squashed in. Michal
Michal Privoznik wrote:
If this fails, we should kill the domain and return an error. It won't happen currently, as ret is equal to zero in this area of code (it's not visible in the context, but this whole block starts with 'if (ret == 0) {'). Moreover, the same bug is present a few line above, just at the beginning of the block. So the code I'm aiming at looks like this:
Thanks for noticing that!
ACK with that squashed in.
Pushed, thanks for the review, Roman Bogorodskiy
participants (2)
-
Michal Privoznik -
Roman Bogorodskiy