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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list