[PATCH] qapi: Fix format of the memory-backend-file's @rom property doc comment

Stefano Garzarella posted 1 patch 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20240229105826.16354-1-sgarzare@redhat.com
Maintainers: Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eduardo Habkost <eduardo@habkost.net>
qapi/qom.json | 27 ++++++++++++++-------------
1 file changed, 14 insertions(+), 13 deletions(-)
[PATCH] qapi: Fix format of the memory-backend-file's @rom property doc comment
Posted by Stefano Garzarella 9 months ago
Reflow paragraph following commit a937b6aa73 ("qapi: Reformat doc
comments to conform to current conventions"): use 4 spaces indentation,
70 columns width, and two spaces to separate sentences.

Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 qapi/qom.json | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/qapi/qom.json b/qapi/qom.json
index 2a6e49365a..db1b0fdea2 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -668,19 +668,20 @@
 # @readonly: if true, the backing file is opened read-only; if false,
 #     it is opened read-write.  (default: false)
 #
-# @rom: whether to create Read Only Memory (ROM) that cannot be modified
-#       by the VM.  Any write attempts to such ROM will be denied.  Most
-#       use cases want writable RAM instead of ROM.  However, selected use
-#       cases, like R/O NVDIMMs, can benefit from ROM.  If set to 'on',
-#       create ROM; if set to 'off', create writable RAM;  if set to
-#       'auto', the value of the @readonly property is used.  This
-#       property is primarily helpful when we want to have proper RAM in
-#       configurations that would traditionally create ROM before this
-#       property was introduced: VM templating, where we want to open a
-#       file readonly (@readonly set to true) and mark the memory to be
-#       private for QEMU (@share set to false).  For this use case, we need
-#       writable RAM instead of ROM, and want to set this property to 'off'.
-#       (default: auto, since 8.2)
+# @rom: whether to create Read Only Memory (ROM) that cannot be
+#     modified by the VM.  Any write attempts to such ROM will be
+#     denied.  Most use cases want writable RAM instead of ROM.
+#     However, selected use cases, like R/O NVDIMMs, can benefit from
+#     ROM.  If set to 'on', create ROM; if set to 'off', create
+#     writable RAM; if set to 'auto', the value of the @readonly
+#     property is used.  This property is primarily helpful when we
+#     want to have proper RAM in configurations that would
+#     traditionally create ROM before this property was introduced: VM
+#     templating, where we want to open a file readonly (@readonly set
+#     to true) and mark the memory to be private for QEMU (@share set
+#     to false).  For this use case, we need writable RAM instead of
+#     ROM, and want to set this property to 'off'.  (default: auto,
+#     since 8.2)
 #
 # Since: 2.1
 ##
-- 
2.44.0
Re: [PATCH] qapi: Fix format of the memory-backend-file's @rom property doc comment
Posted by David Hildenbrand 9 months ago
On 29.02.24 11:58, Stefano Garzarella wrote:
> Reflow paragraph following commit a937b6aa73 ("qapi: Reformat doc
> comments to conform to current conventions"): use 4 spaces indentation,
> 70 columns width, and two spaces to separate sentences.
> 
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>   qapi/qom.json | 27 ++++++++++++++-------------
>   1 file changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 2a6e49365a..db1b0fdea2 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -668,19 +668,20 @@
>   # @readonly: if true, the backing file is opened read-only; if false,
>   #     it is opened read-write.  (default: false)
>   #
> -# @rom: whether to create Read Only Memory (ROM) that cannot be modified
> -#       by the VM.  Any write attempts to such ROM will be denied.  Most
> -#       use cases want writable RAM instead of ROM.  However, selected use
> -#       cases, like R/O NVDIMMs, can benefit from ROM.  If set to 'on',
> -#       create ROM; if set to 'off', create writable RAM;  if set to
> -#       'auto', the value of the @readonly property is used.  This
> -#       property is primarily helpful when we want to have proper RAM in
> -#       configurations that would traditionally create ROM before this
> -#       property was introduced: VM templating, where we want to open a
> -#       file readonly (@readonly set to true) and mark the memory to be
> -#       private for QEMU (@share set to false).  For this use case, we need
> -#       writable RAM instead of ROM, and want to set this property to 'off'.
> -#       (default: auto, since 8.2)
> +# @rom: whether to create Read Only Memory (ROM) that cannot be
> +#     modified by the VM.  Any write attempts to such ROM will be
> +#     denied.  Most use cases want writable RAM instead of ROM.
> +#     However, selected use cases, like R/O NVDIMMs, can benefit from
> +#     ROM.  If set to 'on', create ROM; if set to 'off', create
> +#     writable RAM; if set to 'auto', the value of the @readonly
> +#     property is used.  This property is primarily helpful when we
> +#     want to have proper RAM in configurations that would
> +#     traditionally create ROM before this property was introduced: VM
> +#     templating, where we want to open a file readonly (@readonly set
> +#     to true) and mark the memory to be private for QEMU (@share set
> +#     to false).  For this use case, we need writable RAM instead of
> +#     ROM, and want to set this property to 'off'.  (default: auto,
> +#     since 8.2)
>   #
>   # Since: 2.1
>   ##

Ideally, we'd have a format checker that complains like checkpatch 
usually would.

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb
Re: [PATCH] qapi: Fix format of the memory-backend-file's @rom property doc comment
Posted by Markus Armbruster 9 months ago
David Hildenbrand <david@redhat.com> writes:

> On 29.02.24 11:58, Stefano Garzarella wrote:
>> Reflow paragraph following commit a937b6aa73 ("qapi: Reformat doc
>> comments to conform to current conventions"): use 4 spaces indentation,
>> 70 columns width, and two spaces to separate sentences.
>>
>> Suggested-by: Markus Armbruster <armbru@redhat.com>
>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>> ---
>>   qapi/qom.json | 27 ++++++++++++++-------------
>>   1 file changed, 14 insertions(+), 13 deletions(-)
>> diff --git a/qapi/qom.json b/qapi/qom.json
>> index 2a6e49365a..db1b0fdea2 100644
>> --- a/qapi/qom.json
>> +++ b/qapi/qom.json
>> @@ -668,19 +668,20 @@
>>  # @readonly: if true, the backing file is opened read-only; if false,
>>  #     it is opened read-write.  (default: false)
>>  #
>> -# @rom: whether to create Read Only Memory (ROM) that cannot be modified
>> -#       by the VM.  Any write attempts to such ROM will be denied.  Most
>> -#       use cases want writable RAM instead of ROM.  However, selected use
>> -#       cases, like R/O NVDIMMs, can benefit from ROM.  If set to 'on',
>> -#       create ROM; if set to 'off', create writable RAM;  if set to
>> -#       'auto', the value of the @readonly property is used.  This
>> -#       property is primarily helpful when we want to have proper RAM in
>> -#       configurations that would traditionally create ROM before this
>> -#       property was introduced: VM templating, where we want to open a
>> -#       file readonly (@readonly set to true) and mark the memory to be
>> -#       private for QEMU (@share set to false).  For this use case, we need
>> -#       writable RAM instead of ROM, and want to set this property to 'off'.
>> -#       (default: auto, since 8.2)
>> +# @rom: whether to create Read Only Memory (ROM) that cannot be
>> +#     modified by the VM.  Any write attempts to such ROM will be
>> +#     denied.  Most use cases want writable RAM instead of ROM.
>> +#     However, selected use cases, like R/O NVDIMMs, can benefit from
>> +#     ROM.  If set to 'on', create ROM; if set to 'off', create
>> +#     writable RAM; if set to 'auto', the value of the @readonly
>> +#     property is used.  This property is primarily helpful when we
>> +#     want to have proper RAM in configurations that would
>> +#     traditionally create ROM before this property was introduced: VM
>> +#     templating, where we want to open a file readonly (@readonly set
>> +#     to true) and mark the memory to be private for QEMU (@share set
>> +#     to false).  For this use case, we need writable RAM instead of
>> +#     ROM, and want to set this property to 'off'.  (default: auto,
>> +#     since 8.2)
>>   #
>>   # Since: 2.1
>>   ##
>
> Ideally, we'd have a format checker that complains like checkpatch usually would.

I agree!

A documentation pretty-printer would be best, but feels too hard.  reST
syntax is baroque, and our own extensions make it more so.

A checker catching common mistakes should be feasible.

Patches welcome :)

> Reviewed-by: David Hildenbrand <david@redhat.com>

Thanks!

Reviewed-by: Markus Armbruster <armbru@redhat.com>