Download raw body.
fix accidental vm reboot during `halt -p`
Dave Voutila <dv@sisu.io> writes:
> claudio brought this to my attention recently and it took some thread
> pulling to figure out.
>
> If one runs `halt -p` in an OpenBSD guest under vmd(8) and then smacks
> ENTER as soon as you see:
>
> The operating system has halted.
> Please press any key to reboot.
>
> ...you'll end up with a rebooting vm because you won a race with a
> timeout in vmd. (The tl;dr: we don't really implement "poweroff" support
> in vmd yet, so we anticipate the triple-fault used for reboot and just
> don't re-create the vm.)
>
> I'm not too happy with the diff because it perpetuates some
> BadIdeas(tm), mainly how it's using imsg_flush(3) and relying on the
> boothowto kernel global, but this diff "works for me."
>
> To test, make sure you build a kernel with the vmmci(4) changes and
> install or boot *that* kernel in the guest. You can either copy it into
> the guest or use the `-b` option with vmctl(8) to provide an override
> kernel.
>
Now looking for ok's.
> -dv
>
>
> diffstat refs/heads/master refs/heads/vmmci-reboot
> M sys/dev/pv/vmmci.c | 8+ 3-
> M usr.sbin/vmd/virtio.c | 8+ 3-
> M usr.sbin/vmd/vm.c | 10+ 13-
> M usr.sbin/vmd/vmd.h | 2+ 1-
>
> 4 files changed, 28 insertions(+), 20 deletions(-)
>
> diff refs/heads/master refs/heads/vmmci-reboot
> commit - e283d8fbe4710b00487a897b3ac59307fdc34b04
> commit + fbc277543dd1b43724bb914838fd4059ac2f5a14
> blob - 580228bdb4cb6d7fac30248b07b4c920b427a471
> blob + 665e7389dc3372c28f60f674f8ccd124b44b4d24
> --- sys/dev/pv/vmmci.c
> +++ sys/dev/pv/vmmci.c
> @@ -21,6 +21,7 @@
> #include <sys/timeout.h>
> #include <sys/device.h>
> #include <sys/sensors.h>
> +#include <sys/reboot.h>
>
> #include <machine/bus.h>
>
> @@ -123,10 +124,14 @@ vmmci_activate(struct device *self, int act)
> {
> struct vmmci_softc *sc = (struct vmmci_softc *)self;
> struct virtio_softc *vsc = sc->sc_virtio;
> + int powerdown = 0;
>
> if (virtio_has_feature(vsc, VMMCI_F_ACK) == 0)
> return (0);
>
> + if (boothowto & RB_HALT)
> + powerdown = 1;
> +
> switch (act) {
> case DVACT_POWERDOWN:
> printf("%s: powerdown\n", sc->sc_dev.dv_xname);
> @@ -139,7 +144,7 @@ vmmci_activate(struct device *self, int act)
> * without hooking into the MD code directly.
> */
> virtio_write_device_config_4(vsc, VMMCI_CONFIG_COMMAND,
> - VMMCI_SHUTDOWN);
> + powerdown ? VMMCI_SHUTDOWN : VMMCI_REBOOT);
> break;
> default:
> break;
> @@ -172,10 +177,10 @@ vmmci_config_change(struct virtio_softc *vsc)
> case VMMCI_SYNCRTC:
> inittodr(gettime());
> sc->sc_cmd = VMMCI_NONE;
> - break;
> + break;
> default:
> printf("%s: invalid command %d\n", sc->sc_dev.dv_xname, cmd);
> - cmd = VMMCI_NONE;
> + cmd = VMMCI_NONE;
> break;
> }
>
> blob - fa924bfc2640497ee042b7ef089b3822e6f29502
> blob + 50b2d93fef83a043746b87c3b6d56b087cf64885
> --- usr.sbin/vmd/virtio.c
> +++ usr.sbin/vmd/virtio.c
> @@ -349,10 +349,10 @@ vmmci_ack(unsigned int cmd)
> vmmci.vm_id);
> tv.tv_sec = VMMCI_TIMEOUT;
> evtimer_add(&vmmci.timeout, &tv);
> + vm_notify_vmm(1);
> return;
> }
> - /* FALLTHROUGH */
> - case VMMCI_REBOOT:
> +
> /*
> * If the VM acknowledged our shutdown request, give it
> * enough time to shutdown or reboot gracefully. This
> @@ -365,9 +365,14 @@ vmmci_ack(unsigned int cmd)
> log_debug("%s: vm %u acknowledged shutdown request",
> __func__, vmmci.vm_id);
> tv.tv_sec = VMMCI_SHUTDOWN_TIMEOUT;
> + vm_notify_vmm(1);
> evtimer_add(&vmmci.timeout, &tv);
> }
> break;
> + case VMMCI_REBOOT:
> + log_debug("%s: vm %u requested reboot", __func__, vmmci.vm_id);
> + vm_notify_vmm(0);
> + break;
> case VMMCI_SYNCRTC:
> log_debug("%s: vm %u acknowledged RTC sync request",
> __func__, vmmci.vm_id);
> @@ -383,7 +388,7 @@ void
> vmmci_timeout(int fd, short type, void *arg)
> {
> log_debug("%s: vm %u shutdown", __progname, vmmci.vm_id);
> - vm_shutdown(vmmci.cmd == VMMCI_REBOOT ? VMMCI_REBOOT : VMMCI_SHUTDOWN);
> + vm_shutdown();
> }
>
> int
> blob - 8b94fbc8a590072cfdc94344abe6b0fd3f28e01e
> blob + 679ab17c64e2e9371d80b178e4859d77f53d5df6
> --- usr.sbin/vmd/vm.c
> +++ usr.sbin/vmd/vm.c
> @@ -439,28 +439,25 @@ vm_dispatch_vmm(int fd, short event, void *arg)
> }
>
> /*
> - * vm_shutdown
> + * vm_notify_vmm
> *
> - * Tell the vmm parent process to shutdown or reboot the VM and exit.
> + * Tell the vmm parent process to shutdown or reboot the VM on process exit.
> */
> -__dead void
> -vm_shutdown(unsigned int cmd)
> +void
> +vm_notify_vmm(int shutdown)
> {
> - switch (cmd) {
> - case VMMCI_NONE:
> - case VMMCI_SHUTDOWN:
> + if (shutdown)
> (void)imsg_compose_event(¤t_vm->vm_iev,
> IMSG_VMDOP_VM_SHUTDOWN, 0, 0, -1, NULL, 0);
> - break;
> - case VMMCI_REBOOT:
> + else
> (void)imsg_compose_event(¤t_vm->vm_iev,
> IMSG_VMDOP_VM_REBOOT, 0, 0, -1, NULL, 0);
> - break;
> - default:
> - fatalx("invalid vm ctl command: %d", cmd);
> - }
> imsg_flush(¤t_vm->vm_iev.ibuf);
> +}
>
> +__dead void
> +vm_shutdown(void)
> +{
> if (sev_shutdown(current_vm))
> log_warnx("%s: could not shutdown SEV", __func__);
>
> blob - b84656fc20c20c9a6cdfff4a1b7b420b2f472bac
> blob + 8866574bc1baf1424fee826ac897df11d0f932ec
> --- usr.sbin/vmd/vmd.h
> +++ usr.sbin/vmd/vmd.h
> @@ -547,7 +547,8 @@ void vm_pipe_send(struct vm_dev_pipe *, enum pipe_msg
> enum pipe_msg_type vm_pipe_recv(struct vm_dev_pipe *);
> int write_mem(paddr_t, const void *buf, size_t);
> int remap_guest_mem(struct vmd_vm *, int);
> -__dead void vm_shutdown(unsigned int);
> +__dead void vm_shutdown(void);
> +void vm_notify_vmm(int);
>
> /* config.c */
> int config_init(struct vmd *);
fix accidental vm reboot during `halt -p`