From mboxrd@z Thu Jan  1 00:00:00 1970
From: Yang Hongyang <yanghy@cn.fujitsu.com>
Subject: Re: [PATCH v8 --for 4.6 COLO 03/25] libxc/migration:
 Specification update for DIRTY_BITMAP records
Date: Thu, 16 Jul 2015 15:18:11 +0800
Message-ID: <55A75AB3.1040400@cn.fujitsu.com>
References: <1436951936-3447-1-git-send-email-yanghy@cn.fujitsu.com>
	<1436951936-3447-4-git-send-email-yanghy@cn.fujitsu.com>
	<55A694CA.7040207@citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <55A694CA.7040207@citrix.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Andrew Cooper <andrew.cooper3@citrix.com>, xen-devel@lists.xen.org
Cc: wei.liu2@citrix.com, ian.campbell@citrix.com, wency@cn.fujitsu.com, guijianfeng@cn.fujitsu.com, yunhong.jiang@intel.com, eddie.dong@intel.com, rshriram@cs.ubc.ca, ian.jackson@eu.citrix.com
List-Id: xen-devel@lists.xenproject.org


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@cn.fujitsu.com>
>> ---
>>   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.