Download raw body.
fix accidental vm reboot during `halt -p`
ping
Dave Voutila <dv@sisu.io> writes:
> 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`