From: Mike Larkin <mlarkin@nested.page>
Subject: Re: fix vmctl {reload,reset} hanging on socket receive
To: Dave Voutila <dv@sisu.io>
Cc: tech@openbsd.org
Date: Sat, 27 Sep 2025 03:47:34 -0700

On Mon, Sep 22, 2025 at 11:43:26AM -0400, Dave Voutila wrote:
> As reported on tech@ [1], `vmctl {reload,reset}` currently hangs waiting
> for an imsg reply from vmd's control process. Tracked this down to a bug
> I introduced recently when reworking imsg handling.
>
> In my changes, I removed IMSG_NONE as it appeared unused since it
> explicitly didn't appear in the code. Apparently 0 was being used
> instead to convey the same meaning. :(
>
> This retores the IMSG_NONE value and cleans it up to be more explicit to
> avoid this facepalm in the future. I guess touching proc.{c,h} never
> goes without some suffering.
>
> If testing, this requires rebuilding vmctl(8), too, as it shares headers
> from vmd(8).
>
> ok?
>

Sorry, late to the party here but this is fine. ok mlarkin after the fact
as I see you've already fixed it.

Thanks.

-ml

> -dv
>
>
> [1] https://marc.info/?l=openbsd-misc&m=175846285608823&w=2
>
> diff refs/heads/master refs/heads/vmd-vmctl-reload
> commit - adfd21e9d9de44ec58cb6572dda9c677e59b7964
> commit + 7ff5713e159aefb00396e9ec18f2eb2252351fd0
> blob - 581739abf59d3d1bf0d9e026817c955fc17e6fe8
> blob + 45d39356bb9d0bc9ce2a6f026f7b1fcbdb712711
> --- usr.sbin/vmd/proc.h
> +++ usr.sbin/vmd/proc.h
> @@ -26,6 +26,7 @@
>  #define _PROC_H
>
>  enum {
> +	IMSG_NONE = 0,
>  	IMSG_CTL_OK,
>  	IMSG_CTL_FAIL,
>  	IMSG_CTL_VERBOSE,
> blob - 6132f15735b54204e3743df3bd16827a62bc74b6
> blob + 627d0f3f3adbfda62025c08aa6b4ddd580dfcae0
> --- usr.sbin/vmd/vmd.c
> +++ usr.sbin/vmd/vmd.c
> @@ -91,7 +91,7 @@ int
>  vmd_dispatch_control(int fd, struct privsep_proc *p, struct imsg *imsg)
>  {
>  	struct privsep			*ps = p->p_ps;
> -	int				 res = 0, cmd = 0, verbose;
> +	int				 res = 0, cmd = IMSG_NONE, verbose;
>  	unsigned int			 v = 0, flags;
>  	struct vmop_create_params	 vmc;
>  	struct vmop_id			 vid;
> @@ -251,14 +251,13 @@ vmd_dispatch_control(int fd, struct privsep_proc *p, s
>  		control_reset(&ps->ps_csock);
>  		TAILQ_FOREACH(rcs, &ps->ps_rcsocks, cs_entry)
>  			control_reset(rcs);
> -		cmd = 0;
>  		break;
>  	default:
>  		return (-1);
>  	}
>
>  	switch (cmd) {
> -	case 0:
> +	case IMSG_NONE:
>  		break;
>  	case IMSG_VMDOP_START_VM_RESPONSE:
>  	case IMSG_VMDOP_TERMINATE_VM_RESPONSE:
> blob - 8a33e351745a63401ebff65e6d301766d1f883da
> blob + 08b51df37960de8964cfb2903af8a835da0b39cc
> --- usr.sbin/vmd/vmm.c
> +++ usr.sbin/vmd/vmm.c
> @@ -95,7 +95,7 @@ int
>  vmm_dispatch_parent(int fd, struct privsep_proc *p, struct imsg *imsg)
>  {
>  	struct privsep		*ps = p->p_ps;
> -	int			 res = 0, cmd = 0, verbose;
> +	int			 res = 0, cmd = IMSG_NONE, verbose;
>  	struct vmd_vm		*vm = NULL;
>  	struct vm_terminate_params vtp;
>  	struct vmop_id		 vid;
>