From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 05/29] libxc/progress: Repurpose the current progress reporting infrastructure Date: Thu, 11 Sep 2014 15:03:43 +0100 Message-ID: <5411ABBF.1040503@citrix.com> References: <1410369067-1330-1-git-send-email-andrew.cooper3@citrix.com> <1410369067-1330-6-git-send-email-andrew.cooper3@citrix.com> <1410431533.6166.68.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1410431533.6166.68.camel@kazak.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: Ian Jackson , Xen-devel List-Id: xen-devel@lists.xenproject.org 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