[PATCH] virprocess: Provide non-Linux stubs for virProcessGet{Stat, Sched}Info

[Michal Prívozník]

On 1/7/22 09:05, Ani Sinha wrote:
> 
> 
> On Fri, 7 Jan 2022, Michal Privoznik wrote:
> 
>> Both virProcessGetStatInfo() and virProcessGetSchedInfo() are
>> Linux centric. Provide stubs for non-Linux platforms.
>>
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---
>>  src/util/virprocess.c | 24 ++++++++++++++++++++++++
>>  1 file changed, 24 insertions(+)
>>
>> diff --git a/src/util/virprocess.c b/src/util/virprocess.c
>> index c74bd16fe6..5788faea9c 100644
>> --- a/src/util/virprocess.c
>> +++ b/src/util/virprocess.c
>> @@ -1766,6 +1766,7 @@ virProcessGetStat(pid_t pid,
>>  }
>>
>>
>> +#ifdef __linux__
>>  int
>>  virProcessGetStatInfo(unsigned long long *cpuTime,
>>                        int *lastCpu,
>> @@ -1873,3 +1874,26 @@ virProcessGetSchedInfo(unsigned long long *cpuWait,
>>
>>      return 0;
>>  }
>> +
>> +#else
>> +int
>> +virProcessGetStatInfo(unsigned long long *cpuTime G_GNUC_UNUSED,
>> +                      int *lastCpu G_GNUC_UNUSED,
>> +                      long *vm_rss G_GNUC_UNUSED,
>> +                      pid_t pid G_GNUC_UNUSED,
>> +                      pid_t tid G_GNUC_UNUSED)
>> +{
>> +    errno = ENOSYS;
>> +    return -1;
>> +}
>> +
>> +int
>> +virProcessGetSchedInfo(unsigned long long *cpuWait G_GNUC_UNUSED,
>> +                       pid_t pid G_GNUC_UNUSED,
>> +                       pid_t tid G_GNUC_UNUSED)
>> +{
>> +    virReportSystemError(ENOSYS, "%s",
>> +                         _("scheduler information is not supported on this platform"));
> 
> We should do this for both functions for non-linux platforms or not do it
> at all. Like it should be consistent IMHO.

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 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.

Think of it like this. You have a function that's called from somewhere.
However, the function has two (or more) implementations (e.g. one for
Linux, the other for FreeBSD, another one for MinGW, and so on). And you
call the function like this:

  var = func();
  if (!var) {
    ...

now should there be an error reported inside the if()? If all
implementations of the func() are consistent then there's a clear answer.

Hope this sheds more light.

Michal