On Wed, Oct 05, 2022 at 05:03:16PM -0400, Dave Voutila wrote:
> 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
For what it's worth this works for me (I can use double-p's packer
builder with the diff). Thanks
> 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 *);
