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

[Steven Sistare]

On 10/7/2024 12:36 PM, Peter Xu wrote:

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.sist...@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)?

This is a remnant from cpr-exec mode, where the fd value did not change across
exec, and SCM_RIGHTS was not used.  I will delete it, and I will delete the mode
test that appears in the "cpr-transfer mode" patch:

    qemu_get_sbe32s(f, v);
    if (*v < 0 || migrate_mode() != MIG_MODE_CPR_TRANSFER) {
        return 0;
    }

- Steve

+    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