Re: [Xen-devel] [PATCH v8 --for 4.6 COLO 03/25] libxc/migration: Specification update for DIRTY_BITMAP records

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 15/07/15 10:18, Yang Hongyang wrote:
> Used by secondary to send it's dirty bitmap to primary under COLO.
>
> Signed-off-by: Yang Hongyang <yanghy@xxxxxxxxxxxxxx>
> ---
>  docs/specs/libxc-migration-stream.pandoc | 24 +++++++++++++++++++++++-
>  tools/libxc/xc_sr_common.c               |  1 +
>  tools/libxc/xc_sr_stream_format.h        |  1 +
>  3 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/docs/specs/libxc-migration-stream.pandoc 
> b/docs/specs/libxc-migration-stream.pandoc
> index 68fa513..480d357 100644
> --- a/docs/specs/libxc-migration-stream.pandoc
> +++ b/docs/specs/libxc-migration-stream.pandoc
> @@ -227,7 +227,9 @@ type         0x00000000: END
>  
>               0x0000000E: CHECKPOINT
>  
> -             0x0000000F - 0x7FFFFFFF: Reserved for future _mandatory_
> +             0x0000000F: DIRTY_BITMAP
> +
> +             0x00000010 - 0x7FFFFFFF: Reserved for future _mandatory_
>               records.
>  
>               0x80000000 - 0xFFFFFFFF: Reserved for future _optional_
> @@ -601,6 +603,26 @@ CHECKPOINT record or an END record.
>  
>  \clearpage
>  
> +DIRTY_BITMAP
> +------------

I would name this DIRTY_PFN_LIST or similar, as the content of data
isn't actually a bitmap.

> +
> +A dirty_bitmap record is used for secondary to send it's dirty bitmap
> +to primary while doing a checkpoint under COLO. This record only exists
> +in back channel.

This section should purely be a description of the content.  i.e.

"A DIRTY\_xxx record is used to convey information about dirty memory in
the VM.  It is an unordered list of PFNs."

> +
> +     0     1     2     3     4     5     6     7 octet
> +    +-------------------------------------------------+
> +    | pfn[0]                                          |
> +    +-------------------------------------------------+
> +    ...
> +    +-------------------------------------------------+
> +    | pfn[C-1]                                        |
> +    +-------------------------------------------------+
> +
> +The count of the pfn is: record->length/sizeof(uint64_t).

"The count of pfns is", although I would like to hope that this is
obvious from the diagram.

Down here, there should be more description of record circumstances,
e.g. currently only applicable in the backchannel of a checkpointed stream.

Also please put some validation logic for this in
tools/python/xen/migration/libxc.py

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel