Re: [PATCH] qapi: better docs for calc-dirty-rate and friends

[Markus Armbruster]

<gudkov.andrei@huawei.com> writes:

> On Thu, May 25, 2023 at 03:08:35PM +0200, Markus Armbruster wrote:
>> Andrei Gudkov <gudkov.andrei@huawei.com> writes:
>> 
>> > Rewrote calc-dirty-rate documentation. Briefly described
>> > different modes of dirty page rate measurement. Added some
>> > examples. Fixed obvious grammar errors.
>> >
>> > Signed-off-by: Andrei Gudkov <gudkov.andrei@huawei.com>
>> > ---
>> >  qapi/migration.json | 107 +++++++++++++++++++++++++++++++-------------
>> >  1 file changed, 77 insertions(+), 30 deletions(-)
>> >
>> > diff --git a/qapi/migration.json b/qapi/migration.json
>> > index 179af0c4d8..19b51444b5 100644
>> > --- a/qapi/migration.json
>> > +++ b/qapi/migration.json

[...]

>> > -# @sample-pages: page count per GB for sample dirty pages the default
>> > -#     value is 512 (since 6.1)
>> > +# @sample-pages: number of sampled pages per each GiB of guest
>> 
>> per GiB
>> 
>> > +#     memory.  Value is valid only in page-sampling mode (Since 6.1)
>> 
>> Suggest "Valid only in ..."

Outside this patch's scope, but here goes anway: I think we should have
made this member optional, and present only when it's actually valid.
Too late.

[...]

>> > +# Dirty page rate is the number of pages changed in a given time
>> > +# period expressed in MiB/s.  The following methods of calculation
>> > +# are available:
>> > +#
>> > +# 1. In page sampling mode, a random subset of pages are selected
>> > +#    and hashed twice: once in the beginning of measurement time
>> 
>> Suggest "once at the beginning"
>> 
>> > +#    period, another one -- in the end.  If two hashes for some page
>> 
>> Suggest ", and once again at the end".
>> 
>> > +#    are different, the page is counted as changed.  Since this
>> > +#    method relies on sampling and hashing, calculated dirty page
>> > +#    rate is only the estimation of its true value.  Setting

I'll change this to "is only an estimate of" if you don't mind.

[...]

>> This is *sooo* much better than before.  Thank you!
>> 
>> An R-by from a migration maintainer would be nice.

Got Peter's Acked-by now; all set.

>> If you agree with my suggestions, I can apply them in my tree, saving
>> you a respin.  Let me know.
>
> Yes, sure. Please include also suggestion about wr-protect from Peter. Thanks.

Done.

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

Queued.  Thanks!