From: Dave Voutila Subject: improve vmd error message when out of tap(4)'s To: Tech Date: Wed, 03 Jan 2024 09:51:14 -0500 The below diff improves the error messages related to opening the "next available" tap(4) device when using vmd(8). I feel this pops up every once in awhile and just did again [1] on misc@, so about time we improve it a bit. We really need either custom error codes or something sent back to vmctl and not just piggy backing on errnos as they often don't make sense. That's a diff for another day as it's going to be pretty invasive. This one still relies on ENOENT being communicated...not the best but far better than "Unknown error". The bigger imrovement is in the vmd logging pointing out the tap we couldn't open. ok? before the diff: # for i in `jot 5`; do vmctl start -L -b /bsd.rd "test-${i}"; done vmctl: started vm 5 successfully, tty /dev/ttyp4 vmctl: started vm 6 successfully, tty /dev/ttyp5 vmctl: started vm 7 successfully, tty /dev/ttyp6 vmctl: started vm 8 successfully, tty /dev/ttyp7 vmctl: start vm command failed: Unknown error: -1 # $(which vmd) -dv vmd: startup vmd: started test-1 (vm 5) successfully, tty /dev/ttyp4 vmd: started test-2 (vm 6) successfully, tty /dev/ttyp5 vmd: started test-3 (vm 7) successfully, tty /dev/ttyp6 vmd: started test-4 (vm 8) successfully, tty /dev/ttyp7 vmd: config_setvm: can't open tap tap vmd: failed to start vm test-5 w/ the diff: # for i in `jot 5`; do vmctl start -L -b /bsd.rd "test-${i}"; done vmctl: started vm 5 successfully, tty /dev/ttyp4 vmctl: started vm 6 successfully, tty /dev/ttyp5 vmctl: started vm 7 successfully, tty /dev/ttyp6 vmctl: started vm 8 successfully, tty /dev/ttyp7 vmctl: start vm command failed: No such file or directory # $(which vmd) -dv vmd: startup vmd: started test-1 (vm 5) successfully, tty /dev/ttyp4 vmd: started test-2 (vm 6) successfully, tty /dev/ttyp5 vmd: started test-3 (vm 7) successfully, tty /dev/ttyp6 vmd: started test-4 (vm 8) successfully, tty /dev/ttyp7 vmd: config_setvm: can't open /dev/tap4 vmd: failed to start vm test-5 [1] https://marc.info/?l=openbsd-misc&m=170421999315682&w=2 -dv@ diffstat refs/heads/master refs/heads/vmd-tap M usr.sbin/vmd/config.c | 2+ 1- M usr.sbin/vmd/vmm.c | 23+ 8- 2 files changed, 25 insertions(+), 9 deletions(-) diff refs/heads/master refs/heads/vmd-tap commit - b98228b267823b1070a646dd33a2951b3fcc0082 commit + eae7a7dce824b151f8307a72f0be9673441175e7 blob - 3bb246a0092cbd4318721794c81ee11964528820 blob + 23afa2ddf617bbfefcc66a4ef46ec34a990fd27d --- usr.sbin/vmd/config.c +++ usr.sbin/vmd/config.c @@ -417,7 +417,8 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, ui s = ifname; } if (tapfds[i] == -1) { - log_warnx("%s: can't open tap %s", __func__, s); + ret = errno; + log_warnx("%s: can't open /dev/%s", __func__, s); goto fail; } if ((vif->vif_name = strdup(s)) == NULL) { blob - b6b138a6e3f1cc8660e95fb2b6668be256a36308 blob + 7bc6adf721df80d51b3a0775a3e633d00eb7a2fc --- usr.sbin/vmd/vmm.c +++ usr.sbin/vmd/vmm.c @@ -586,26 +586,41 @@ terminate_vm(struct vm_terminate_params *vtp) * Parameters * ifname: a buffer of at least IF_NAMESIZE bytes. * - * Returns a file descriptor to the tap node opened, or -1 if no tap - * devices were available. + * Returns a file descriptor to the tap node opened or -1 if no tap devices were + * available, setting errno to the open(2) error. */ int opentap(char *ifname) { - int i, fd; + int err = 0, i, fd; char path[PATH_MAX]; for (i = 0; i < MAX_TAP; i++) { snprintf(path, PATH_MAX, "/dev/tap%d", i); + + errno = 0; fd = open(path, O_RDWR | O_NONBLOCK); - if (fd != -1) { - snprintf(ifname, IF_NAMESIZE, "tap%d", i); - return (fd); + if (fd != -1) + break; + err = errno; + if (err == EBUSY) { + /* Busy...try next tap. */ + continue; + } else if (err == ENOENT) { + /* Ran out of /dev/tap* special files. */ + break; + } else { + log_warn("%s: unexpected error", __func__); + break; } } - strlcpy(ifname, "tap", IF_NAMESIZE); - return (-1); + /* Record the last opened tap device. */ + snprintf(ifname, IF_NAMESIZE, "tap%d", i); + + if (err) + errno = err; + return (fd); } /*