[Qemu-devel] [PATCH] migration: comment VMSTATE_UNUSED*() properly

Peter Xu posted 1 patch 5 years ago
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test checkpatch passed
Test asan passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190329095713.14177-1-peterx@redhat.com
Maintainers: "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Juan Quintela <quintela@redhat.com>
include/migration/vmstate.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
[Qemu-devel] [PATCH] migration: comment VMSTATE_UNUSED*() properly
Posted by Peter Xu 5 years ago
It is error prone to use VMSTATE_UNUSED*() sometimes especially when
the size of the migration stream of the field is not the same as the
size of the structure (boolean is one example).  Comment it well so
people will be aware of this when people want to use it.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 include/migration/vmstate.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index a668ec75b8..9224370ed5 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -1035,6 +1035,20 @@ extern const VMStateInfo vmstate_info_qtailq;
 #define VMSTATE_BUFFER_UNSAFE(_field, _state, _version, _size)        \
     VMSTATE_BUFFER_UNSAFE_INFO(_field, _state, _version, vmstate_info_buffer, _size)
 
+/*
+ * These VMSTATE_UNUSED*() macros can be used to fill in the holes
+ * when some of the vmstate fields are obsolete to be compatible with
+ * migrations between new/old binaries.
+ *
+ * CAUTION: when using any of the VMSTATE_UNUSED*() macros please be
+ * sure that the size passed in is the size that was actually *sent*
+ * rather than the size of the *structure*.  One example is the
+ * boolean type - the size of the structure can vary depending on the
+ * definition of boolean, however the size we actually sent is always
+ * 1 byte (please refer to implementation of VMSTATE_BOOL_V and
+ * vmstate_info_bool).  So here we should always pass in size==1
+ * rather than size==sizeof(bool).
+ */
 #define VMSTATE_UNUSED_V(_v, _size)                                   \
     VMSTATE_UNUSED_BUFFER(NULL, _v, _size)
 
-- 
2.17.1


Re: [Qemu-devel] [PATCH] migration: comment VMSTATE_UNUSED*() properly
Posted by Dr. David Alan Gilbert 5 years ago
* Peter Xu (peterx@redhat.com) wrote:
> It is error prone to use VMSTATE_UNUSED*() sometimes especially when
> the size of the migration stream of the field is not the same as the
> size of the structure (boolean is one example).  Comment it well so
> people will be aware of this when people want to use it.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  include/migration/vmstate.h | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index a668ec75b8..9224370ed5 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -1035,6 +1035,20 @@ extern const VMStateInfo vmstate_info_qtailq;
>  #define VMSTATE_BUFFER_UNSAFE(_field, _state, _version, _size)        \
>      VMSTATE_BUFFER_UNSAFE_INFO(_field, _state, _version, vmstate_info_buffer, _size)
>  
> +/*
> + * These VMSTATE_UNUSED*() macros can be used to fill in the holes
> + * when some of the vmstate fields are obsolete to be compatible with
> + * migrations between new/old binaries.
> + *
> + * CAUTION: when using any of the VMSTATE_UNUSED*() macros please be
> + * sure that the size passed in is the size that was actually *sent*
> + * rather than the size of the *structure*.  One example is the
> + * boolean type - the size of the structure can vary depending on the
> + * definition of boolean, however the size we actually sent is always
> + * 1 byte (please refer to implementation of VMSTATE_BOOL_V and
> + * vmstate_info_bool).  So here we should always pass in size==1
> + * rather than size==sizeof(bool).
> + */
>  #define VMSTATE_UNUSED_V(_v, _size)                                   \
>      VMSTATE_UNUSED_BUFFER(NULL, _v, _size)

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

>  
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH] migration: comment VMSTATE_UNUSED*() properly
Posted by Dr. David Alan Gilbert 4 years, 11 months ago
* Peter Xu (peterx@redhat.com) wrote:
> It is error prone to use VMSTATE_UNUSED*() sometimes especially when
> the size of the migration stream of the field is not the same as the
> size of the structure (boolean is one example).  Comment it well so
> people will be aware of this when people want to use it.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>

Queued

> ---
>  include/migration/vmstate.h | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index a668ec75b8..9224370ed5 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -1035,6 +1035,20 @@ extern const VMStateInfo vmstate_info_qtailq;
>  #define VMSTATE_BUFFER_UNSAFE(_field, _state, _version, _size)        \
>      VMSTATE_BUFFER_UNSAFE_INFO(_field, _state, _version, vmstate_info_buffer, _size)
>  
> +/*
> + * These VMSTATE_UNUSED*() macros can be used to fill in the holes
> + * when some of the vmstate fields are obsolete to be compatible with
> + * migrations between new/old binaries.
> + *
> + * CAUTION: when using any of the VMSTATE_UNUSED*() macros please be
> + * sure that the size passed in is the size that was actually *sent*
> + * rather than the size of the *structure*.  One example is the
> + * boolean type - the size of the structure can vary depending on the
> + * definition of boolean, however the size we actually sent is always
> + * 1 byte (please refer to implementation of VMSTATE_BOOL_V and
> + * vmstate_info_bool).  So here we should always pass in size==1
> + * rather than size==sizeof(bool).
> + */
>  #define VMSTATE_UNUSED_V(_v, _size)                                   \
>      VMSTATE_UNUSED_BUFFER(NULL, _v, _size)
>  
> -- 
> 2.17.1
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK