Download raw body.
disambiguate fd variable naming in vmd
Boring but helpful diff leading up to my sharing a much larger one. The
global instance of struct vmd uses its vmd_fd member for multiple things
depending on the vmd process. In one case, it's a socket(2)...in
another, an open vmm(4).
This redesigns struct vmd to have distinct members named based on usage
to make vmd slightly easier to reason about.
Bonus tweaks: properly initializes the SEV, ptm, and socket fd's to -1
and changes a logic check from the strange semantic of "> -1" to "!=
-1". (At no point should 0 be a valid fd value for the psp or vmm
devices.)
ok?
diffstat refs/heads/master refs/heads/vmd-fd-cleanup-simple
M usr.sbin/vmd/priv.c | 12+ 12-
M usr.sbin/vmd/virtio.c | 1+ 1-
M usr.sbin/vmd/vm.c | 5+ 5-
M usr.sbin/vmd/vmd.c | 11+ 7-
M usr.sbin/vmd/vmd.h | 4+ 3-
M usr.sbin/vmd/vmm.c | 7+ 7-
6 files changed, 40 insertions(+), 35 deletions(-)
diff refs/heads/master refs/heads/vmd-fd-cleanup-simple
commit - 9598e7ed943eac95d9532381b8c8e3bb46321fbd
commit + 291de405160c12f78e9103b08983b3dae43d372e
blob - 5c7338ba24c7d867fde84a2b1d1eddcdac03b5a7
blob + d8739aa6010babc401b91f0d0277ff6ac7793657
--- usr.sbin/vmd/priv.c
+++ usr.sbin/vmd/priv.c
@@ -63,11 +63,11 @@ priv_run(struct privsep *ps, struct privsep_proc *p, v
*/
/* Open our own socket for generic interface ioctls */
- if ((env->vmd_fd = socket(AF_INET, SOCK_DGRAM, 0)) == -1)
+ if ((env->vmd_sock_fd = socket(AF_INET, SOCK_DGRAM, 0)) == -1)
fatal("socket");
/* But we need a different fd for IPv6 */
- if ((env->vmd_fd6 = socket(AF_INET6, SOCK_DGRAM, 0)) == -1)
+ if ((env->vmd_sock_fd6 = socket(AF_INET6, SOCK_DGRAM, 0)) == -1)
fatal("socket6");
}
@@ -124,13 +124,13 @@ priv_dispatch_parent(int fd, struct privsep_proc *p, s
/* Set the interface description */
strlcpy(ifr.ifr_name, vfr.vfr_name, sizeof(ifr.ifr_name));
ifr.ifr_data = (caddr_t)vfr.vfr_value;
- if (ioctl(env->vmd_fd, SIOCSIFDESCR, &ifr) == -1)
+ if (ioctl(env->vmd_sock_fd, SIOCSIFDESCR, &ifr) == -1)
log_warn("SIOCSIFDESCR");
break;
case IMSG_VMDOP_PRIV_IFRDOMAIN:
strlcpy(ifr.ifr_name, vfr.vfr_name, sizeof(ifr.ifr_name));
ifr.ifr_rdomainid = vfr.vfr_id;
- if (ioctl(env->vmd_fd, SIOCSIFRDOMAIN, &ifr) == -1)
+ if (ioctl(env->vmd_sock_fd, SIOCSIFRDOMAIN, &ifr) == -1)
log_warn("SIOCSIFRDOMAIN");
break;
case IMSG_VMDOP_PRIV_IFADD:
@@ -143,14 +143,14 @@ priv_dispatch_parent(int fd, struct privsep_proc *p, s
sizeof(ifbr.ifbr_name));
strlcpy(ifbr.ifbr_ifsname, vfr.vfr_value,
sizeof(ifbr.ifbr_ifsname));
- if (ioctl(env->vmd_fd, SIOCBRDGADD, &ifbr) == -1 &&
+ if (ioctl(env->vmd_sock_fd, SIOCBRDGADD, &ifbr) == -1 &&
errno != EEXIST)
log_warn("SIOCBRDGADD");
break;
case IMSG_VMDOP_PRIV_IFEXISTS:
/* Determine if bridge exists */
strlcpy(ifr.ifr_name, vfr.vfr_name, sizeof(ifr.ifr_name));
- if (ioctl(env->vmd_fd, SIOCGIFFLAGS, &ifr) == -1)
+ if (ioctl(env->vmd_sock_fd, SIOCGIFFLAGS, &ifr) == -1)
fatalx("%s: bridge \"%s\" does not exist",
__func__, vfr.vfr_name);
break;
@@ -158,7 +158,7 @@ priv_dispatch_parent(int fd, struct privsep_proc *p, s
case IMSG_VMDOP_PRIV_IFDOWN:
/* Set the interface status */
strlcpy(ifr.ifr_name, vfr.vfr_name, sizeof(ifr.ifr_name));
- if (ioctl(env->vmd_fd, SIOCGIFFLAGS, &ifr) == -1) {
+ if (ioctl(env->vmd_sock_fd, SIOCGIFFLAGS, &ifr) == -1) {
log_warn("SIOCGIFFLAGS");
break;
}
@@ -166,7 +166,7 @@ priv_dispatch_parent(int fd, struct privsep_proc *p, s
ifr.ifr_flags |= IFF_UP;
else
ifr.ifr_flags &= ~IFF_UP;
- if (ioctl(env->vmd_fd, SIOCSIFFLAGS, &ifr) == -1)
+ if (ioctl(env->vmd_sock_fd, SIOCSIFFLAGS, &ifr) == -1)
log_warn("SIOCSIFFLAGS");
break;
case IMSG_VMDOP_PRIV_IFGROUP:
@@ -179,7 +179,7 @@ priv_dispatch_parent(int fd, struct privsep_proc *p, s
sizeof(ifgr.ifgr_group)) >= sizeof(ifgr.ifgr_group))
fatalx("%s: group name too long", __func__);
- if (ioctl(env->vmd_fd, SIOCAIFGROUP, &ifgr) == -1 &&
+ if (ioctl(env->vmd_sock_fd, SIOCAIFGROUP, &ifgr) == -1 &&
errno != EEXIST)
log_warn("SIOCAIFGROUP");
break;
@@ -198,7 +198,7 @@ priv_dispatch_parent(int fd, struct privsep_proc *p, s
memcpy(&ifra.ifra_mask, &vfr.vfr_mask,
sizeof(ifra.ifra_mask));
- if (ioctl(env->vmd_fd, SIOCAIFADDR, &ifra) == -1)
+ if (ioctl(env->vmd_sock_fd, SIOCAIFADDR, &ifra) == -1)
log_warn("SIOCAIFADDR");
break;
case IMSG_VMDOP_PRIV_IFADDR6:
@@ -221,11 +221,11 @@ priv_dispatch_parent(int fd, struct privsep_proc *p, s
in6_ifra.ifra_lifetime.ia6t_vltime = ND6_INFINITE_LIFETIME;
in6_ifra.ifra_lifetime.ia6t_pltime = ND6_INFINITE_LIFETIME;
- if (ioctl(env->vmd_fd6, SIOCDIFADDR_IN6, &in6_ifra) == -1 &&
+ if (ioctl(env->vmd_sock_fd6, SIOCDIFADDR_IN6, &in6_ifra) == -1 &&
errno != EADDRNOTAVAIL)
log_warn("SIOCDIFADDR_IN6");
- if (ioctl(env->vmd_fd6, SIOCAIFADDR_IN6, &in6_ifra) == -1)
+ if (ioctl(env->vmd_sock_fd6, SIOCAIFADDR_IN6, &in6_ifra) == -1)
log_warn("SIOCAIFADDR_IN6");
break;
case IMSG_VMDOP_PRIV_GET_ADDR:
blob - 75f783d8fa8aeadc181a952c091ee2d6d7de85d1
blob + d10040759267eb24bfb991643fef02be88b0a5a8
--- usr.sbin/vmd/virtio.c
+++ usr.sbin/vmd/virtio.c
@@ -1657,7 +1657,7 @@ virtio_dev_launch(struct vmd_vm *vm, struct virtio_dev
memset(num, 0, sizeof(num));
snprintf(num, sizeof(num), "%d", sync_fds[1]);
memset(vmm_fd, 0, sizeof(vmm_fd));
- snprintf(vmm_fd, sizeof(vmm_fd), "%d", env->vmd_fd);
+ snprintf(vmm_fd, sizeof(vmm_fd), "%d", env->vmd_vmm_fd);
memset(vm_name, 0, sizeof(vm_name));
snprintf(vm_name, sizeof(vm_name), "%s",
vm->vm_params.vmc_name);
blob - e2a7451dfa63ea8c8b512766e48acbcb2533e4be
blob + 66b6e2f705591cb9a43d9a82d563132ad1a57924
--- usr.sbin/vmd/vm.c
+++ usr.sbin/vmd/vm.c
@@ -89,7 +89,7 @@ vm_main(int fd, int fd_vmm)
/*
* The vm process relies on global state. Set the fd for /dev/vmm.
*/
- env->vmd_fd = fd_vmm;
+ env->vmd_vmm_fd = fd_vmm;
/*
* We aren't root, so we can't chroot(2). Use unveil(2) instead.
@@ -507,7 +507,7 @@ vcpu_reset(uint32_t vmid, uint32_t vcpu_id, struct vcp
log_debug("%s: resetting vcpu %d for vm %d", __func__, vcpu_id, vmid);
- if (ioctl(env->vmd_fd, VMM_IOC_RESETCPU, &vrp) == -1)
+ if (ioctl(env->vmd_vmm_fd, VMM_IOC_RESETCPU, &vrp) == -1)
return (errno);
return (0);
@@ -557,7 +557,7 @@ vmm_create_vm(struct vmd_vm *vm)
vcp.vcp_sev = vmc->vmc_sev;
vcp.vcp_seves = vmc->vmc_seves;
- if (ioctl(env->vmd_fd, VMM_IOC_CREATE, &vcp) == -1)
+ if (ioctl(env->vmd_vmm_fd, VMM_IOC_CREATE, &vcp) == -1)
return (errno);
vm->vm_vmmid = vcp.vcp_id;
@@ -894,7 +894,7 @@ vcpu_run_loop(void *arg)
/* Still more interrupts pending? */
vrp->vrp_intr_pending = intr_pending(current_vm);
- if (ioctl(env->vmd_fd, VMM_IOC_RUN, vrp) == -1) {
+ if (ioctl(env->vmd_vmm_fd, VMM_IOC_RUN, vrp) == -1) {
/* If run ioctl failed, exit */
ret = errno;
log_warn("%s: vm %d / vcpu %d run ioctl failed",
@@ -941,7 +941,7 @@ vcpu_intr(uint32_t vmm_id, uint32_t vcpu_id, uint8_t i
vip.vip_vcpu_id = vcpu_id; /* XXX always 0? */
vip.vip_intr = intr;
- if (ioctl(env->vmd_fd, VMM_IOC_INTR, &vip) == -1)
+ if (ioctl(env->vmd_vmm_fd, VMM_IOC_INTR, &vip) == -1)
return (errno);
return (0);
blob - 0a28c186bf2d0c9905123fe5d1f2701cbc287c77
blob + dd4720fad2ac58c09a34d105ef3b3cd58bc49de7
--- usr.sbin/vmd/vmd.c
+++ usr.sbin/vmd/vmd.c
@@ -549,9 +549,13 @@ main(int argc, char **argv)
if ((env = calloc(1, sizeof(*env))) == NULL)
fatal("calloc: env");
- env->vmd_fd = -1;
- env->vmd_fd6 = -1;
+ env->vmd_ptm_fd = -1;
+ env->vmd_psp_fd = -1;
+ env->vmd_sock_fd = -1;
+ env->vmd_sock_fd6 = -1;
+ env->vmd_vmm_fd = -1;
+
while ((ch = getopt(argc, argv, "D:P:V:X:df:i:j:nt:vp:")) != -1) {
switch (ch) {
case 'D':
@@ -680,8 +684,8 @@ main(int argc, char **argv)
/* Open /dev/vmm early. */
if (env->vmd_noaction == 0 && proc_id == PROC_PARENT) {
- env->vmd_fd = open(VMM_NODE, O_RDWR | O_CLOEXEC);
- if (env->vmd_fd == -1)
+ env->vmd_vmm_fd = open(VMM_NODE, O_RDWR | O_CLOEXEC);
+ if (env->vmd_vmm_fd == -1)
fatal("%s", VMM_NODE);
}
@@ -785,7 +789,7 @@ vmd_configure(void)
" chown fattr flock", NULL) == -1)
fatal("pledge");
- if ((env->vmd_ptmfd = getptmfd()) == -1)
+ if ((env->vmd_ptm_fd = getptmfd()) == -1)
fatal("getptmfd %s", PATH_PTMDEV);
if (parse_config(env->vmd_conffile) == -1) {
@@ -801,7 +805,7 @@ vmd_configure(void)
/* Send VMM device fd to vmm proc. */
proc_compose_imsg(&env->vmd_ps, PROC_VMM,
- IMSG_VMDOP_RECEIVE_VMM_FD, -1, env->vmd_fd, NULL, 0);
+ IMSG_VMDOP_RECEIVE_VMM_FD, -1, env->vmd_vmm_fd, NULL, 0);
/* Send PSP device fd to vmm proc. */
if (env->vmd_psp_fd != -1) {
@@ -1607,7 +1611,7 @@ vm_opentty(struct vmd_vm *vm)
/*
* Open tty with pre-opened PTM fd
*/
- if (fdopenpty(env->vmd_ptmfd, &vm->vm_tty, &tty_slave, vm->vm_ttyname,
+ if (fdopenpty(env->vmd_ptm_fd, &vm->vm_tty, &tty_slave, vm->vm_ttyname,
NULL, NULL) == -1) {
log_warn("fdopenpty");
return (-1);
blob - eced284a8b6f93751acd606272cb09deaa47df37
blob + 2dadede5d6e20b59ed8141fe67c12368cfd75ebd
--- usr.sbin/vmd/vmd.h
+++ usr.sbin/vmd/vmd.h
@@ -408,10 +408,11 @@ struct vmd {
uint32_t vmd_nswitches;
struct switchlist *vmd_switches;
- int vmd_fd;
- int vmd_fd6;
- int vmd_ptmfd;
+ int vmd_ptm_fd;
int vmd_psp_fd;
+ int vmd_sock_fd;
+ int vmd_sock_fd6;
+ int vmd_vmm_fd;
};
struct vm_dev_pipe {
blob - ebf5a5c5f6dbb41cb03f967119451be32b624396
blob + 06bd649662ea0d9af4953b13d6fe6370c3cc31f9
--- usr.sbin/vmd/vmm.c
+++ usr.sbin/vmd/vmm.c
@@ -271,15 +271,15 @@ vmm_dispatch_parent(int fd, struct privsep_proc *p, st
imsg_get_fd(imsg), &var, sizeof(var));
break;
case IMSG_VMDOP_RECEIVE_VMM_FD:
- if (env->vmd_fd > -1)
+ if (env->vmd_vmm_fd != -1)
fatalx("already received vmm fd");
- env->vmd_fd = imsg_get_fd(imsg);
+ env->vmd_vmm_fd = imsg_get_fd(imsg);
/* Get and terminate all running VMs */
get_info_vm(ps, NULL, 1);
break;
case IMSG_VMDOP_RECEIVE_PSP_FD:
- if (env->vmd_psp_fd > -1)
+ if (env->vmd_psp_fd != -1)
fatalx("already received psp fd");
env->vmd_psp_fd = imsg_get_fd(imsg);
break;
@@ -530,7 +530,7 @@ vmm_dispatch_vm(int fd, short event, void *arg)
int
terminate_vm(struct vm_terminate_params *vtp)
{
- if (ioctl(env->vmd_fd, VMM_IOC_TERM, vtp) == -1)
+ if (ioctl(env->vmd_vmm_fd, VMM_IOC_TERM, vtp) == -1)
return (errno);
return (0);
@@ -734,7 +734,7 @@ vmm_start_vm(struct imsg *imsg, uint32_t *id, pid_t *p
memset(num, 0, sizeof(num));
snprintf(num, sizeof(num), "%d", fds[1]);
memset(vmm_fd, 0, sizeof(vmm_fd));
- snprintf(vmm_fd, sizeof(vmm_fd), "%d", env->vmd_fd);
+ snprintf(vmm_fd, sizeof(vmm_fd), "%d", env->vmd_vmm_fd);
memset(psp_fd, 0, sizeof(psp_fd));
snprintf(psp_fd, sizeof(psp_fd), "%d", env->vmd_psp_fd);
@@ -817,7 +817,7 @@ get_info_vm(struct privsep *ps, struct imsg *imsg, int
memset(&vir, 0, sizeof(vir));
/* First ioctl to see how many bytes needed (vip.vip_size) */
- if (ioctl(env->vmd_fd, VMM_IOC_INFO, &vip) == -1)
+ if (ioctl(env->vmd_vmm_fd, VMM_IOC_INFO, &vip) == -1)
return (errno);
if (vip.vip_info_ct != 0)
@@ -829,7 +829,7 @@ get_info_vm(struct privsep *ps, struct imsg *imsg, int
/* Second ioctl to get the actual list */
vip.vip_info = info;
- if (ioctl(env->vmd_fd, VMM_IOC_INFO, &vip) == -1) {
+ if (ioctl(env->vmd_vmm_fd, VMM_IOC_INFO, &vip) == -1) {
ret = errno;
free(info);
return (ret);
disambiguate fd variable naming in vmd