diff mbox

[qemu] migration/vmstate: document VMStateFlags

Message ID 1453408767-4905-1-git-send-email-silbe@linux.vnet.ibm.com
State New
Headers show

Commit Message

Sascha Silbe Jan. 21, 2016, 8:39 p.m. UTC
The VMState API is rather sparsely documented. Start by describing the
meaning of all VMStateFlags.

Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
---
Since I had to dive into the code for debugging the migration breakage
anyway, I figured I could just as well write down the result in the
form of comments so the next one trying to make some sense out of it
will have a better start.

This is based on my understanding of the code, not necessarily what
the original author(s) intended.

 include/migration/vmstate.h | 93 ++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 84 insertions(+), 9 deletions(-)

Comments

Amit Shah Feb. 3, 2016, 12:38 p.m. UTC | #1
On (Thu) 21 Jan 2016 [21:39:27], Sascha Silbe wrote:
> The VMState API is rather sparsely documented. Start by describing the
> meaning of all VMStateFlags.
> 
> Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>

Thanks, this is much needed.  I'm just returning from a long trip,
will go through this in a bit.

		Amit
Amit Shah Feb. 23, 2016, 10:39 a.m. UTC | #2
On (Thu) 21 Jan 2016 [21:39:27], Sascha Silbe wrote:
> The VMState API is rather sparsely documented. Start by describing the
> meaning of all VMStateFlags.
> 
> Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>

Reviewed-by: Amit Shah <amit.shah@redhat.com>

> ---
> Since I had to dive into the code for debugging the migration breakage
> anyway, I figured I could just as well write down the result in the
> form of comments so the next one trying to make some sense out of it
> will have a better start.

Yes, nice.  Thanks!

		Amit
Juan Quintela Feb. 23, 2016, 12:32 p.m. UTC | #3
Sascha Silbe <silbe@linux.vnet.ibm.com> wrote:
> The VMState API is rather sparsely documented. Start by describing the
> meaning of all VMStateFlags.
>
> Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
> ---
> Since I had to dive into the code for debugging the migration breakage
> anyway, I figured I could just as well write down the result in the
> form of comments so the next one trying to make some sense out of it
> will have a better start.
>
> This is based on my understanding of the code, not necessarily what
> the original author(s) intended.
>
>  include/migration/vmstate.h | 93 ++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 84 insertions(+), 9 deletions(-)
>

Reviewed-by: Juan Quintela <quintela@redhat.com>

Thanks very much


> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index 97d44d3..b1bbf68 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -88,20 +88,95 @@ struct VMStateInfo {
>  };
>  
>  enum VMStateFlags {
> +    /* Ignored */
>      VMS_SINGLE           = 0x001,
> +
> +    /* The struct member at opaque + VMStateField.offset is a pointer
> +     * to the actual field (e.g. struct a { uint8_t *b;
> +     * }). Dereference the pointer before using it as basis for
> +     * further pointer arithmetic (see e.g. VMS_ARRAY). Does not
> +     * affect the meaning of VMStateField.num_offset or
> +     * VMStateField.size_offset; see VMS_VARRAY* and VMS_VBUFFER for
> +     * those. */
>      VMS_POINTER          = 0x002,
> +
> +    /* The field is an array of fixed size. VMStateField.num contains
> +     * the number of entries in the array. The size of each entry is
> +     * given by VMStateField.size and / or opaque +
> +     * VMStateField.size_offset; see VMS_VBUFFER and
> +     * VMS_MULTIPLY. Each array entry will be processed individually
> +     * (VMStateField.info.get()/put() if VMS_STRUCT is not set,
> +     * recursion into VMStateField.vmsd if VMS_STRUCT is set). May not
> +     * be combined with VMS_VARRAY*. */
>      VMS_ARRAY            = 0x004,
> +
> +    /* The field is itself a struct, containing one or more
> +     * fields. Recurse into VMStateField.vmsd. Most useful in
> +     * combination with VMS_ARRAY / VMS_VARRAY*, recursing into each
> +     * array entry. */
>      VMS_STRUCT           = 0x008,
> -    VMS_VARRAY_INT32     = 0x010,  /* Array with size in int32_t field*/
> -    VMS_BUFFER           = 0x020,  /* static sized buffer */
> +
> +    /* The field is an array of variable size. The int32_t at opaque +
> +     * VMStateField.num_offset contains the number of entries in the
> +     * array. See the VMS_ARRAY description regarding array handling
> +     * in general. May not be combined with VMS_ARRAY or any other
> +     * VMS_VARRAY*. */
> +    VMS_VARRAY_INT32     = 0x010,
> +
> +    /* Ignored */
> +    VMS_BUFFER           = 0x020,
> +
> +    /* The field is a (fixed-size or variable-size) array of pointers
> +     * (e.g. struct a { uint8_t *b[]; }). Dereference each array entry
> +     * before using it. Note: Does not imply any one of VMS_ARRAY /
> +     * VMS_VARRAY*; these need to be set explicitly. */
>      VMS_ARRAY_OF_POINTER = 0x040,
> -    VMS_VARRAY_UINT16    = 0x080,  /* Array with size in uint16_t field */
> -    VMS_VBUFFER          = 0x100,  /* Buffer with size in int32_t field */
> -    VMS_MULTIPLY         = 0x200,  /* multiply "size" field by field_size */
> -    VMS_VARRAY_UINT8     = 0x400,  /* Array with size in uint8_t field*/
> -    VMS_VARRAY_UINT32    = 0x800,  /* Array with size in uint32_t field*/
> -    VMS_MUST_EXIST       = 0x1000, /* Field must exist in input */
> -    VMS_ALLOC            = 0x2000, /* Alloc a buffer on the destination */
> +
> +    /* The field is an array of variable size. The uint16_t at opaque
> +     * + VMStateField.num_offset contains the number of entries in the
> +     * array. See the VMS_ARRAY description regarding array handling
> +     * in general. May not be combined with VMS_ARRAY or any other
> +     * VMS_VARRAY*. */
> +    VMS_VARRAY_UINT16    = 0x080,
> +
> +    /* The size of the individual entries (a single array entry if
> +     * VMS_ARRAY or VMS_VARRAY are set, or the field itself if neither
> +     * is set) is variable (i.e. not known at compile-time), but the
> +     * same for all entries. Use the int32_t at opaque +
> +     * VMStateField.size_offset (subject to VMS_MULTIPLY) to determine
> +     * the size of each (and every) entry. */
> +    VMS_VBUFFER          = 0x100,
> +
> +    /* Multiply the entry size given by the int32_t at opaque +
> +     * VMStateField.size_offset (see VMS_VBUFFER description) with
> +     * VMStateField.size to determine the number of bytes to be
> +     * allocated. Only valid in combination with VMS_VBUFFER. */
> +    VMS_MULTIPLY         = 0x200,
> +
> +    /* The field is an array of variable size. The uint8_t at opaque +
> +     * VMStateField.num_offset contains the number of entries in the
> +     * array. See the VMS_ARRAY description regarding array handling
> +     * in general. May not be combined with VMS_ARRAY or any other
> +     * VMS_VARRAY*. */
> +    VMS_VARRAY_UINT8     = 0x400,
> +
> +    /* The field is an array of variable size. The uint32_t at opaque
> +     * + VMStateField.num_offset contains the number of entries in the
> +     * array. See the VMS_ARRAY description regarding array handling
> +     * in general. May not be combined with VMS_ARRAY or any other
> +     * VMS_VARRAY*. */
> +    VMS_VARRAY_UINT32    = 0x800,
> +
> +    /* Fail loading the serialised VM state if this field is missing
> +     * from the input. */
> +    VMS_MUST_EXIST       = 0x1000,
> +
> +    /* When loading serialised VM state, allocate memory for the
> +     * (entire) field. Only valid in combination with
> +     * VMS_POINTER. Note: Not all combinations with other flags are
> +     * currently supported, e.g. VMS_ALLOC|VMS_ARRAY_OF_POINTER won't
> +     * cause the individual entries to be allocated. */
> +    VMS_ALLOC            = 0x2000,
>  };
>  
>  typedef struct {
Amit Shah Feb. 25, 2016, 5:05 a.m. UTC | #4
On (Thu) 21 Jan 2016 [21:39:27], Sascha Silbe wrote:
> The VMState API is rather sparsely documented. Start by describing the
> meaning of all VMStateFlags.
> 
> Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
> ---
> Since I had to dive into the code for debugging the migration breakage
> anyway, I figured I could just as well write down the result in the
> form of comments so the next one trying to make some sense out of it
> will have a better start.
> 
> This is based on my understanding of the code, not necessarily what
> the original author(s) intended.

Thanks, I've pulled this in my queue.

There's one new flag added: VMS_MULTIPLY_ELEMENTS.  Do you want to
send a follow-up patch to add a description to that one as well?

		Amit
Sascha Silbe Feb. 25, 2016, 8:25 p.m. UTC | #5
Dear Amit,

Amit Shah <amit.shah@redhat.com> writes:

> On (Thu) 21 Jan 2016 [21:39:27], Sascha Silbe wrote:
>> The VMState API is rather sparsely documented. Start by describing the
>> meaning of all VMStateFlags.

> Thanks, I've pulled this in my queue.
>
> There's one new flag added: VMS_MULTIPLY_ELEMENTS.  Do you want to
> send a follow-up patch to add a description to that one as well?

I can either send a new version with the conflicts resolved and
VMS_MULTIPLY_ELEMENTS documented (i.e. rebased on current master), or a
follow-up patch with just the differences to your version if you point
me at your git repo. Whatever works best for you.

Sascha
diff mbox

Patch

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 97d44d3..b1bbf68 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -88,20 +88,95 @@  struct VMStateInfo {
 };
 
 enum VMStateFlags {
+    /* Ignored */
     VMS_SINGLE           = 0x001,
+
+    /* The struct member at opaque + VMStateField.offset is a pointer
+     * to the actual field (e.g. struct a { uint8_t *b;
+     * }). Dereference the pointer before using it as basis for
+     * further pointer arithmetic (see e.g. VMS_ARRAY). Does not
+     * affect the meaning of VMStateField.num_offset or
+     * VMStateField.size_offset; see VMS_VARRAY* and VMS_VBUFFER for
+     * those. */
     VMS_POINTER          = 0x002,
+
+    /* The field is an array of fixed size. VMStateField.num contains
+     * the number of entries in the array. The size of each entry is
+     * given by VMStateField.size and / or opaque +
+     * VMStateField.size_offset; see VMS_VBUFFER and
+     * VMS_MULTIPLY. Each array entry will be processed individually
+     * (VMStateField.info.get()/put() if VMS_STRUCT is not set,
+     * recursion into VMStateField.vmsd if VMS_STRUCT is set). May not
+     * be combined with VMS_VARRAY*. */
     VMS_ARRAY            = 0x004,
+
+    /* The field is itself a struct, containing one or more
+     * fields. Recurse into VMStateField.vmsd. Most useful in
+     * combination with VMS_ARRAY / VMS_VARRAY*, recursing into each
+     * array entry. */
     VMS_STRUCT           = 0x008,
-    VMS_VARRAY_INT32     = 0x010,  /* Array with size in int32_t field*/
-    VMS_BUFFER           = 0x020,  /* static sized buffer */
+
+    /* The field is an array of variable size. The int32_t at opaque +
+     * VMStateField.num_offset contains the number of entries in the
+     * array. See the VMS_ARRAY description regarding array handling
+     * in general. May not be combined with VMS_ARRAY or any other
+     * VMS_VARRAY*. */
+    VMS_VARRAY_INT32     = 0x010,
+
+    /* Ignored */
+    VMS_BUFFER           = 0x020,
+
+    /* The field is a (fixed-size or variable-size) array of pointers
+     * (e.g. struct a { uint8_t *b[]; }). Dereference each array entry
+     * before using it. Note: Does not imply any one of VMS_ARRAY /
+     * VMS_VARRAY*; these need to be set explicitly. */
     VMS_ARRAY_OF_POINTER = 0x040,
-    VMS_VARRAY_UINT16    = 0x080,  /* Array with size in uint16_t field */
-    VMS_VBUFFER          = 0x100,  /* Buffer with size in int32_t field */
-    VMS_MULTIPLY         = 0x200,  /* multiply "size" field by field_size */
-    VMS_VARRAY_UINT8     = 0x400,  /* Array with size in uint8_t field*/
-    VMS_VARRAY_UINT32    = 0x800,  /* Array with size in uint32_t field*/
-    VMS_MUST_EXIST       = 0x1000, /* Field must exist in input */
-    VMS_ALLOC            = 0x2000, /* Alloc a buffer on the destination */
+
+    /* The field is an array of variable size. The uint16_t at opaque
+     * + VMStateField.num_offset contains the number of entries in the
+     * array. See the VMS_ARRAY description regarding array handling
+     * in general. May not be combined with VMS_ARRAY or any other
+     * VMS_VARRAY*. */
+    VMS_VARRAY_UINT16    = 0x080,
+
+    /* The size of the individual entries (a single array entry if
+     * VMS_ARRAY or VMS_VARRAY are set, or the field itself if neither
+     * is set) is variable (i.e. not known at compile-time), but the
+     * same for all entries. Use the int32_t at opaque +
+     * VMStateField.size_offset (subject to VMS_MULTIPLY) to determine
+     * the size of each (and every) entry. */
+    VMS_VBUFFER          = 0x100,
+
+    /* Multiply the entry size given by the int32_t at opaque +
+     * VMStateField.size_offset (see VMS_VBUFFER description) with
+     * VMStateField.size to determine the number of bytes to be
+     * allocated. Only valid in combination with VMS_VBUFFER. */
+    VMS_MULTIPLY         = 0x200,
+
+    /* The field is an array of variable size. The uint8_t at opaque +
+     * VMStateField.num_offset contains the number of entries in the
+     * array. See the VMS_ARRAY description regarding array handling
+     * in general. May not be combined with VMS_ARRAY or any other
+     * VMS_VARRAY*. */
+    VMS_VARRAY_UINT8     = 0x400,
+
+    /* The field is an array of variable size. The uint32_t at opaque
+     * + VMStateField.num_offset contains the number of entries in the
+     * array. See the VMS_ARRAY description regarding array handling
+     * in general. May not be combined with VMS_ARRAY or any other
+     * VMS_VARRAY*. */
+    VMS_VARRAY_UINT32    = 0x800,
+
+    /* Fail loading the serialised VM state if this field is missing
+     * from the input. */
+    VMS_MUST_EXIST       = 0x1000,
+
+    /* When loading serialised VM state, allocate memory for the
+     * (entire) field. Only valid in combination with
+     * VMS_POINTER. Note: Not all combinations with other flags are
+     * currently supported, e.g. VMS_ALLOC|VMS_ARRAY_OF_POINTER won't
+     * cause the individual entries to be allocated. */
+    VMS_ALLOC            = 0x2000,
 };
 
 typedef struct {