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

[Yang Hongyang <yanghy@xxxxxxxxxxxxxx>]

On 07/16/2015 01:13 AM, Andrew Cooper wrote:
> 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.
This sounds better, thanks!

> > +
> > +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."
Ok.

> > +
> > +     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
Will do, thank you.

> ~Andrew
> .
--
Thanks,
Yang.

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