Re: [PATCH] virprocess: Provide non-Linux stubs for virProcessGet{Stat, Sched}Info
On a Friday in 2022, Ani Sinha wrote:
On Fri, 7 Jan 2022, Michal Prívozník wrote:
> I don't think so. Just like we've discussed under one patch of yours, a
> function should either report error in all cases or none. And in case of
> virProcessGetSchedInfo() the linux version does report error
I see your point but there is also a bug in that function - not all error
paths report errors. For example, !proc and !lines cases. We need to fix
that.
I don't see a !proc error path in virProcessGetSchedInfo.
The !lines case is inconsistent but thankfully it can only happen
if /proc/%d/sched is empty.
hence the
> non-linux variant should report an error too. And in case of
> virProcessGetStatInfo() no error is reported for linux version thus
> non-linux version shouldn't report an error.
>
No this is not my understanding from the discussion. What I understood is
that the lowest level of functions should always report error when an
error path is encountered.
Errors should be reported by the function that has the context to
provide a helpful error message. For some low-level helpers, we rely
on errno's and let the caller report a less-specific error - either
out of laziness because (like above) there would have to be a buggy
kernel, or because most of the code paths are syscalls which set errno.
Jano
For example virFileReadAll() does this nicely.
Currently there is no error path in virProcessGetStatInfo() and it
uncondiotionally returns 0. For non-linux variant, I think it would be
correct to report an error.
Now having done that, we should also fix the callers so that the callers
are not overriding the narrower errors with broader ones.
Attachments:
- signature.asc (application/pgp-signature — 488 bytes)