Still looking for an OK or feedback on the below. This is finishing work
to fixes made previously to vmd(8)/vmctl(8) regarding vm
stopping/running state corruption when using vmctl(8) to wait for a vm
to stop.
Dave Voutila writes:
> ping
>
> Dave Voutila writes:
>
>> Dave Voutila writes:
>>
>>> The conclusion of my previous fixes to vmd(8) [1] changes the event
>>> handling in vmctl(8) to support receiving IMSG_VMDOP_TERMINATE_VM_EVENTs
>>> from the control process. (This removes a XXX comment from vmd.)
>>>
>>> For clarity, the messaging logic was changed previously:
>>>
>>> - ...TERMINATE_VM_RESPONSE conveying success/failure of the request to
>>> terminate a guest regardless of waiting for termination
>>> - ...TERMINATE_VM_EVENT conveying the actual termination of a guest
>>>
>>> This diff finishes bringing that logic from vmd(8) to vmctl(8).
>>>
>>> OK?
>>
>> Ping. Looking to close this gap.
>>
>> Note: this diff does preserve some errno abuse in vmd & vmctl that I'm
>> working on separately.
>>
>>>
>>> -dv
>>>
>>>
>>> Index: usr.sbin/vmd/control.c
>>> ===================================================================
>>> RCS file: /cvs/src/usr.sbin/vmd/control.c,v
>>> retrieving revision 1.35
>>> diff -u -p -r1.35 control.c
>>> --- usr.sbin/vmd/control.c 26 Apr 2021 22:58:27 -0000 1.35
>>> +++ usr.sbin/vmd/control.c 30 Apr 2021 12:31:22 -0000
>>> @@ -154,9 +154,8 @@ control_dispatch_vmd(int fd, struct priv
>>> if (notify->ctl_vmid != vmr.vmr_id)
>>> continue;
>>> if ((c = control_connbyfd(notify->ctl_fd)) != NULL) {
>>> - /* XXX vmctl expects *_RESPONSE, not *_EVENT */
>>> - imsg_compose_event(&c->iev,
>>> - IMSG_VMDOP_TERMINATE_VM_RESPONSE,
>>> + /* Forward to the vmctl(8) client */
>>> + imsg_compose_event(&c->iev, imsg->hdr.type,
>>> 0, 0, -1, imsg->data, IMSG_DATA_SIZE(imsg));
>>> TAILQ_REMOVE(&ctl_notify_q, notify, entry);
>>> free(notify);
>>> Index: usr.sbin/vmctl/vmctl.c
>>> ===================================================================
>>> RCS file: /cvs/src/usr.sbin/vmctl/vmctl.c,v
>>> retrieving revision 1.77
>>> diff -u -p -r1.77 vmctl.c
>>> --- usr.sbin/vmctl/vmctl.c 22 Mar 2021 18:50:11 -0000 1.77
>>> +++ usr.sbin/vmctl/vmctl.c 30 Apr 2021 12:31:22 -0000
>>> @@ -461,7 +461,7 @@ terminate_vm(uint32_t terminate_id, cons
>>> * terminate_vm_complete
>>> *
>>> * Callback function invoked when we are expecting an
>>> - * IMSG_VMDOP_TERMINATE_VM_RESPONSE message indicating the completion of
>>> + * IMSG_VMDOP_TERMINATE_VM_EVENT message indicating the completion of
>>> * a terminate vm operation.
>>> *
>>> * Parameters:
>>> @@ -484,41 +484,50 @@ terminate_vm_complete(struct imsg *imsg,
>>> struct vmop_result *vmr;
>>> int res;
>>>
>>> - if (imsg->hdr.type == IMSG_VMDOP_TERMINATE_VM_RESPONSE) {
>>> + switch (imsg->hdr.type) {
>>> + case IMSG_VMDOP_TERMINATE_VM_RESPONSE:
>>> + IMSG_SIZE_CHECK(imsg, &vmr);
>>> vmr = (struct vmop_result *)imsg->data;
>>> res = vmr->vmr_result;
>>> - if (res) {
>>> - switch (res) {
>>> - case VMD_VM_STOP_INVALID:
>>> - fprintf(stderr,
>>> - "cannot stop vm that is not running\n");
>>> - *ret = EINVAL;
>>> - break;
>>> - case ENOENT:
>>> - fprintf(stderr, "vm not found\n");
>>> - *ret = EIO;
>>> - break;
>>> - case EINTR:
>>> - fprintf(stderr, "interrupted call\n");
>>> - *ret = EIO;
>>> - break;
>>> - default:
>>> - errno = res;
>>> - fprintf(stderr, "failed: %s\n",
>>> - strerror(res));
>>> - *ret = EIO;
>>> - }
>>> - } else if (flags & VMOP_WAIT) {
>>> +
>>> + switch (res) {
>>> + case 0:
>>> + fprintf(stderr, "requested to shutdown vm %d\n",
>>> + vmr->vmr_id);
>>> + *ret = 0;
>>> + break;
>>> + case VMD_VM_STOP_INVALID:
>>> + fprintf(stderr,
>>> + "cannot stop vm that is not running\n");
>>> + *ret = EINVAL;
>>> + break;
>>> + case ENOENT:
>>> + fprintf(stderr, "vm not found\n");
>>> + *ret = EIO;
>>> + break;
>>> + case EINTR:
>>> + fprintf(stderr, "interrupted call\n");
>>> + *ret = EIO;
>>> + break;
>>> + default:
>>> + errno = res;
>>> + fprintf(stderr, "failed: %s\n",
>>> + strerror(res));
>>> + *ret = EIO;
>>> + }
>>> + break;
>>> + case IMSG_VMDOP_TERMINATE_VM_EVENT:
>>> + IMSG_SIZE_CHECK(imsg, &vmr);
>>> + vmr = (struct vmop_result *)imsg->data;
>>> + if (flags & VMOP_WAIT) {
>>> fprintf(stderr, "terminated vm %d\n", vmr->vmr_id);
>>> } else if (flags & VMOP_FORCE) {
>>> fprintf(stderr, "forced to terminate vm %d\n",
>>> vmr->vmr_id);
>>> - } else {
>>> - fprintf(stderr, "requested to shutdown vm %d\n",
>>> - vmr->vmr_id);
>>> - *ret = 0;
>>> }
>>> - } else {
>>> + *ret = 0;
>>> + break;
>>> + default:
>>> fprintf(stderr, "unexpected response received from vmd\n");
>>> *ret = EINVAL;
>>> }
>>> @@ -951,4 +960,3 @@ create_imagefile(int type, const char *i
>>>
>>> return (ret);
>>> }
>>> -
--
-Dave Voutila
