From f6e5c9eb7e9081dc2abaae8f39a56d6a510abeb2 Mon Sep 17 00:00:00 2001 From: reyk Date: Wed, 11 Jul 2018 09:35:44 +0000 Subject: [PATCH] Add -f option to vmctl stop to forcefully kill a VM. This also fixes a bug in vmm_sighdlr where it might have missed forwarding the TERMINATE_EVENT to the vmd parent after a VM child died, leading to an abandoned VM in the vmd parent process. OK ccardenas@ mlarkin@ benno@ kn@ --- usr.sbin/vmctl/main.c | 30 +++++++++++++++++++++------- usr.sbin/vmctl/vmctl.8 | 12 ++++++++---- usr.sbin/vmctl/vmctl.c | 14 ++++++++++---- usr.sbin/vmctl/vmctl.h | 5 +++-- usr.sbin/vmd/control.c | 4 +++- usr.sbin/vmd/vmd.c | 6 ++++-- usr.sbin/vmd/vmd.h | 3 ++- usr.sbin/vmd/vmm.c | 44 ++++++++++++++++++++++++------------------ 8 files changed, 78 insertions(+), 40 deletions(-) diff --git a/usr.sbin/vmctl/main.c b/usr.sbin/vmctl/main.c index 8a44bd568d4..69d93d1a771 100644 --- a/usr.sbin/vmctl/main.c +++ b/usr.sbin/vmctl/main.c @@ -1,4 +1,4 @@ -/* $OpenBSD: main.c,v 1.35 2018/02/24 10:39:35 phessler Exp $ */ +/* $OpenBSD: main.c,v 1.36 2018/07/11 09:35:44 reyk Exp $ */ /* * Copyright (c) 2015 Reyk Floeter @@ -73,7 +73,7 @@ struct ctl_command ctl_commands[] = { " [-Lc] [-b image] [-r image] [-m size]\n" "\t\t[-n switch] [-i count] [-d disk]*" }, { "status", CMD_STATUS, ctl_status, "[id]" }, - { "stop", CMD_STOP, ctl_stop, "id" }, + { "stop", CMD_STOP, ctl_stop, "id [-f]" }, { "pause", CMD_PAUSE, ctl_pause, "id" }, { "unpause", CMD_UNPAUSE, ctl_unpause, "id" }, { "send", CMD_SEND, ctl_send, "id", 1}, @@ -212,7 +212,7 @@ vmmaction(struct parse_result *res) } break; case CMD_STOP: - terminate_vm(res->id, res->name); + terminate_vm(res->id, res->name, res->force); break; case CMD_STATUS: get_info_vm(res->id, res->name, 0); @@ -641,12 +641,28 @@ ctl_start(struct parse_result *res, int argc, char *argv[]) int ctl_stop(struct parse_result *res, int argc, char *argv[]) { - if (argc == 2) { - if (parse_vmid(res, argv[1], 0) == -1) - errx(1, "invalid id: %s", argv[1]); - } else if (argc != 2) + int ch; + + if (argc < 2) ctl_usage(res->ctl); + if (parse_vmid(res, argv[1], 0) == -1) + errx(1, "invalid id: %s", argv[1]); + + argc--; + argv++; + + while ((ch = getopt(argc, argv, "f")) != -1) { + switch (ch) { + case 'f': + res->force = 1; + break; + default: + ctl_usage(res->ctl); + /* NOTREACHED */ + } + } + return (vmmaction(res)); } diff --git a/usr.sbin/vmctl/vmctl.8 b/usr.sbin/vmctl/vmctl.8 index 7178ca9f468..4bd39460e6f 100644 --- a/usr.sbin/vmctl/vmctl.8 +++ b/usr.sbin/vmctl/vmctl.8 @@ -1,4 +1,4 @@ -.\" $OpenBSD: vmctl.8,v 1.39 2018/02/24 13:14:09 jmc Exp $ +.\" $OpenBSD: vmctl.8,v 1.40 2018/07/11 09:35:44 reyk Exp $ .\" .\" Copyright (c) 2015 Mike Larkin .\" @@ -14,7 +14,7 @@ .\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. .\" -.Dd $Mdocdate: February 24 2018 $ +.Dd $Mdocdate: July 11 2018 $ .Dt VMCTL 8 .Os .Sh NAME @@ -147,14 +147,18 @@ with '.', '-' or '_'. .It Cm status Op Ar id Lists VMs running on the host, optionally listing just the selected VM .Ar id . -.It Cm stop Ar id +.It Cm stop Ar id Op Fl f Stops (terminates) a VM defined by the specified VM .Ar id . -A graceful shutdown will be attempted if the VM supports the +By default, +a graceful shutdown will be attempted if the VM supports the .Xr vmmci 4 device. Once stopped, if the VM was not defined in a configuration file, then it is removed. +The +.Fl f +option forcefully stops the VM without attempting a graceful shutdown. .It Cm unpause Ar id Unpause (resume from a paused state) a VM with the specified .Ar id . diff --git a/usr.sbin/vmctl/vmctl.c b/usr.sbin/vmctl/vmctl.c index 1a53ce5586b..fff8afcf680 100644 --- a/usr.sbin/vmctl/vmctl.c +++ b/usr.sbin/vmctl/vmctl.c @@ -1,4 +1,4 @@ -/* $OpenBSD: vmctl.c,v 1.50 2018/07/04 02:55:37 anton Exp $ */ +/* $OpenBSD: vmctl.c,v 1.51 2018/07/11 09:35:44 reyk Exp $ */ /* * Copyright (c) 2014 Mike Larkin @@ -410,19 +410,25 @@ unpause_vm_complete(struct imsg *imsg, int *ret) * Parameters: * terminate_id: ID of the vm to be terminated * name: optional name of the VM to be terminated + * force: forcefully kill the VM process */ void -terminate_vm(uint32_t terminate_id, const char *name) +terminate_vm(uint32_t terminate_id, const char *name, int force) { struct vmop_id vid; + int cmd; memset(&vid, 0, sizeof(vid)); vid.vid_id = terminate_id; if (name != NULL) (void)strlcpy(vid.vid_name, name, sizeof(vid.vid_name)); - imsg_compose(ibuf, IMSG_VMDOP_TERMINATE_VM_REQUEST, 0, 0, -1, - &vid, sizeof(vid)); + if (force) + cmd = IMSG_VMDOP_KILL_VM_REQUEST; + else + cmd = IMSG_VMDOP_TERMINATE_VM_REQUEST; + + imsg_compose(ibuf, cmd, 0, 0, -1, &vid, sizeof(vid)); } /* diff --git a/usr.sbin/vmctl/vmctl.h b/usr.sbin/vmctl/vmctl.h index 203c89b85e8..337a5382f86 100644 --- a/usr.sbin/vmctl/vmctl.h +++ b/usr.sbin/vmctl/vmctl.h @@ -1,4 +1,4 @@ -/* $OpenBSD: vmctl.h,v 1.18 2018/01/03 05:39:56 ccardenas Exp $ */ +/* $OpenBSD: vmctl.h,v 1.19 2018/07/11 09:35:44 reyk Exp $ */ /* * Copyright (c) 2015 Reyk Floeter @@ -54,6 +54,7 @@ struct parse_result { char **disks; int disable; int verbose; + int force; unsigned int mode; struct ctl_command *ctl; }; @@ -85,7 +86,7 @@ int create_imagefile(const char *, long); int vm_start(uint32_t, const char *, int, int, char **, int, char **, char *, char *); int vm_start_complete(struct imsg *, int *, int); -void terminate_vm(uint32_t, const char *); +void terminate_vm(uint32_t, const char *, int); int terminate_vm_complete(struct imsg *, int *); void pause_vm(uint32_t, const char *); int pause_vm_complete(struct imsg *, int *); diff --git a/usr.sbin/vmd/control.c b/usr.sbin/vmd/control.c index d9d33c37d4a..37cc538ccb3 100644 --- a/usr.sbin/vmd/control.c +++ b/usr.sbin/vmd/control.c @@ -1,4 +1,4 @@ -/* $OpenBSD: control.c,v 1.25 2018/07/10 20:46:50 reyk Exp $ */ +/* $OpenBSD: control.c,v 1.26 2018/07/11 09:35:44 reyk Exp $ */ /* * Copyright (c) 2010-2015 Reyk Floeter @@ -352,6 +352,7 @@ control_dispatch_imsg(int fd, short event, void *arg) switch (imsg.hdr.type) { case IMSG_VMDOP_GET_INFO_VM_REQUEST: case IMSG_VMDOP_TERMINATE_VM_REQUEST: + case IMSG_VMDOP_KILL_VM_REQUEST: case IMSG_VMDOP_START_VM_REQUEST: case IMSG_VMDOP_PAUSE_VM: case IMSG_VMDOP_UNPAUSE_VM: @@ -410,6 +411,7 @@ control_dispatch_imsg(int fd, short event, void *arg) } break; case IMSG_VMDOP_TERMINATE_VM_REQUEST: + case IMSG_VMDOP_KILL_VM_REQUEST: if (IMSG_DATA_SIZE(&imsg) < sizeof(vid)) goto fail; memcpy(&vid, imsg.data, sizeof(vid)); diff --git a/usr.sbin/vmd/vmd.c b/usr.sbin/vmd/vmd.c index ddd3cca8125..0112ea04b27 100644 --- a/usr.sbin/vmd/vmd.c +++ b/usr.sbin/vmd/vmd.c @@ -1,4 +1,4 @@ -/* $OpenBSD: vmd.c,v 1.90 2018/07/10 21:12:20 reyk Exp $ */ +/* $OpenBSD: vmd.c,v 1.91 2018/07/11 09:35:44 reyk Exp $ */ /* * Copyright (c) 2015 Reyk Floeter @@ -111,6 +111,7 @@ vmd_dispatch_control(int fd, struct privsep_proc *p, struct imsg *imsg) } break; case IMSG_VMDOP_TERMINATE_VM_REQUEST: + case IMSG_VMDOP_KILL_VM_REQUEST: IMSG_SIZE_CHECK(imsg, &vid); memcpy(&vid, imsg->data, sizeof(vid)); if ((id = vid.vid_id) == 0) { @@ -119,7 +120,8 @@ vmd_dispatch_control(int fd, struct privsep_proc *p, struct imsg *imsg) res = ENOENT; cmd = IMSG_VMDOP_TERMINATE_VM_RESPONSE; break; - } else if (vm->vm_shutdown) { + } else if (vm->vm_shutdown && + imsg->hdr.type != IMSG_VMDOP_KILL_VM_REQUEST) { res = EALREADY; cmd = IMSG_VMDOP_TERMINATE_VM_RESPONSE; break; diff --git a/usr.sbin/vmd/vmd.h b/usr.sbin/vmd/vmd.h index 85a46d36d2d..8b1b7a5f058 100644 --- a/usr.sbin/vmd/vmd.h +++ b/usr.sbin/vmd/vmd.h @@ -1,4 +1,4 @@ -/* $OpenBSD: vmd.h,v 1.72 2018/07/10 20:43:15 reyk Exp $ */ +/* $OpenBSD: vmd.h,v 1.73 2018/07/11 09:35:44 reyk Exp $ */ /* * Copyright (c) 2015 Mike Larkin @@ -82,6 +82,7 @@ enum imsg_type { IMSG_VMDOP_RECEIVE_VM_RESPONSE, IMSG_VMDOP_RECEIVE_VM_END, IMSG_VMDOP_TERMINATE_VM_REQUEST, + IMSG_VMDOP_KILL_VM_REQUEST, IMSG_VMDOP_TERMINATE_VM_RESPONSE, IMSG_VMDOP_TERMINATE_VM_EVENT, IMSG_VMDOP_GET_INFO_VM_REQUEST, diff --git a/usr.sbin/vmd/vmm.c b/usr.sbin/vmd/vmm.c index 5ac3f0f7025..cd721de8989 100644 --- a/usr.sbin/vmd/vmm.c +++ b/usr.sbin/vmd/vmm.c @@ -1,4 +1,4 @@ -/* $OpenBSD: vmm.c,v 1.84 2018/07/10 20:52:51 reyk Exp $ */ +/* $OpenBSD: vmm.c,v 1.85 2018/07/11 09:35:44 reyk Exp $ */ /* * Copyright (c) 2015 Mike Larkin @@ -150,6 +150,7 @@ vmm_dispatch_parent(int fd, struct privsep_proc *p, struct imsg *imsg) cmd = IMSG_VMDOP_START_VM_RESPONSE; break; case IMSG_VMDOP_TERMINATE_VM_REQUEST: + case IMSG_VMDOP_KILL_VM_REQUEST: IMSG_SIZE_CHECK(imsg, &vtp); memcpy(&vtp, imsg->data, sizeof(vtp)); id = vtp.vtp_vm_id; @@ -159,7 +160,12 @@ vmm_dispatch_parent(int fd, struct privsep_proc *p, struct imsg *imsg) if (id == 0) { res = ENOENT; } else if ((vm = vm_getbyvmid(id)) != NULL) { - if (vm->vm_shutdown == 0) { + if (imsg->hdr.type == IMSG_VMDOP_KILL_VM_REQUEST) { + vtp.vtp_vm_id = vm_vmid2id(vm->vm_vmid, vm); + vm->vm_shutdown = 1; + (void)terminate_vm(&vtp); + res = 0; + } else if (vm->vm_shutdown == 0) { log_debug("%s: sending shutdown request" " to vm %d", __func__, id); @@ -369,22 +375,23 @@ vmm_sighdlr(int sig, short event, void *arg) vmid = vm->vm_params.vmc_params.vcp_id; vtp.vtp_vm_id = vmid; - log_debug("%s: attempting to terminate vm %d", - __func__, vm->vm_vmid); - if (terminate_vm(&vtp) == 0) { - memset(&vmr, 0, sizeof(vmr)); - vmr.vmr_result = ret; - vmr.vmr_id = vm_id2vmid(vmid, vm); - if (proc_compose_imsg(ps, PROC_PARENT, - -1, IMSG_VMDOP_TERMINATE_VM_EVENT, - 0, -1, &vmr, sizeof(vmr)) == -1) - log_warnx("could not signal " - "termination of VM %u to " - "parent", vm->vm_vmid); - } else - log_warnx("could not terminate VM %u", + + if (terminate_vm(&vtp) == 0) + log_debug("%s: terminated vm %s" + " (id %d)", __func__, + vm->vm_params.vmc_params.vcp_name, vm->vm_vmid); + memset(&vmr, 0, sizeof(vmr)); + vmr.vmr_result = ret; + vmr.vmr_id = vm_id2vmid(vmid, vm); + if (proc_compose_imsg(ps, PROC_PARENT, + -1, IMSG_VMDOP_TERMINATE_VM_EVENT, + 0, -1, &vmr, sizeof(vmr)) == -1) + log_warnx("could not signal " + "termination of VM %u to " + "parent", vm->vm_vmid); + vm_remove(vm, __func__); } else fatalx("unexpected cause of SIGCHLD"); @@ -536,8 +543,7 @@ vmm_dispatch_vm(int fd, short event, void *arg) int terminate_vm(struct vm_terminate_params *vtp) { - log_debug("%s: terminating vmid %d", __func__, vtp->vtp_vm_id); - if (ioctl(env->vmd_fd, VMM_IOC_TERM, vtp) < 0) + if (ioctl(env->vmd_fd, VMM_IOC_TERM, vtp) == -1) return (errno); return (0); @@ -748,7 +754,7 @@ get_info_vm(struct privsep *ps, struct imsg *imsg, int terminate) vtp.vtp_vm_id = info[i].vir_id; if ((ret = terminate_vm(&vtp)) != 0) return (ret); - log_debug("%s: terminated VM %s (id %d)", __func__, + log_debug("%s: terminated vm %s (id %d)", __func__, info[i].vir_name, info[i].vir_id); continue; } -- 2.20.1