Re: [Xen-devel] [PATCH v9 02/15] libxc/progress: Extend the progress interface

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 15/04/15 11:55, Ian Campbell wrote:
> On Fri, 2015-04-10 at 18:15 +0100, Andrew Cooper wrote:
>> Not everything which needs reporting as progress comes with a range.  Extend
>> the interface to allow reporting of a single statement.
>>
>> The programming interface now looks like:
>>   xc_set_progress_prefix()
>>     set the prefix string to be used
>>   xc_report_progress_single()
>>     report a single action
>>   xc_report_progress_step()
>>     report $X of $Y
>>
>> The new programming interface is implemented in a compatible way with the
>> existing caller interface (by reporting a single action as "0 of 0").
> I suppose the underlying motivation here is that there a difference
> between this new xc_report_progress_step and calling xc_report/v? IOW
> some difference between the semantics of the logger's ->vmessage and
> ->progress hooks. What is it though?

They arrive via separate callbacks to the callee.

xc_report_progress & friends come through the domain build logger
(parameter 2 of xc_interface_open()) while other logging tends to come
through the regular logger (parameter 1 of xc_interface_open()).

Technically speaking, xc_report/v does allow the caller to choose which
logger is used, but also requires the caller to provide all the fine
detail, which detracts from the readability of the callsite.

This new progress API attempts to resolve this by providing a high level
way of putting a single message through the progress logger.

>
> I suspected the distinction was in the automatic inclusion of
> xch->currently_progress_reporting into the messages, but you appear to
> make that non-mandatory below.
>
> Speaking of which, I think it should be mandatory now to call
> xc_set_progress_prefix as it was to call progress_start before, and that
> both of your new functions should assert. Those who think they want to
> use xc_report_progress_single without calling xc_set_progress_prefix
> should be using xc_report() instead.

Technically speaking it is safe to use a NULL prefix; the vsprintf won't
fall over.

I can assert() that these invariants are held, but it is not critical to
the functionality.

~Andrew

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