Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 12 May 2021 17:43:08 -0800
From:      Rob Wing <rew@freebsd.org>
To:        src-committers@freebsd.org, dev-commits-src-all@freebsd.org,  dev-commits-src-main@freebsd.org
Subject:   Re: git: d4870e3a7256 - main - bhyve/snapshot: provide a way to send other messages/data to bhyve
Message-ID:  <CAF3%2Bn_c3eMbOurbOAYhV9%2BEJJfW8%2BbBDjvyVHevnwq9y_vjURA@mail.gmail.com>
In-Reply-To: <202105130133.14D1XXD6013556@gitrepo.freebsd.org>
References:  <202105130133.14D1XXD6013556@gitrepo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
shoot, I messed up - the differential revision is
https://reviews.freebsd.org/D29213

On Wed, May 12, 2021 at 5:33 PM Robert Wing <rew@freebsd.org> wrote:

> The branch main has been updated by rew:
>
> URL:
> https://cgit.FreeBSD.org/src/commit/?id=d4870e3a7256704905655e997b37a866024c2894
>
> commit d4870e3a7256704905655e997b37a866024c2894
> Author:     Robert Wing <rew@FreeBSD.org>
> AuthorDate: 2021-03-03 06:05:47 +0000
> Commit:     Robert Wing <rew@FreeBSD.org>
> CommitDate: 2021-05-13 01:20:15 +0000
>
>     bhyve/snapshot: provide a way to send other messages/data to bhyve
>
>     This is a step towards sending messages (other than suspend/checkpoint)
>     from bhyvectl to bhyve.
>
>     Introduce a new struct, ipc_message - this struct stores the type of
>     message and a union containing message specific structures for the type
>     of message being sent.
>
>     Reviewed by:    grehan
>     Differential Revision: https://reviews.freebsd.org/D30221
> ---
>  usr.sbin/bhyve/snapshot.c    | 16 ++++++++--------
>  usr.sbin/bhyve/snapshot.h    | 21 ++++++++++++++++++---
>  usr.sbin/bhyvectl/bhyvectl.c | 15 +++++++++------
>  3 files changed, 35 insertions(+), 17 deletions(-)
>
> diff --git a/usr.sbin/bhyve/snapshot.c b/usr.sbin/bhyve/snapshot.c
> index 221558b6f205..019f4fdd6cb0 100644
> --- a/usr.sbin/bhyve/snapshot.c
> +++ b/usr.sbin/bhyve/snapshot.c
> @@ -1441,16 +1441,16 @@ done:
>  }
>
>  int
> -handle_message(struct checkpoint_op *checkpoint_op, struct vmctx *ctx)
> +handle_message(struct ipc_message *imsg, struct vmctx *ctx)
>  {
>         int err;
>
> -       switch (checkpoint_op->op) {
> +       switch (imsg->code) {
>                 case START_CHECKPOINT:
> -                       err = vm_checkpoint(ctx,
> checkpoint_op->snapshot_filename, false);
> +                       err = vm_checkpoint(ctx,
> imsg->data.op.snapshot_filename, false);
>                         break;
>                 case START_SUSPEND:
> -                       err = vm_checkpoint(ctx,
> checkpoint_op->snapshot_filename, true);
> +                       err = vm_checkpoint(ctx,
> imsg->data.op.snapshot_filename, true);
>                         break;
>                 default:
>                         EPRINTLN("Unrecognized checkpoint operation\n");
> @@ -1469,7 +1469,7 @@ handle_message(struct checkpoint_op *checkpoint_op,
> struct vmctx *ctx)
>  void *
>  checkpoint_thread(void *param)
>  {
> -       struct checkpoint_op op;
> +       struct ipc_message imsg;
>         struct checkpoint_thread_info *thread_info;
>         ssize_t n;
>
> @@ -1477,14 +1477,14 @@ checkpoint_thread(void *param)
>         thread_info = (struct checkpoint_thread_info *)param;
>
>         for (;;) {
> -               n = recvfrom(thread_info->socket_fd, &op, sizeof(op), 0,
> NULL, 0);
> +               n = recvfrom(thread_info->socket_fd, &imsg, sizeof(imsg),
> 0, NULL, 0);
>
>                 /*
>                  * slight sanity check: see if there's enough data to at
>                  * least determine the type of message.
>                  */
> -               if (n >= sizeof(op.op))
> -                       handle_message(&op, thread_info->ctx);
> +               if (n >= sizeof(imsg.code))
> +                       handle_message(&imsg, thread_info->ctx);
>                 else
>                         EPRINTLN("Failed to receive message: %s\n",
>                             n == -1 ? strerror(errno) : "unknown error");
> diff --git a/usr.sbin/bhyve/snapshot.h b/usr.sbin/bhyve/snapshot.h
> index 8a6ee67ef19d..f28b56cf0a7f 100644
> --- a/usr.sbin/bhyve/snapshot.h
> +++ b/usr.sbin/bhyve/snapshot.h
> @@ -60,14 +60,29 @@ struct restore_state {
>         ucl_object_t *meta_root_obj;
>  };
>
> +/* Filename that will be used for save/restore */
> +struct checkpoint_op {
> +       char snapshot_filename[MAX_SNAPSHOT_FILENAME];
> +};
> +
> +/* Messages that a bhyve process understands. */
>  enum ipc_opcode {
>         START_CHECKPOINT,
>         START_SUSPEND,
>  };
>
> -struct checkpoint_op {
> -       unsigned int op;
> -       char snapshot_filename[MAX_SNAPSHOT_FILENAME];
> +/*
> + * The type of message and associated data to
> + * send to a bhyve process.
> + */
> +struct ipc_message {
> +        enum ipc_opcode code;
> +        union {
> +                /*
> +                 * message specific structures
> +                 */
> +                struct checkpoint_op op;
> +        } data;
>  };
>
>  struct checkpoint_thread_info {
> diff --git a/usr.sbin/bhyvectl/bhyvectl.c b/usr.sbin/bhyvectl/bhyvectl.c
> index df02f7caf345..017427db4d4f 100644
> --- a/usr.sbin/bhyvectl/bhyvectl.c
> +++ b/usr.sbin/bhyvectl/bhyvectl.c
> @@ -1684,7 +1684,7 @@ show_memseg(struct vmctx *ctx)
>
>  #ifdef BHYVE_SNAPSHOT
>  static int
> -send_checkpoint_op_req(struct vmctx *ctx, struct checkpoint_op *op)
> +send_message(struct vmctx *ctx, void *data, size_t len)
>  {
>         struct sockaddr_un addr;
>         ssize_t len_sent;
> @@ -1709,7 +1709,7 @@ send_checkpoint_op_req(struct vmctx *ctx, struct
> checkpoint_op *op)
>
>         snprintf(addr.sun_path, sizeof(addr.sun_path), "%s%s",
> BHYVE_RUN_DIR, vmname_buf);
>
> -       len_sent = sendto(socket_fd, op, sizeof(*op), 0,
> +       len_sent = sendto(socket_fd, data, len, 0,
>             (struct sockaddr *)&addr, sizeof(struct sockaddr_un));
>
>         if (len_sent < 0) {
> @@ -1726,12 +1726,15 @@ done:
>  static int
>  snapshot_request(struct vmctx *ctx, const char *file, enum ipc_opcode
> code)
>  {
> -       struct checkpoint_op op;
> +       struct ipc_message imsg;
> +       size_t length;
>
> -       op.op = code;
> -       strlcpy(op.snapshot_filename, file, MAX_SNAPSHOT_FILENAME);
> +       imsg.code = code;
> +       strlcpy(imsg.data.op.snapshot_filename, file,
> MAX_SNAPSHOT_FILENAME);
>
> -       return (send_checkpoint_op_req(ctx, &op));
> +       length = offsetof(struct ipc_message, data) + sizeof(imsg.data.op);
> +
> +       return (send_message(ctx, (void *)&imsg, length));
>  }
>  #endif
>
>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAF3%2Bn_c3eMbOurbOAYhV9%2BEJJfW8%2BbBDjvyVHevnwq9y_vjURA>