[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] Add documentation for qemu_progres_print()
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] Add documentation for qemu_progres_print() |
Date: |
Fri, 06 May 2011 17:10:47 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) |
Jes Sorensen <address@hidden> writes:
> On 05/06/11 12:40, Brad Hards wrote:
>> On Fri, 6 May 2011 07:39:10 PM address@hidden wrote:
>>> +/*
>>> + * Add delta to current state, and print the output if the current
>>> + * state has progressed more than min_skip since the last value was
>>> + * printed. 'max' specifies the relative percentage, ie. a function
>>> + * can count for 30% of the total work, and still count from 0-100, by
>>> + * setting max to 30. If max is set to zero, the percent argument
>>> + * becomes an absolute value for current state.
>>> + */
>>> void qemu_progress_print(float percent, int max)
>> I hate to critique anyone adding docs, but this makes no sense at all to me
>> without reading the code. Is "percent" the amount we are adding (i.e. the
>> delta) or the result (i.e. absolute progress)? Or does it vary according to
>> the value of max?
>
> What you add is a delta, which is relative to the max. We can change the
> argument name of the function to be delta instead if that makes it
> easier to follow.
Here's my try:
/*
* Report progress.
* @percent is how much progress we made.
* If @max is zero, @percent is how much of the job is done.
* Else, @percent is a progress delta since the last call, as a fraction
* of @max. I.e. delta is @percent * @max / 100. This is for
* convenience, it lets you account for @max% of the total work in some
* function, and still count @percent from 0 to 100.
*/
Document min_skip with qemu_progress_init():
/*
* Initialize progress reporting.
* If @enabled is false, actual reporting is suppressed. The user can
* still trigger a report by sending SIGUSR1.
* Reports are also suppressed unless we've had at least @min_skip
* percent progress since the last report.
*/
To be honest, the @max feature is a pain to document. I'd ditch it.
For non-zero max,
qemu_progress_report(x, max)
becomes
qemu_progress_report(x * max/100)
Not much of an inconvenience, in my opinion. None at all when max is
100, which is the case for all non-zero max arguments so far.
The only use of the absolute feature (zero max) so far is setting it to
"all done", like this:
qemu_progress_print(100, 0);
Can be done just as well with a delta >= 100, e.g.
qemu_progress_print(100);
If a need for setting other absolute progress arises, I'd consider
adding second function.
[Qemu-devel] [PATCH 2/2] qemu-img.c: Remove superfluous parenthesis, Jes . Sorensen, 2011/05/06
Re: [Qemu-devel] [PATCH qemu-block 0/2] cleanup progress code, Markus Armbruster, 2011/05/06