[PATCH] Revert "report error when virProcessGetStatInfo() is unable to parse data"
Ani Sinha
ani at anisinha.ca
Thu Jan 20 15:48:08 UTC 2022
On Thu, Jan 20, 2022 at 21:14 Michal Prívozník <mprivozn at redhat.com> wrote:
> 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.
AKA kicking the can one more time 🙃
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20220120/55fd5af4/attachment-0001.htm>
More information about the libvir-list
mailing list