From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:59429) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QJQKb-0004dl-Ar for qemu-devel@nongnu.org; Mon, 09 May 2011 09:17:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QJQKa-0001HP-Ak for qemu-devel@nongnu.org; Mon, 09 May 2011 09:17:01 -0400 Received: from mx1.redhat.com ([209.132.183.28]:19044) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QJQKa-0001HL-3Y for qemu-devel@nongnu.org; Mon, 09 May 2011 09:17:00 -0400 Message-ID: <4DC7E900.5010306@redhat.com> Date: Mon, 09 May 2011 15:15:44 +0200 From: Jes Sorensen MIME-Version: 1.0 References: <1304674751-19111-1-git-send-email-Jes.Sorensen@redhat.com> <1304674751-19111-2-git-send-email-Jes.Sorensen@redhat.com> <201105062040.00128.bradh@frogmouth.net> <4DC3D77C.4030703@redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] Add documentation for qemu_progres_print() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org, Brad Hards On 05/06/11 17:10, Markus Armbruster wrote: > Jes Sorensen 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(). */ > 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? > 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. It is an attempt to be forward compatible for when it happens :) > 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