Re: [PATCH V2 08/13] migration: VMSTATE_FD

[Peter Xu <peterx@redhat.com>]

On Mon, Sep 30, 2024 at 12:40:39PM -0700, Steve Sistare wrote:
> Define VMSTATE_FD for declaring a file descriptor field in a
> VMStateDescription.
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>  include/migration/vmstate.h |  9 +++++++++
>  migration/vmstate-types.c   | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+)
> 
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index f313f2f..a1dfab4 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -230,6 +230,7 @@ extern const VMStateInfo vmstate_info_uint8;
>  extern const VMStateInfo vmstate_info_uint16;
>  extern const VMStateInfo vmstate_info_uint32;
>  extern const VMStateInfo vmstate_info_uint64;
> +extern const VMStateInfo vmstate_info_fd;
>  
>  /** Put this in the stream when migrating a null pointer.*/
>  #define VMS_NULLPTR_MARKER (0x30U) /* '0' */
> @@ -902,6 +903,9 @@ extern const VMStateInfo vmstate_info_qlist;
>  #define VMSTATE_UINT64_V(_f, _s, _v)                                  \
>      VMSTATE_SINGLE(_f, _s, _v, vmstate_info_uint64, uint64_t)
>  
> +#define VMSTATE_FD_V(_f, _s, _v)                                  \
> +    VMSTATE_SINGLE(_f, _s, _v, vmstate_info_fd, int32_t)
> +
>  #ifdef CONFIG_LINUX
>  
>  #define VMSTATE_U8_V(_f, _s, _v)                                   \
> @@ -936,6 +940,9 @@ extern const VMStateInfo vmstate_info_qlist;
>  #define VMSTATE_UINT64(_f, _s)                                        \
>      VMSTATE_UINT64_V(_f, _s, 0)
>  
> +#define VMSTATE_FD(_f, _s)                                            \
> +    VMSTATE_FD_V(_f, _s, 0)
> +
>  #ifdef CONFIG_LINUX
>  
>  #define VMSTATE_U8(_f, _s)                                         \
> @@ -1009,6 +1016,8 @@ extern const VMStateInfo vmstate_info_qlist;
>  #define VMSTATE_UINT64_TEST(_f, _s, _t)                                  \
>      VMSTATE_SINGLE_TEST(_f, _s, _t, 0, vmstate_info_uint64, uint64_t)
>  
> +#define VMSTATE_FD_TEST(_f, _s, _t)                                            \
> +    VMSTATE_SINGLE_TEST(_f, _s, _t, 0, vmstate_info_fd, int32_t)
>  
>  #define VMSTATE_TIMER_PTR_TEST(_f, _s, _test)                             \
>      VMSTATE_POINTER_TEST(_f, _s, _test, vmstate_info_timer, QEMUTimer *)
> diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c
> index e83bfcc..6e45a4a 100644
> --- a/migration/vmstate-types.c
> +++ b/migration/vmstate-types.c
> @@ -314,6 +314,38 @@ const VMStateInfo vmstate_info_uint64 = {
>      .put  = put_uint64,
>  };
>  
> +/* File descriptor communicated via SCM_RIGHTS */
> +
> +static int get_fd(QEMUFile *f, void *pv, size_t size,
> +                  const VMStateField *field)
> +{
> +    int32_t *v = pv;
> +    qemu_get_sbe32s(f, v);

Why we need to send/recv the fd integer alone?  Can't that change anyway
across migration?  What happens if we drop this (and the put side)?

> +    if (*v < 0) {
> +        return 0;
> +    }
> +    *v = qemu_file_get_fd(f);
> +    return 0;
> +}
> +
> +static int put_fd(QEMUFile *f, void *pv, size_t size,
> +                  const VMStateField *field, JSONWriter *vmdesc)
> +{
> +    int32_t *v = pv;
> +
> +    qemu_put_sbe32s(f, v);
> +    if (*v < 0) {
> +        return 0;
> +    }
> +    return qemu_file_put_fd(f, *v);
> +}
> +
> +const VMStateInfo vmstate_info_fd = {
> +    .name = "fd",
> +    .get  = get_fd,
> +    .put  = put_fd,
> +};
> +
>  static int get_nullptr(QEMUFile *f, void *pv, size_t size,
>                         const VMStateField *field)
>  
> -- 
> 1.8.3.1
> 

-- 
Peter Xu