From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:57374)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <wency@cn.fujitsu.com>) id 1ZZajJ-0007CM-6x
	for qemu-devel@nongnu.org; Wed, 09 Sep 2015 04:27:47 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <wency@cn.fujitsu.com>) id 1ZZajH-0003Ty-6y
	for qemu-devel@nongnu.org; Wed, 09 Sep 2015 04:27:45 -0400
References: <1441183880-26993-1-git-send-email-wency@cn.fujitsu.com>
	<1441183880-26993-16-git-send-email-wency@cn.fujitsu.com>
	<55E72599.6080107@redhat.com>
From: Wen Congyang <wency@cn.fujitsu.com>
Message-ID: <55EFED60.9090000@cn.fujitsu.com>
Date: Wed, 9 Sep 2015 16:27:12 +0800
MIME-Version: 1.0
In-Reply-To: <55E72599.6080107@redhat.com>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 15/16] support replication driver in
 blockdev-add
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Eric Blake <eblake@redhat.com>, qemu devel <qemu-devel@nongnu.org>, Fam Zheng <famz@redhat.com>, Max Reitz <mreitz@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>, qemu block <qemu-block@nongnu.org>, Jiang Yunhong <yunhong.jiang@intel.com>, Dong Eddie <eddie.dong@intel.com>, "Dr.
	David Alan Gilbert" <dgilbert@redhat.com>, "Michael R. Hines" <mrhines@linux.vnet.ibm.com>, Yang Hongyang <yanghy@cn.fujitsu.com>

On 09/03/2015 12:36 AM, Eric Blake wrote:
> On 09/02/2015 02:51 AM, Wen Congyang wrote:
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>>  qapi/block-core.json | 18 ++++++++++++++++--
>>  1 file changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index 96f0530..86275e3 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -1383,7 +1383,7 @@
>>              'dmg', 'file', 'ftp', 'ftps', 'host_cdrom', 'host_device',
>>              'host_floppy', 'http', 'https', 'null-aio', 'null-co', 'parallels',
>>              'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'tftp', 'vdi', 'vhdx',
>> -            'vmdk', 'vpc', 'vvfat', 'nbd' ] }
>> +            'vmdk', 'vpc', 'vvfat', 'nbd', 'replication' ] }
> 
> 'nbd' is not in the current qemu.git; which means your patch series
> depends on a prerequisite series.  Please mention that sort of
> information in your cover letter.
> 
> Please keep this enum in alphabetical order.
> 
> Missing documentation under @drv of BlockDeviceInfo that this was
> introduced in 2.5.

OK

> 
>>  
>>  ##
>>  # @BlockdevOptionsBase
>> @@ -1825,6 +1825,19 @@
>>  { 'enum' : 'ReplicationMode', 'data' : [ 'primary', 'secondary' ] }
>>  
>>  ##
>> +# @BlockdevOptionsReplication
>> +#
>> +# Driver specific block device options for replication
>> +#
>> +# @mode: the replication mode
> 
> Can the mode be 'unprotected', or must it be 'primary' or 'secondary'
> when first creating a replication BDS?

I will check it, and fix it in the next version.

> 
>> +#
>> +# Since: 2.5
>> +##
>> +{ 'struct': 'BlockdevOptionsReplication',
>> +  'base': 'BlockdevOptionsGenericFormat',
>> +  'data': { 'mode': 'ReplicationMode'  } }
>> +
>> +##
>>  # @BlockdevOptions
>>  #
>>  # Options for creating a block device.
>> @@ -1869,7 +1882,8 @@
>>        'vhdx':       'BlockdevOptionsGenericFormat',
>>        'vmdk':       'BlockdevOptionsGenericCOWFormat',
>>        'vpc':        'BlockdevOptionsGenericFormat',
>> -      'vvfat':      'BlockdevOptionsVVFAT'
>> +      'vvfat':      'BlockdevOptionsVVFAT',
>> +      'replication':'BlockdevOptionsReplication'
>>    } }
> 
> It helps to keep this alphabetical.
> 

OK

Thanks
Wen Congyang