On Thu, Sep 07, 2017 at 04:47:53PM -0700, Carlos Cardenas wrote:
> * Fix logic handling stopping a VM. Prevents VMD from crashing.
> * Add additional error code to notify the user that a vm cannot be
> stopped when not running.
> * Add additional log_debug statements.
>
I split this one into a few commits - one for the spaces vs tab issue,
one for the wrong comment at the end, and a third for everything else.
Thanks for these diffs, with these, vm termination is more predictable
with better error messages. There is still one case that doesn't work,
I'll follow up with you off-list.
See inline below for one other place I fixed.
-ml
> diff --git usr.sbin/vmctl/vmctl.c usr.sbin/vmctl/vmctl.c
> index 64d82ca847d..d1517d0d26d 100644
> --- usr.sbin/vmctl/vmctl.c
> +++ usr.sbin/vmctl/vmctl.c
> @@ -206,7 +206,7 @@ vm_start_complete(struct imsg *imsg, int *ret, int
> autoconnect)
> break;
> case VMD_DISK_INVALID:
> warnx("specified disk image(s) are "
> -"not regular files");
> + "not regular files");
> *ret = ENOENT;
> break;
> default:
> @@ -439,12 +439,19 @@ terminate_vm_complete(struct imsg *imsg, int *ret)
> vmr = (struct vmop_result *)imsg->data;
> res = vmr->vmr_result;
> if (res) {
> - errno = res;
> - if (res == ENOENT)
> + switch (res) {
> + case VMD_VM_STOP_INVALID:
> + warnx("cannot stop vm that is not running");
> + *ret = EINVAL;
> + break;
> + case ENOENT:
> warnx("vm not found");
> - else
> + *ret = EIO;
> + break;
> + default:
> warn("terminate vm command failed");
> - *ret = EIO;
> + *ret = EIO;
> + }
> } else {
> warnx("sent request to terminate vm %d", vmr->vmr_id);
> *ret = 0;
> @@ -453,6 +460,7 @@ terminate_vm_complete(struct imsg *imsg, int *ret)
> warnx("unexpected response received from vmd");
> *ret = EINVAL;
> }
> + errno = *ret;
>
> return (1);
> }
> diff --git usr.sbin/vmd/vmd.h usr.sbin/vmd/vmd.h
> index 22da6d58a7b..1240339db52 100644
> --- usr.sbin/vmd/vmd.h
> +++ usr.sbin/vmd/vmd.h
> @@ -54,6 +54,7 @@
> #define VMD_BIOS_MISSING 1001
> #define VMD_DISK_MISSING 1002
> #define VMD_DISK_INVALID 1003
> +#define VMD_VM_STOP_INVALID 1004
>
> /* 100.64.0.0/10 from rfc6598 (IPv4 Prefix for Shared Address Space) */
> #define VMD_DHCP_PREFIX "100.64.0.0/10"
> diff --git usr.sbin/vmd/vmm.c usr.sbin/vmd/vmm.c
> index 0e5ed1ed605..e3ff3be2f35 100644
> --- usr.sbin/vmd/vmm.c
> +++ usr.sbin/vmd/vmm.c
> @@ -150,29 +150,45 @@ vmm_dispatch_parent(int fd, struct privsep_proc *p,
> struct imsg *imsg)
>
> if (id == 0) {
> res = ENOENT;
> - } else if ((vm = vm_getbyvmid(id)) != NULL &&
> - vm->vm_shutdown == 0) {
> - log_debug("%s: sending shutdown request to vm %d",
> - __func__, id);
> -
> - /*
> - * Request reboot but mark the VM as shutting down.
> - * This way we can terminate the VM after the triple
> - * fault instead of reboot and avoid being stuck in
> - * the ACPI-less powerdown ("press any key to reboot")
> - * of the VM.
> - */
> - vm->vm_shutdown = 1;
> - if (imsg_compose_event(&vm->vm_iev,
> - IMSG_VMDOP_VM_REBOOT, 0, 0, -1, NULL, 0) == -1)
> - res = errno;
> - else
> - res = 0;
> + } else if ((vm = vm_getbyvmid(id)) != NULL) {
> + if (vm->vm_shutdown == 0) {
> + log_debug("%s: sending shutdown req to vm %d",
> + __func__, id);
> +
> + /*
> + * Request reboot but mark the VM as shutting
> + * down. This way we can terminate the VM after
> + * the triple fault instead of reboot and
> + * avoid being stuck in the ACPI-less powerdown
> + * ("press any key to reboot") of the VM.
> + */
> +
