diff mbox series

[V2,08/13] migration: VMSTATE_FD

Message ID 1727725244-105198-9-git-send-email-steven.sistare@oracle.com
State New
Headers show
Series Live update: cpr-transfer | expand

Commit Message

Steven Sistare Sept. 30, 2024, 7:40 p.m. UTC 
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(+)

Comments

Peter Xu Oct. 7, 2024, 4:36 p.m. UTC | #1 
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
>
Steven Sistare Oct. 7, 2024, 7:31 p.m. UTC | #2 
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.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)?

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
>>
>
diff mbox series

Patch

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);
+    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)