On Fri, 7 Jan 2022, Michal Prívozník wrote:
On 1/7/22 10:09, Ani Sinha wrote:
> Currently virProcessGetStatInfo() always returns success and only logs error
> when it is unable to parse the data. Make this function actually report the
> error and return a negative value in this error scenario.
>
> Signed-off-by: Ani Sinha <ani(a)anisinha.ca>
> ---
> src/util/virprocess.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/src/util/virprocess.c b/src/util/virprocess.c
> index c74bd16fe6..b9f498d5d8 100644
> --- a/src/util/virprocess.c
> +++ b/src/util/virprocess.c
> @@ -1783,7 +1783,11 @@ 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) {
> - VIR_WARN("cannot parse process status data");
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("cannot parse process status data for pid
'%d/%d'"),
> + (int) pid, (int) tid);
> +
> + return -1;
> }
>
> /* We got jiffies
Couple of problems with this patch as is. I'm not against the idea of
reporting an error here.
Good. now we are moving in the right direction.
But couple of things needs to be addressed first:
1) Currently, all callers check for retval and report an error if -1 was
returned. This means, that even though this new message is reported it
is immediately overwritten in caller.
Let me fix the callers and send an updated patch. Meanwhile ...
2) The non-linux implementation now has to report error too. I believe
it's obvious why from our previous discussion this morning.
Maybe you can fix your patch.