[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH v22 23/30] qmp: add persistent flag
Re: [Qemu-block] [Qemu-devel] [PATCH v22 23/30] qmp: add persistent flag to block-dirty-bitmap-add
Fri, 07 Jul 2017 15:25:26 +0200
Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux)
Vladimir Sementsov-Ogievskiy <address@hidden> writes:
> 07.07.2017 10:54, Markus Armbruster wrote:
>> QAPI schema review only... I apologize for its lateness.
>> Vladimir Sementsov-Ogievskiy <address@hidden> writes:
>>> Add optional 'persistent' flag to qmp command block-dirty-bitmap-add.
>>> Default is false.
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>>> Signed-off-by: Denis V. Lunev <address@hidden>
>>> Reviewed-by: Max Reitz <address@hidden>
>>> Reviewed-by: John Snow <address@hidden>
>>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>>> index f85c2235c7..13f98ec146 100644
>>> --- a/qapi/block-core.json
>>> +++ b/qapi/block-core.json
>>> @@ -1561,10 +1561,16 @@
>>> # @granularity: the bitmap granularity, default is 64k for
>>> # block-dirty-bitmap-add
>>> +# @persistent: the bitmap is persistent, i.e. it will be saved to the
>>> +# corresponding block device image file on its close. For now
>>> +# Qcow2 disks support persistent bitmaps. Default is false for
>>> +# block-dirty-bitmap-add. (Since: 2.10)
>> "for block-dirty-bitmap-add" suggests there could be other users, with
>> different (but unspecified) defaults. What about replacing the sentence
>> by "(default: false)"?
>> Please wrap your comment lines around column 70.
> Why 70, is it written somewhere? There are a lot of lines over 70
> characters in this file, so, as series are already in Max's block
> branch I think it would be better to fix the whole file, if it is
> really needed.
There's no hard rule on comment line length. I routinely advise people
to wrap anyway, for legibility. Humans tend to have trouble following
long lines with their eyes (I sure do). Typographic manuals suggest to
limit columns to roughly 60 characters for exactly that reason[*].
I'm not the maintainer here, so this is really advice, not a demand.