Matthew Martin recently presented a patch on tech@ [1] fixing some missed scaling from when I converted vmd(8) to use bytes instead of megabytes everywhere. I finally found time to wade through the code it touches and am proposing we simply "tedu" the incomplete feature.
Does anyone use this? (And if so, how?) I don't see much value in this framework and it only adds additional state to track. Users can be confined by limits associated in login.conf(5) for the most part. There are more interesting things to work on, so unless anyone speaks up I'll look for an OK to remove it. -dv [1] https://marc.info/?l=openbsd-tech&m=166346196317673&w=2 diff refs/heads/master refs/heads/vmd-user commit - bfe2092d87b190d9f89c4a6f2728a539b7f88233 commit + e84ff2c7628a811e00044a447ad906d6e24beac0 blob - 374d7de6629e072065b5c0232536c23c1e5bbbe0 blob + a192223cf118e2a8764b24f965a15acbf8ae506f --- usr.sbin/vmd/config.c +++ usr.sbin/vmd/config.c @@ -98,12 +98,6 @@ config_init(struct vmd *env) return (-1); TAILQ_INIT(env->vmd_switches); } - if (what & CONFIG_USERS) { - if ((env->vmd_users = calloc(1, - sizeof(*env->vmd_users))) == NULL) - return (-1); - TAILQ_INIT(env->vmd_users); - } return (0); } @@ -238,13 +232,6 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, ui return (EALREADY); } - /* increase the user reference counter and check user limits */ - if (vm->vm_user != NULL && user_get(vm->vm_user->usr_id.uid) != NULL) { - user_inc(vcp, vm->vm_user, 1); - if (user_checklimit(vm->vm_user, vcp) == -1) - return (EPERM); - } - /* * Rate-limit the VM so that it cannot restart in a loop: * if the VM restarts after less than VM_START_RATE_SEC seconds, blob - 2f3ac1a76f2c3e458919eca85c238a668c10422a blob + 755cbedb6a18502a87724502ec86e9e426961701 --- usr.sbin/vmd/vmd.c +++ usr.sbin/vmd/vmd.c @@ -1188,9 +1188,6 @@ vm_stop(struct vmd_vm *vm, int keeptty, const char *ca vm->vm_state &= ~(VM_STATE_RECEIVED | VM_STATE_RUNNING | VM_STATE_SHUTDOWN); - user_inc(&vm->vm_params.vmc_params, vm->vm_user, 0); - user_put(vm->vm_user); - if (vm->vm_iev.ibuf.fd != -1) { event_del(&vm->vm_iev.ev); close(vm->vm_iev.ibuf.fd); @@ -1243,7 +1240,6 @@ vm_remove(struct vmd_vm *vm, const char *caller) TAILQ_REMOVE(env->vmd_vms, vm, vm_entry); - user_put(vm->vm_user); vm_stop(vm, 0, caller); free(vm); } @@ -1286,7 +1282,6 @@ vm_register(struct privsep *ps, struct vmop_create_par struct vmd_vm *vm = NULL, *vm_parent = NULL; struct vm_create_params *vcp = &vmc->vmc_params; struct vmop_owner *vmo = NULL; - struct vmd_user *usr = NULL; uint32_t nid, rng; unsigned int i, j; struct vmd_switch *sw; @@ -1362,13 +1357,6 @@ vm_register(struct privsep *ps, struct vmop_create_par } } - /* track active users */ - if (uid != 0 && env->vmd_users != NULL && - (usr = user_get(uid)) == NULL) { - log_warnx("could not add user"); - goto fail; - } - if ((vm = calloc(1, sizeof(*vm))) == NULL) goto fail; @@ -1379,7 +1367,6 @@ vm_register(struct privsep *ps, struct vmop_create_par vm->vm_tty = -1; vm->vm_receive_fd = -1; vm->vm_state &= ~VM_STATE_PAUSED; - vm->vm_user = usr; for (i = 0; i < VMM_MAX_DISKS_PER_VM; i++) for (j = 0; j < VM_MAX_BASE_PER_DISK; j++) @@ -1903,104 +1890,6 @@ struct vmd_user * return (NULL); } -struct vmd_user * -user_get(uid_t uid) -{ - struct vmd_user *usr; - - if (uid == 0) - return (NULL); - - /* first try to find an existing user */ - TAILQ_FOREACH(usr, env->vmd_users, usr_entry) { - if (usr->usr_id.uid == uid) - goto done; - } - - if ((usr = calloc(1, sizeof(*usr))) == NULL) { - log_warn("could not allocate user"); - return (NULL); - } - - usr->usr_id.uid = uid; - usr->usr_id.gid = -1; - TAILQ_INSERT_TAIL(env->vmd_users, usr, usr_entry); - - done: - DPRINTF("%s: uid %d #%d +", - __func__, usr->usr_id.uid, usr->usr_refcnt + 1); - usr->usr_refcnt++; - - return (usr); -} - -void -user_put(struct vmd_user *usr) -{ - if (usr == NULL) - return; - - DPRINTF("%s: uid %d #%d -", - __func__, usr->usr_id.uid, usr->usr_refcnt - 1); - - if (--usr->usr_refcnt > 0) - return; - - TAILQ_REMOVE(env->vmd_users, usr, usr_entry); - free(usr); -} - -void -user_inc(struct vm_create_params *vcp, struct vmd_user *usr, int inc) -{ - char mem[FMT_SCALED_STRSIZE]; - - if (usr == NULL) - return; - - /* increment or decrement counters */ - inc = inc ? 1 : -1; - - usr->usr_maxcpu += vcp->vcp_ncpus * inc; - usr->usr_maxmem += vcp->vcp_memranges[0].vmr_size * inc; - usr->usr_maxifs += vcp->vcp_nnics * inc; - - if (log_getverbose() > 1) { - (void)fmt_scaled(usr->usr_maxmem * 1024 * 1024, mem); - log_debug("%s: %c uid %d ref %d cpu %llu mem %s ifs %llu", - __func__, inc == 1 ? '+' : '-', - usr->usr_id.uid, usr->usr_refcnt, - usr->usr_maxcpu, mem, usr->usr_maxifs); - } -} - -int -user_checklimit(struct vmd_user *usr, struct vm_create_params *vcp) -{ - const char *limit = ""; - - /* XXX make the limits configurable */ - if (usr->usr_maxcpu > VM_DEFAULT_USER_MAXCPU) { - limit = "cpu "; - goto fail; - } - if (usr->usr_maxmem > VM_DEFAULT_USER_MAXMEM) { - limit = "memory "; - goto fail; - } - if (usr->usr_maxifs > VM_DEFAULT_USER_MAXIFS) { - limit = "interface "; - goto fail; - } - - return (0); - - fail: - log_warnx("%s: user %d %slimit reached", vcp->vcp_name, - usr->usr_id.uid, limit); - return (-1); -} - char * get_string(uint8_t *ptr, size_t len) { blob - 9010ad6eb9f4b593a6b74d69b6109bd68b9e585c blob + 5e9f81fc8fd2d3d6245cede0503628ecd0482320 --- usr.sbin/vmd/vmd.h +++ usr.sbin/vmd/vmd.h @@ -65,11 +65,6 @@ #define VM_START_RATE_SEC 6 /* min. seconds since last reboot */ #define VM_START_RATE_LIMIT 3 /* max. number of fast reboots */ -/* default user instance limits */ -#define VM_DEFAULT_USER_MAXCPU 4 -#define VM_DEFAULT_USER_MAXMEM 2048 -#define VM_DEFAULT_USER_MAXIFS 8 - /* vmd -> vmctl error codes */ #define VMD_BIOS_MISSING 1001 #define VMD_DISK_MISSING 1002 @@ -287,7 +282,6 @@ struct vmd_vm { struct imsgev vm_iev; uid_t vm_uid; int vm_receive_fd; - struct vmd_user *vm_user; unsigned int vm_state; /* When set, VM is running now (PROC_PARENT only) */ #define VM_STATE_RUNNING 0x01 @@ -307,17 +301,6 @@ struct vmd_user { }; TAILQ_HEAD(vmlist, vmd_vm); -struct vmd_user { - struct vmop_owner usr_id; - uint64_t usr_maxcpu; - uint64_t usr_maxmem; - uint64_t usr_maxifs; - int usr_refcnt; - - TAILQ_ENTRY(vmd_user) usr_entry; -}; -TAILQ_HEAD(userlist, vmd_user); - struct name2id { char name[VMM_MAX_NAME_LEN]; int uid; @@ -373,7 +356,6 @@ struct vmd { struct name2idlist *vmd_known; uint32_t vmd_nswitches; struct switchlist *vmd_switches; - struct userlist *vmd_users; int vmd_fd; int vmd_fd6; @@ -445,10 +427,6 @@ struct vmd_user *user_get(uid_t); void vm_closetty(struct vmd_vm *); void switch_remove(struct vmd_switch *); struct vmd_switch *switch_getbyname(const char *); -struct vmd_user *user_get(uid_t); -void user_put(struct vmd_user *); -void user_inc(struct vm_create_params *, struct vmd_user *, int); -int user_checklimit(struct vmd_user *, struct vm_create_params *); char *get_string(uint8_t *, size_t); uint32_t prefixlen2mask(uint8_t); void prefixlen2mask6(u_int8_t, struct in6_addr *);