Re: [PATCH 05/29] libxc/progress: Repurpose the current progress reporting infrastructure

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 11/09/14 11:32, Ian Campbell wrote:
> On Wed, 2014-09-10 at 18:10 +0100, Andrew Cooper wrote:
>> Not everything which needs reporting as progress comes with a range.
> Please can you expand on your reasoning wrt the places where you are
> removing a usage of start with a size.

I suppose more correctly, "I wish to introduce new uses of the progress
functionality, which involves reporting progress without a range".

>  Especially since you aren't
> changing any subsequent progress_step call to a you new singleton
> variant.

xc_domain_{save,restore}() are the only generators of progress, and they
are/were being completely replaced, which would make
xc_domain_{save,restore}2() the only generators of progress.

>
>> Allow reporting "0 of 0" for a single progress statement.
> Can we not arrange to suppress this entirely? Perhaps by turning total=0
> into percent=-1 and having the lower level code omit that bit of the
> message in that case? If doing that you should update xentoollog.h to
> make this clear.

The text string is generated by the provider of the progress callback,
and I wanted a way which didn't alter this callback.

Changing to use percent=-1 for this would also work, but produce less
meaningful messages by an existing implementation expecting the old
behaviour.

>
> It appears you are also doing more than just this as well, by changing
> start to set for some reason. Even if you want to change the parameters
> it's not clear that the new name is in any way an improvement.
>
> Thirdly you appear to also be arranging to make it allowable to call
> progress_step without having previously called progress_start/set. Why
> is this needed?

It logically separates setting the string describing the current step,
and providing the progress numbers.

This is IMO a failure in the previous design, and the distinction is
used by the v2 code so the common functions to shunt pages don't need to
be handed a string from their caller to identify the exact current step
(which has already been latched in xch anyway).

~Andrew