Download raw body.
make vmctl stop mucking with imsg innards
Dave Voutila <dv@sisu.io> writes:
ping
> Similar to fixing up vmd, this stops vmctl from touching the internals
> of imsg objects.
>
> Took the same approach (and functions) from vmd to unmarshall imsg
> payloads.
>
> ok?
>
> diffstat refs/heads/master refs/heads/vmctl-imsg
> M usr.sbin/vmctl/main.c | 5+ 3-
> M usr.sbin/vmctl/vmctl.c | 71+ 29-
> M usr.sbin/vmctl/vmctl.h | 3+ 6-
>
> 3 files changed, 79 insertions(+), 38 deletions(-)
>
> diff refs/heads/master refs/heads/vmctl-imsg
> commit - 38645cd8d9647e941cedb9bed41c2480381552cc
> commit + 862de7e80b180fec8a7fcf1a6f1f457b84a29b2a
> blob - d44d385df8adf5142071e6cfed566ded1864e7d6
> blob + a807ed83670116195dd7c0bdbbed9168d03ab421
> --- usr.sbin/vmctl/main.c
> +++ usr.sbin/vmctl/main.c
> @@ -191,6 +191,7 @@ vmmaction(struct parse_result *res)
> int n;
> int ret, action;
> unsigned int flags;
> + uint32_t type;
>
> if (ctl_sock == -1) {
> if (unveil(SOCKET_NAME, "w") == -1)
> @@ -290,9 +291,10 @@ vmmaction(struct parse_result *res)
> if (n == 0)
> break;
>
> - if (imsg.hdr.type == IMSG_CTL_FAIL) {
> - if (IMSG_DATA_SIZE(&imsg) == sizeof(ret))
> - memcpy(&ret, imsg.data, sizeof(ret));
> + type = imsg_get_type(&imsg);
> + if (type == IMSG_CTL_FAIL) {
> + if (imsg_get_len(&imsg) == sizeof(ret))
> + ret = imsg_int_read(&imsg);
> else
> ret = 0;
> if (ret != 0) {
> blob - 07ac51fe57278f96ff20d15d9e86286578d899c2
> blob + 27f4a5783b6c1c4c0e9035e354cdf0a2b392d7f8
> --- usr.sbin/vmctl/vmctl.c
> +++ usr.sbin/vmctl/vmctl.c
> @@ -238,12 +238,14 @@ vm_start(uint32_t start_id, const char *name, size_t m
> int
> vm_start_complete(struct imsg *imsg, int *ret, int autoconnect)
> {
> - struct vmop_result *vmr;
> + struct vmop_result vmr;
> + uint32_t type;
> int res;
>
> - if (imsg->hdr.type == IMSG_VMDOP_START_VM_RESPONSE) {
> - vmr = (struct vmop_result *)imsg->data;
> - res = vmr->vmr_result;
> + type = imsg_get_type(imsg);
> + if (type == IMSG_VMDOP_START_VM_RESPONSE) {
> + vmop_result_read(imsg, &vmr);
> + res = vmr.vmr_result;
> if (res) {
> switch (res) {
> case VMD_BIOS_MISSING:
> @@ -274,10 +276,10 @@ vm_start_complete(struct imsg *imsg, int *ret, int aut
> }
> } else if (autoconnect) {
> /* does not return */
> - ctl_openconsole(vmr->vmr_ttyname);
> + ctl_openconsole(vmr.vmr_ttyname);
> } else {
> warnx("started vm %d successfully, tty %s",
> - vmr->vmr_id, vmr->vmr_ttyname);
> + vmr.vmr_id, vmr.vmr_ttyname);
> *ret = 0;
> }
> } else {
> @@ -383,18 +385,20 @@ pause_vm(uint32_t pause_id, const char *name)
> int
> pause_vm_complete(struct imsg *imsg, int *ret)
> {
> - struct vmop_result *vmr;
> + struct vmop_result vmr;
> + uint32_t type;
> int res;
>
> - if (imsg->hdr.type == IMSG_VMDOP_PAUSE_VM_RESPONSE) {
> - vmr = (struct vmop_result *)imsg->data;
> - res = vmr->vmr_result;
> + type = imsg_get_type(imsg);
> + if (type == IMSG_VMDOP_PAUSE_VM_RESPONSE) {
> + vmop_result_read(imsg, &vmr);
> + res = vmr.vmr_result;
> if (res) {
> errno = res;
> warn("pause vm command failed");
> *ret = EIO;
> } else {
> - warnx("paused vm %d successfully", vmr->vmr_id);
> + warnx("paused vm %d successfully", vmr.vmr_id);
> *ret = 0;
> }
> } else {
> @@ -422,18 +426,20 @@ unpause_vm(uint32_t pause_id, const char *name)
> int
> unpause_vm_complete(struct imsg *imsg, int *ret)
> {
> - struct vmop_result *vmr;
> + struct vmop_result vmr;
> + uint32_t type;
> int res;
>
> - if (imsg->hdr.type == IMSG_VMDOP_UNPAUSE_VM_RESPONSE) {
> - vmr = (struct vmop_result *)imsg->data;
> - res = vmr->vmr_result;
> + type = imsg_get_type(imsg);
> + if (type == IMSG_VMDOP_UNPAUSE_VM_RESPONSE) {
> + vmop_result_read(imsg, &vmr);
> + res = vmr.vmr_result;
> if (res) {
> errno = res;
> warn("unpause vm command failed");
> *ret = EIO;
> } else {
> - warnx("unpaused vm %d successfully", vmr->vmr_id);
> + warnx("unpaused vm %d successfully", vmr.vmr_id);
> *ret = 0;
> }
> } else {
> @@ -499,19 +505,20 @@ terminate_vm(uint32_t terminate_id, const char *name,
> int
> terminate_vm_complete(struct imsg *imsg, int *ret, unsigned int flags)
> {
> - struct vmop_result *vmr;
> + struct vmop_result vmr;
> + uint32_t type;
> int res;
>
> - switch (imsg->hdr.type) {
> + type = imsg_get_type(imsg);
> + switch (type) {
> case IMSG_VMDOP_TERMINATE_VM_RESPONSE:
> - IMSG_SIZE_CHECK(imsg, &vmr);
> - vmr = (struct vmop_result *)imsg->data;
> - res = vmr->vmr_result;
> + vmop_result_read(imsg, &vmr);
> + res = vmr.vmr_result;
>
> switch (res) {
> case 0:
> fprintf(stderr, "requested to shutdown vm %d\n",
> - vmr->vmr_id);
> + vmr.vmr_id);
> *ret = 0;
> break;
> case VMD_VM_STOP_INVALID:
> @@ -535,13 +542,12 @@ terminate_vm_complete(struct imsg *imsg, int *ret, uns
> }
> break;
> case IMSG_VMDOP_TERMINATE_VM_EVENT:
> - IMSG_SIZE_CHECK(imsg, &vmr);
> - vmr = (struct vmop_result *)imsg->data;
> + vmop_result_read(imsg, &vmr);
> if (flags & VMOP_WAIT) {
> - fprintf(stderr, "terminated vm %d\n", vmr->vmr_id);
> + 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);
> + vmr.vmr_id);
> }
> *ret = 0;
> break;
> @@ -698,20 +704,22 @@ add_info(struct imsg *imsg, int *ret)
> {
> static size_t ct = 0;
> static struct vmop_info_result *vir = NULL;
> + uint32_t type;
>
> *ret = 0;
>
> - if (imsg->hdr.type == IMSG_VMDOP_GET_INFO_VM_DATA) {
> + type = imsg_get_type(imsg);
> + if (type == IMSG_VMDOP_GET_INFO_VM_DATA) {
> vir = reallocarray(vir, ct + 1,
> sizeof(struct vmop_info_result));
> if (vir == NULL) {
> *ret = ENOMEM;
> return (1);
> }
> - memcpy(&vir[ct], imsg->data, sizeof(struct vmop_info_result));
> + vmop_info_result_read(imsg, &vir[ct]);
> ct++;
> return (0);
> - } else if (imsg->hdr.type == IMSG_VMDOP_GET_INFO_VM_END_DATA) {
> + } else if (type == IMSG_VMDOP_GET_INFO_VM_END_DATA) {
> switch (info_action) {
> case CMD_CONSOLE:
> vm_console(vir, ct);
> @@ -987,3 +995,37 @@ create_imagefile(int type, const char *imgfile_path, c
>
> return (ret);
> }
> +
> +void
> +vmop_result_read(struct imsg *imsg, struct vmop_result *vmr)
> +{
> + if (imsg_get_data(imsg, vmr, sizeof(*vmr)))
> + fatal("%s", __func__);
> +
> + vmr->vmr_ttyname[sizeof(vmr->vmr_ttyname) - 1] = '\0';
> +}
> +
> +void
> +vmop_info_result_read(struct imsg *imsg, struct vmop_info_result *vir)
> +{
> + struct vm_info_result *r;
> +
> + if (imsg_get_data(imsg, vir, sizeof(*vir)))
> + fatal("%s", __func__);
> +
> + r = &vir->vir_info;
> + r->vir_name[sizeof(r->vir_name) - 1] = '\0';
> +
> + vir->vir_ttyname[sizeof(vir->vir_ttyname) - 1] = '\0';
> +}
> +
> +int
> +imsg_int_read(struct imsg *imsg)
> +{
> + int val;
> +
> + if (imsg_get_data(imsg, &val, sizeof(val)))
> + fatal("%s", __func__);
> +
> + return (val);
> +}
> blob - 13fe75bb60f561e1760892ff0f81db9aa0cbbc21
> blob + 0cb06652f14c57852ad127a1f13942d69f624694
> --- usr.sbin/vmctl/vmctl.h
> +++ usr.sbin/vmctl/vmctl.h
> @@ -113,11 +113,8 @@ void terminate_all(struct vmop_info_result *, size_t,
> __dead void
> vm_console(struct vmop_info_result *, size_t);
>
> -/* XXX remove once vmctl is cleaned up of imsg shenanigans. */
> -#define IMSG_SIZE_CHECK(imsg, p) do { \
> - if (IMSG_DATA_SIZE(imsg) < sizeof(*p)) \
> - fatalx("bad length imsg received (%s)", #p); \
> -} while (0)
> -#define IMSG_DATA_SIZE(imsg) ((imsg)->hdr.len - IMSG_HEADER_SIZE)
> +int imsg_int_read(struct imsg *);
> +void vmop_result_read(struct imsg *, struct vmop_result *);
> +void vmop_info_result_read(struct imsg *, struct vmop_info_result *);
>
> #endif /* VMCTL_PARSER_H */
make vmctl stop mucking with imsg innards