Re: [Qemu-devel] [PATCH v23 11/12] support replication driver in blockdev-add

[Changlong Xie <xiecl.fnst@cn.fujitsu.com>]

On 07/27/2016 12:22 AM, Max Reitz wrote:
> On 26.07.2016 10:15, Changlong Xie wrote:
>> From: Wen Congyang <wency@cn.fujitsu.com>
>>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> Signed-off-by: Changlong Xie <xiecl.fnst@cn.fujitsu.com>
>> Signed-off-by: Wang WeiWei <wangww.fnst@cn.fujitsu.com>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> ---
>>   qapi/block-core.json | 22 ++++++++++++++++++++--
>>   1 file changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index 7258a87..48aa112 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -248,6 +248,7 @@
>>   #       2.3: 'host_floppy' deprecated
>>   #       2.5: 'host_floppy' dropped
>>   #       2.6: 'luks' added
>> +#       2.8: 'replication' added
>>   #
>>   # @backing_file: #optional the name of the backing file (for copy-on-write)
>>   #
>> @@ -1696,8 +1697,8 @@
>>     'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop',
>>               'dmg', 'file', 'ftp', 'ftps', 'gluster', 'host_cdrom',
>>               'host_device', 'http', 'https', 'luks', 'null-aio', 'null-co',
>> -            'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'tftp',
>> -            'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] }
>> +            'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'replication',
>> +            'tftp', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] }
>>
>>   ##
>>   # @BlockdevOptionsFile
>> @@ -2160,6 +2161,22 @@
>>   { 'enum' : 'ReplicationMode', 'data' : [ 'primary', 'secondary' ] }
>>
>>   ##
>> +# @BlockdevOptionsReplication
>> +#
>> +# Driver specific block device options for replication
>> +#
>> +# @mode: the replication mode
>> +#
>> +# @top-id: the id to protect replication model chain
>
> It's hard for me to understand this sentence without reading the code
> and thus knowing what this ID is used for. I'd use the following instead:
>
> @top-id: In secondary mode, node name or device ID of the root node who
> owns the replication node chain. Ignored in primary mode.

Pretty description

>
> Also, since this parameter is only necessary in secondary mode and
> completely ignored in primary mode, I would probably make it an optional
> parameter.

Surely.

>
> Max
>
>> +#
>> +# Since: 2.8
>> +##
>> +{ 'struct': 'BlockdevOptionsReplication',
>> +  'base': 'BlockdevOptionsGenericFormat',
>> +  'data': { 'mode': 'ReplicationMode',
>> +            'top-id': 'str' } }
>> +
>> +##
>>   # @BlockdevOptions
>>   #
>>   # Options for creating a block device.  Many options are available for all
>> @@ -2224,6 +2241,7 @@
>>         'quorum':     'BlockdevOptionsQuorum',
>>         'raw':        'BlockdevOptionsGenericFormat',
>>   # TODO rbd: Wait for structured options
>> +      'replication':'BlockdevOptionsReplication',
>>   # TODO sheepdog: Wait for structured options
>>   # TODO ssh: Should take InetSocketAddress for 'host'?
>>         'tftp':       'BlockdevOptionsFile',
>>
>
>