[v24,11/12] support replication driver in blockdev-add

Commit Message

Changlong Xie July 27, 2016, 7:01 a.m. UTC

From: Wen Congyang <[e-mail]>

Signed-off-by: Wen Congyang <[e-mail]>
Signed-off-by: Changlong Xie <[e-mail]>
Signed-off-by: Wang WeiWei <[e-mail]>
Signed-off-by: zhanghailiang <[e-mail]>
Signed-off-by: Gonglei <[e-mail]>
Reviewed-by: Eric Blake <[e-mail]>
---
 qapi/block-core.json | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

Comments

Kevin Wolf Aug. 9, 2016, 9:08 a.m. UTC | #1

Am 27.07.2016 um 09:01 hat Changlong Xie geschrieben:
> From: Wen Congyang <[e-mail]>
> 
> Signed-off-by: Wen Congyang <[e-mail]>
> Signed-off-by: Changlong Xie <[e-mail]>
> Signed-off-by: Wang WeiWei <[e-mail]>
> Signed-off-by: zhanghailiang <[e-mail]>
> Signed-off-by: Gonglei <[e-mail]>
> Reviewed-by: Eric Blake <[e-mail]>

> @@ -2078,6 +2079,23 @@
>  { 'enum' : 'ReplicationMode', 'data' : [ 'primary', 'secondary' ] }
>  
>  ##
> +# @BlockdevOptionsReplication
> +#
> +# Driver specific block device options for replication
> +#
> +# @mode: the replication mode
> +#
> +# @top-id: #optional In secondary mode, node name or device ID of the root
> +#          node who owns the replication node chain. Ignored in primary mode.

Can we change this to "Must not be given in primary mode"? Not sure what
the code currently does, but I think it should error out if top-id is
given there.

> +#
> +# Since: 2.8
> +##
> +{ 'struct': 'BlockdevOptionsReplication',
> +  'base': 'BlockdevOptionsGenericFormat',
> +  'data': { 'mode': 'ReplicationMode',
> +            '*top-id': 'str' } }

Kevin

Changlong Xie Aug. 15, 2016, 1:49 a.m. UTC | #2

On 08/09/2016 05:08 PM, Kevin Wolf wrote:
> Am 27.07.2016 um 09:01 hat Changlong Xie geschrieben:
>> From: Wen Congyang <[e-mail]>
>>
>> Signed-off-by: Wen Congyang <[e-mail]>
>> Signed-off-by: Changlong Xie <[e-mail]>
>> Signed-off-by: Wang WeiWei <[e-mail]>
>> Signed-off-by: zhanghailiang <[e-mail]>
>> Signed-off-by: Gonglei <[e-mail]>
>> Reviewed-by: Eric Blake <[e-mail]>
>
>> @@ -2078,6 +2079,23 @@
>>   { 'enum' : 'ReplicationMode', 'data' : [ 'primary', 'secondary' ] }
>>
>>   ##
>> +# @BlockdevOptionsReplication
>> +#
>> +# Driver specific block device options for replication
>> +#
>> +# @mode: the replication mode
>> +#
>> +# @top-id: #optional In secondary mode, node name or device ID of the root
>> +#          node who owns the replication node chain. Ignored in primary mode.
>
> Can we change this to "Must not be given in primary mode"? Not sure what
> the code currently does, but I think it should error out if top-id is

Replication driver will ignore "top-id" parameter in Primary mode.

> given there.
>
>> +#
>> +# Since: 2.8
>> +##
>> +{ 'struct': 'BlockdevOptionsReplication',
>> +  'base': 'BlockdevOptionsGenericFormat',
>> +  'data': { 'mode': 'ReplicationMode',
>> +            '*top-id': 'str' } }
>
> Kevin
>
>
> .
>

Kevin Wolf Aug. 15, 2016, 8:37 a.m. UTC | #3

Am 15.08.2016 um 03:49 hat Changlong Xie geschrieben:
> On 08/09/2016 05:08 PM, Kevin Wolf wrote:
> >Am 27.07.2016 um 09:01 hat Changlong Xie geschrieben:
> >>From: Wen Congyang <[e-mail]>
> >>
> >>Signed-off-by: Wen Congyang <[e-mail]>
> >>Signed-off-by: Changlong Xie <[e-mail]>
> >>Signed-off-by: Wang WeiWei <[e-mail]>
> >>Signed-off-by: zhanghailiang <[e-mail]>
> >>Signed-off-by: Gonglei <[e-mail]>
> >>Reviewed-by: Eric Blake <[e-mail]>
> >
> >>@@ -2078,6 +2079,23 @@
> >>  { 'enum' : 'ReplicationMode', 'data' : [ 'primary', 'secondary' ] }
> >>
> >>  ##
> >>+# @BlockdevOptionsReplication
> >>+#
> >>+# Driver specific block device options for replication
> >>+#
> >>+# @mode: the replication mode
> >>+#
> >>+# @top-id: #optional In secondary mode, node name or device ID of the root
> >>+#          node who owns the replication node chain. Ignored in primary mode.
> >
> >Can we change this to "Must not be given in primary mode"? Not sure what
> >the code currently does, but I think it should error out if top-id is
> 
> Replication driver will ignore "top-id" parameter in Primary mode.

This is not good behaviour, which is why I requested a change.

Kevin

> >given there.
> >
> >>+#
> >>+# Since: 2.8
> >>+##
> >>+{ 'struct': 'BlockdevOptionsReplication',
> >>+  'base': 'BlockdevOptionsGenericFormat',
> >>+  'data': { 'mode': 'ReplicationMode',
> >>+            '*top-id': 'str' } }
> >
> >Kevin

Changlong Xie Aug. 15, 2016, 9:32 a.m. UTC | #4

On 08/15/2016 04:37 PM, Kevin Wolf wrote:
> Am 15.08.2016 um 03:49 hat Changlong Xie geschrieben:
>> On 08/09/2016 05:08 PM, Kevin Wolf wrote:
>>> Am 27.07.2016 um 09:01 hat Changlong Xie geschrieben:
>>>> From: Wen Congyang <[e-mail]>
>>>>
>>>> Signed-off-by: Wen Congyang <[e-mail]>
>>>> Signed-off-by: Changlong Xie <[e-mail]>
>>>> Signed-off-by: Wang WeiWei <[e-mail]>
>>>> Signed-off-by: zhanghailiang <[e-mail]>
>>>> Signed-off-by: Gonglei <[e-mail]>
>>>> Reviewed-by: Eric Blake <[e-mail]>
>>>
>>>> @@ -2078,6 +2079,23 @@
>>>>   { 'enum' : 'ReplicationMode', 'data' : [ 'primary', 'secondary' ] }
>>>>
>>>>   ##
>>>> +# @BlockdevOptionsReplication
>>>> +#
>>>> +# Driver specific block device options for replication
>>>> +#
>>>> +# @mode: the replication mode
>>>> +#
>>>> +# @top-id: #optional In secondary mode, node name or device ID of the root
>>>> +#          node who owns the replication node chain. Ignored in primary mode.
>>>
>>> Can we change this to "Must not be given in primary mode"? Not sure what
>>> the code currently does, but I think it should error out if top-id is
>>
>> Replication driver will ignore "top-id" parameter in Primary mode.
>
> This is not good behaviour, which is why I requested a change.
>

Hi stefan

Would you like me send another [PATCH v25] based your block-next? Or a 
separate patch until your tree is merged.

Thanks
	-Xie

> Kevin
>
>>> given there.
>>>
>>>> +#
>>>> +# Since: 2.8
>>>> +##
>>>> +{ 'struct': 'BlockdevOptionsReplication',
>>>> +  'base': 'BlockdevOptionsGenericFormat',
>>>> +  'data': { 'mode': 'ReplicationMode',
>>>> +            '*top-id': 'str' } }
>>>
>>> Kevin
>
>
> .
>

Stefan Hajnoczi Sept. 12, 2016, 2:01 p.m. UTC | #5

On Mon, Aug 15, 2016 at 05:32:19PM +0800, Changlong Xie wrote:
> On 08/15/2016 04:37 PM, Kevin Wolf wrote:
> > Am 15.08.2016 um 03:49 hat Changlong Xie geschrieben:
> > > On 08/09/2016 05:08 PM, Kevin Wolf wrote:
> > > > Am 27.07.2016 um 09:01 hat Changlong Xie geschrieben:
> > > > > From: Wen Congyang <[e-mail]>
> > > > > 
> > > > > Signed-off-by: Wen Congyang <[e-mail]>
> > > > > Signed-off-by: Changlong Xie <[e-mail]>
> > > > > Signed-off-by: Wang WeiWei <[e-mail]>
> > > > > Signed-off-by: zhanghailiang <[e-mail]>
> > > > > Signed-off-by: Gonglei <[e-mail]>
> > > > > Reviewed-by: Eric Blake <[e-mail]>
> > > > 
> > > > > @@ -2078,6 +2079,23 @@
> > > > >   { 'enum' : 'ReplicationMode', 'data' : [ 'primary', 'secondary' ] }
> > > > > 
> > > > >   ##
> > > > > +# @BlockdevOptionsReplication
> > > > > +#
> > > > > +# Driver specific block device options for replication
> > > > > +#
> > > > > +# @mode: the replication mode
> > > > > +#
> > > > > +# @top-id: #optional In secondary mode, node name or device ID of the root
> > > > > +#          node who owns the replication node chain. Ignored in primary mode.
> > > > 
> > > > Can we change this to "Must not be given in primary mode"? Not sure what
> > > > the code currently does, but I think it should error out if top-id is
> > > 
> > > Replication driver will ignore "top-id" parameter in Primary mode.
> > 
> > This is not good behaviour, which is why I requested a change.
> > 
> 
> Hi stefan
> 
> Would you like me send another [PATCH v25] based your block-next? Or a
> separate patch until your tree is merged.

Sorry for the slow response.  Please send a new patch on top of my
block-next tree.

Stefan

Wang WeiWei Sept. 28, 2016, 1:53 a.m. UTC | #6

在 2016年09月12日 22:01, Stefan Hajnoczi 写道:
> On Mon, Aug 15, 2016 at 05:32:19PM +0800, Changlong Xie wrote:
>> On 08/15/2016 04:37 PM, Kevin Wolf wrote:
>>> Am 15.08.2016 um 03:49 hat Changlong Xie geschrieben:
>>>> On 08/09/2016 05:08 PM, Kevin Wolf wrote:
>>>>> Am 27.07.2016 um 09:01 hat Changlong Xie geschrieben:
>>>>>> From: Wen Congyang <[e-mail]>
>>>>>>
>>>>>> Signed-off-by: Wen Congyang <[e-mail]>
>>>>>> Signed-off-by: Changlong Xie <[e-mail]>
>>>>>> Signed-off-by: Wang WeiWei <[e-mail]>
>>>>>> Signed-off-by: zhanghailiang <[e-mail]>
>>>>>> Signed-off-by: Gonglei <[e-mail]>
>>>>>> Reviewed-by: Eric Blake <[e-mail]>
>>>>>
>>>>>> @@ -2078,6 +2079,23 @@
>>>>>>    { 'enum' : 'ReplicationMode', 'data' : [ 'primary', 'secondary' ] }
>>>>>>
>>>>>>    ##
>>>>>> +# @BlockdevOptionsReplication
>>>>>> +#
>>>>>> +# Driver specific block device options for replication
>>>>>> +#
>>>>>> +# @mode: the replication mode
>>>>>> +#
>>>>>> +# @top-id: #optional In secondary mode, node name or device ID of the root
>>>>>> +#          node who owns the replication node chain. Ignored in primary mode.
>>>>>
>>>>> Can we change this to "Must not be given in primary mode"? Not sure what
>>>>> the code currently does, but I think it should error out if top-id is
>>>>
>>>> Replication driver will ignore "top-id" parameter in Primary mode.
>>>
>>> This is not good behaviour, which is why I requested a change.
>>>
>>
>> Hi stefan
>>
>> Would you like me send another [PATCH v25] based your block-next? Or a
>> separate patch until your tree is merged.
>
> Sorry for the slow response.  Please send a new patch on top of my
> block-next tree.
>
> Stefan
>
ok, I will send a new patch on the top of block-next tree.

Patch

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 8c250ec..aadc75a 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)
 #
@@ -1671,8 +1672,8 @@ 
   'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop',
             'dmg', 'file', 'ftp', 'ftps', 'host_cdrom', 'host_device',
             'http', 'https', 'luks', 'null-aio', 'null-co', 'parallels',
-            'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'tftp', 'vdi', 'vhdx',
-            'vmdk', 'vpc', 'vvfat' ] }
+            'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'replication', 'tftp',
+            'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] }
 
 ##
 # @BlockdevOptionsFile
@@ -2078,6 +2079,23 @@ 
 { 'enum' : 'ReplicationMode', 'data' : [ 'primary', 'secondary' ] }
 
 ##
+# @BlockdevOptionsReplication
+#
+# Driver specific block device options for replication
+#
+# @mode: the replication mode
+#
+# @top-id: #optional In secondary mode, node name or device ID of the root
+#          node who owns the replication node chain. Ignored in primary mode.
+#
+# 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
@@ -2142,6 +2160,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',