Hi,

It seems there is an inconsistency when it comes to terminating a VM by
id or name (4/web point to the same VM here):

before:
% vmctl stop 4
stopping vm: requested to shutdown vm 4
% vmctl stop web
stopping vm web: failed: Invalid argument

Here's a diff which moves the checks out of the block which is only
entered when we pass it a name:

after:
% vmctl stop 4
stopping vm: failed: Invalid argument
% vmctl stop web
stopping vm web: failed: Invalid argument

If EINVAL is actually correct in this case I think is open for
discussion.
ENOTSUP might not be a bad candidate actually if the VM isn't running.

OK?

Index: vmd.c
===================================================================
RCS file: /cvs/src/usr.sbin/vmd/vmd.c,v
retrieving revision 1.150
diff -u -p -r1.150 vmd.c
--- vmd.c       18 Jun 2023 11:45:11 -0000      1.150
+++ vmd.c       2 Jul 2023 12:30:33 -0000
@@ -159,20 +159,22 @@ vmd_dispatch_control(int fd, struct priv
                        if ((vm = vm_getbyname(vid.vid_name)) == NULL) {
                                res = ENOENT;
                                break;
-                       } else if ((vm->vm_state & VM_STATE_SHUTDOWN) &&
-                           (flags & VMOP_FORCE) == 0) {
-                               res = EALREADY;
-                               break;
-                       } else if (!(vm->vm_state & VM_STATE_RUNNING)) {
-                               res = EINVAL;
-                               break;
                        }
                        id = vm->vm_vmid;
                } else if ((vm = vm_getbyvmid(id)) == NULL) {
                        res = ENOENT;
                        break;
                }
-               if (vm_checkperm(vm, &vm->vm_params.vmc_owner, vid.vid_uid)) {
+
+               /* Validate curent state of vm */
+               if ((vm->vm_state & VM_STATE_SHUTDOWN) &&
+                           (flags & VMOP_FORCE) == 0) {
+                               res = EALREADY;
+                               break;
+               } else if (!(vm->vm_state & VM_STATE_RUNNING)) {
+                       res = EINVAL;
+                       break;
+               } else if (vm_checkperm(vm, &vm->vm_params.vmc_owner, 
vid.vid_uid)) {
                        res = EPERM;
                        break;
                }
-- 
jasper

Reply via email to