Re: [PATCH] Revert "report error when virProcessGetStatInfo() is unable to parse data"
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.