In vmd, the vmm process forks to create the resulting vm process. After
this fork, the vmm parent process closes all the file descriptors
pointing to the vm's devices (cdrom, kernel, disks, nics, etc.).
The logic was a bit funky, so this change relies on the fact we can
attempt the close(2) call and use its success/failure to determine if we
have an fd to mark -1 in the vm structure. (I guess we could just
blindly set them to -1 post-close, but this feels more sensical to me.)
While in the function, it cleans up some commentary and logging around
the actual fork(2) call. Since fork(2) sets errno, we can use it in the
log message, too.
This reduces some noise in my upcoming diff to introduce execvp to the
forking of child vm processes (as presented at AsiaBSDCon).
9 insertions, 18 deletions :)
ok?
diff refs/heads/master refs/heads/vmd-vmm-fd-dance
commit - 8371c6b4d5765a69d520f93d64f57c6a2989f9ae
commit + ebe7117d30cfe8a792d4c2e9a05f3b9b3e86c651
blob - d9eff3c8f703854c7b1e49fee04b8e426956cfbb
blob + 859e27652ba129a23c6e4ff510dab5bafaac653a
--- usr.sbin/vmd/vmm.c
+++ usr.sbin/vmd/vmm.c
@@ -641,12 +641,10 @@ vmm_start_vm(struct imsg *imsg, uint32_t *id, pid_t *p
if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, fds) == -1)
fatal("socketpair");
- /* Start child vmd for this VM (fork, chroot, drop privs) */
+ /* Fork the vmm process to create the vm, inheriting open device fds. */
vm_pid = fork();
-
- /* Start child failed? - cleanup and leave */
if (vm_pid == -1) {
- log_warnx("%s: start child failed", __func__);
+ log_warn("%s: fork child failed", __func__);
ret = EIO;
goto err;
}
@@ -658,27 +656,20 @@ vmm_start_vm(struct imsg *imsg, uint32_t *id, pid_t *p
for (i = 0 ; i < vcp->vcp_ndisks; i++) {
for (j = 0; j < VM_MAX_BASE_PER_DISK; j++) {
- if (vm->vm_disks[i][j] != -1)
- close(vm->vm_disks[i][j]);
- vm->vm_disks[i][j] = -1;
+ if (close(vm->vm_disks[i][j]) == 0)
+ vm->vm_disks[i][j] = -1;
}
}
for (i = 0 ; i < vcp->vcp_nnics; i++) {
- close(vm->vm_ifs[i].vif_fd);
- vm->vm_ifs[i].vif_fd = -1;
+ if (close(vm->vm_ifs[i].vif_fd) == 0)
+ vm->vm_ifs[i].vif_fd = -1;
}
- if (vm->vm_kernel != -1) {
- close(vm->vm_kernel);
+ if (close(vm->vm_kernel) == 0)
vm->vm_kernel = -1;
- }
- if (vm->vm_cdrom != -1) {
- close(vm->vm_cdrom);
+ if (close(vm->vm_cdrom) == 0)
vm->vm_cdrom = -1;
- }
- if (vm->vm_tty != -1) {
- close(vm->vm_tty);
+ if (close(vm->vm_tty) == 0)
vm->vm_tty = -1;
- }
/* Read back the kernel-generated vm id from the child */
sz = atomicio(read, fds[0], &vcp->vcp_id, sizeof(vcp->vcp_id));
