Add -f option to vmctl stop to forcefully kill a VM.
authorreyk <reyk@openbsd.org>
Wed, 11 Jul 2018 09:35:44 +0000 (09:35 +0000)
committerreyk <reyk@openbsd.org>
Wed, 11 Jul 2018 09:35:44 +0000 (09:35 +0000)
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
usr.sbin/vmctl/vmctl.8
usr.sbin/vmctl/vmctl.c
usr.sbin/vmctl/vmctl.h
usr.sbin/vmd/control.c
usr.sbin/vmd/vmd.c
usr.sbin/vmd/vmd.h
usr.sbin/vmd/vmm.c

index 8a44bd5..69d93d1 100644 (file)
@@ -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 <reyk@openbsd.org>
@@ -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));
 }
 
index 7178ca9..4bd3946 100644 (file)
@@ -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 <mlarkin@openbsd.org>
 .\"
@@ -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 .
index 1a53ce5..fff8afc 100644 (file)
@@ -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 <mlarkin@openbsd.org>
@@ -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));
 }
 
 /*
index 203c89b..337a538 100644 (file)
@@ -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 <reyk@openbsd.org>
@@ -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 *);
index d9d33c3..37cc538 100644 (file)
@@ -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 <reyk@openbsd.org>
@@ -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));
index ddd3cca..0112ea0 100644 (file)
@@ -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 <reyk@openbsd.org>
@@ -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;
index 85a46d3..8b1b7a5 100644 (file)
@@ -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 <mlarkin@openbsd.org>
@@ -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,
index 5ac3f0f..cd721de 100644 (file)
@@ -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 <mlarkin@openbsd.org>
@@ -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;
                }