Jes Sorensen <jes.soren...@redhat.com> writes: > On 05/06/11 17:10, Markus Armbruster wrote: >> Jes Sorensen <jes.soren...@redhat.com> writes: >>> 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. >> */ > > Thanks! I made it a little more explicit based on your input: > > /* > * Report progress. > * @delta is how much progress we made. > * If @max is zero, @delta is an absolut value of the total job done. > * Else, @delta is a progress delta since the last call, as a fraction > * of @max. I.e. the delta is @delta * @max / 100. This allows > * relative accounting of functions which may be a different fraction of > * the full job, depending on the context they are called in. I.e. > * a function might be considered 40% of the full job if used from > * bdrv_img_create() but only 20% if called from img_convert(). > */
Looks good. >> 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. >> */ > > excellent! > >> 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) > > I have to say I prefer having the max setting here - what would be an > option would be to keep the max value as a state, and then have a > qemu_progress_set_current_max() or something like that, so you wouldn't > have to type it every time? I guess that could make it actually convenient, because... >> 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 reason for introducing this is that some functions are called in > different ways, and to keep the same accounting code, it would be > possible to add the 'max' argument to those functions when they do their > counting. ... you wouldn't have to pass around these max arguments then. > It is an attempt to be forward compatible for when it happens :) Based on my own track record at predicting the future, I've come to refrain from providing convenience features for future needs, in particular when it complicates things. >> 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. > > Getting rid of the absolute setting would be fine with me. You're right > that it is quite easy to set it that way. > > Cheers, > Jes
