[PATCH] Revert "report error when virProcessGetStatInfo() is unable to parse data"

Michal Prívozník mprivozn at redhat.com
Thu Jan 20 15:44:18 UTC 2022


On 1/20/22 16:31, Ani Sinha wrote:
> 
> 
> On Thu, 20 Jan 2022, Andrea Bolognani wrote:
> 
>> On Tue, Jan 18, 2022 at 12:44:51PM +0100, Michal Privoznik wrote:
>>> +++ b/src/util/virprocess.c
>>> @@ -1784,10 +1784,7 @@ virProcessGetStatInfo(unsigned long long *cpuTime,
>>>          virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_STIME], NULL, 10, &systime) < 0 ||
>>>          virStrToLong_l(proc_stat[VIR_PROCESS_STAT_RSS], NULL, 10, &rss) < 0 ||
>>>          virStrToLong_i(proc_stat[VIR_PROCESS_STAT_PROCESSOR], NULL, 10, &cpu) < 0) {
>>> -        virReportError(VIR_ERR_INTERNAL_ERROR,
>>> -                       _("cannot parse process status data for pid '%d/%d'"),
>>> -                       (int) pid, (int) tid);
>>> -        return -1;
>>> +        VIR_WARN("cannot parse process status data");
>>
>> Shame to lose the improved error/warning message. Perhaps it could be
>> reintroduced separately.
>>
> 
> Functions in general are not coded around success path only. Most
> well written functions have a success path and an error path. In case of
> error, they should be able to report warning/errors. If raising an error
> from a function causes a breakage of an external api, then that is an
> architectural problem imho. Instead of reverting the error/warning, we
> should try to fix the larger problem at hand in the longer term.
> 

Well, until we get there we should fix the upstream so that we don't
have another broken release.

Michal




More information about the libvir-list mailing list