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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2014-09-11 at 15:03 +0100, Andrew Cooper wrote:
> 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).

This may well all be reasonable, in which case please explain it in the
commit log.

> 
> ~Andrew
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel