[Qemu-devel] [PATCH] block: modify top-id's comments

[Qemu-devel] [PATCH] block: modify top-id's comments

[Wang WeiWei]

Kevin Wolf's advice:
http://lists.gnu.org/archive/html/qemu-devel/2016-09/msg02391.html

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>
---
 qapi/block-core.json | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index ada3202..a605107 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2184,7 +2184,8 @@
 # @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.
+#          node who owns the replication node chain. Must not be given in
+#          primary mode.
 #
 # Since: 2.8
 ##
-- 
2.7.4

Re: [Qemu-devel] [Qemu-block] [PATCH] block: modify top-id's comments

[Kashyap Chamarthy]

On Fri, Sep 30, 2016 at 04:53:38PM +0800, Wang WeiWei wrote:
> Kevin Wolf's advice:
> http://lists.gnu.org/archive/html/qemu-devel/2016-09/msg02391.html

IIUC, what Stefan meant is to write a simple descriptive commit message
that explains why this change is necessary -- the above URL might have
context, but having it spelled out in the Git commit message will be
useful when someone is examining Git history offline.

I think Stefan was looking for something like (came up with this after
reading the above thread):

  QAPI: BlockdevOptionsReplication: Clarify 'top-id' parameter usage

  In primary mode, the replication driver will ignore 'top-id'
  parameter, thus it must not be supplied.  Explicitly spell that out in
  the QAPI schema for BlockdevOptionsReplication.

> 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>
> ---
>  qapi/block-core.json | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index ada3202..a605107 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2184,7 +2184,8 @@
>  # @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.
> +#          node who owns the replication node chain. Must not be given in
> +#          primary mode.
>  #
>  # Since: 2.8
>  ##
> -- 
> 2.7.4
> 
> 
> 
> 

-- 
/kashyap

Re: [Qemu-devel] [Qemu-block] [PATCH] block: modify top-id's comments

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1251 bytes --]

On Fri, Sep 30, 2016 at 11:48:44AM +0200, Kashyap Chamarthy wrote:
> On Fri, Sep 30, 2016 at 04:53:38PM +0800, Wang WeiWei wrote:
> > Kevin Wolf's advice:
> > http://lists.gnu.org/archive/html/qemu-devel/2016-09/msg02391.html
> 
> IIUC, what Stefan meant is to write a simple descriptive commit message
> that explains why this change is necessary -- the above URL might have
> context, but having it spelled out in the Git commit message will be
> useful when someone is examining Git history offline.
> 
> I think Stefan was looking for something like (came up with this after
> reading the above thread):
> 
>   QAPI: BlockdevOptionsReplication: Clarify 'top-id' parameter usage
> 
>   In primary mode, the replication driver will ignore 'top-id'
>   parameter, thus it must not be supplied.  Explicitly spell that out in
>   the QAPI schema for BlockdevOptionsReplication.

Yes.  Posting just a URL as the commit description is not enough.

I tried to explain the purpose of commit messages/descriptions in my
previous reply.

You can read a lot more about it elsewhere too:

http://chris.beams.io/posts/git-commit/
(The key points are the introduction and "7. Use the body to explain
what and why vs. how")

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH v2] block: modify top-id's comments

[Kevin Wolf]

This is v2, should be included in the subject line.

Am 30.09.2016 um 10:53 hat Wang WeiWei geschrieben:
> Kevin Wolf's advice:
> http://lists.gnu.org/archive/html/qemu-devel/2016-09/msg02391.html
> 
> 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>
> ---
>  qapi/block-core.json | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index ada3202..a605107 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2184,7 +2184,8 @@
>  # @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.
> +#          node who owns the replication node chain. Must not be given in
> +#          primary mode.
>  #
>  # Since: 2.8
>  ##

As I already wrote for v1:

The matching code in block/replication.c is missing to actually error
out if the option is given in primary mode.

Kevin

Re: [Qemu-devel] [PATCH v2] block: modify top-id's comments

[wangweiwei]

Ok,thanks.
On 09/30/2016 05:52 PM, Kevin Wolf wrote:
> This is v2, should be included in the subject line.
>
> Am 30.09.2016 um 10:53 hat Wang WeiWei geschrieben:
>> Kevin Wolf's advice:
>> http://lists.gnu.org/archive/html/qemu-devel/2016-09/msg02391.html
>>
>> 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>
>> ---
>>  qapi/block-core.json | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index ada3202..a605107 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -2184,7 +2184,8 @@
>>  # @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.
>> +#          node who owns the replication node chain. Must not be given in
>> +#          primary mode.
>>  #
>>  # Since: 2.8
>>  ##
>
> As I already wrote for v1:
>
> The matching code in block/replication.c is missing to actually error
> out if the option is given in primary mode.
>
> Kevin
>
>
> .
>