[PATCH v3] report error when virProcessGetStatInfo() is unable to parse data

[Michal Prívozník]

On 1/11/22 11:20, 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.
> 
> Fix the callers so that they do not override the error generated.
> Also fix non-linux implementation of this function so as to report error.
> 
> Signed-off-by: Ani Sinha <ani at anisinha.ca>
> ---
>  src/ch/ch_driver.c     | 2 --
>  src/qemu/qemu_driver.c | 7 +------
>  src/util/virprocess.c  | 9 +++++++--
>  3 files changed, 8 insertions(+), 10 deletions(-)
> 
> changelog:
> v3: fix non-linux implementation
> v2: fix callers
> 

> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index d3d76c003f..9a17c93b08 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c

> @@ -10530,8 +10526,7 @@ qemuDomainMemoryStatsInternal(virQEMUDriver *driver,
>      }
>  
>      if (virProcessGetStatInfo(NULL, NULL, &rss, vm->pid, 0) < 0) {
> -        virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> -                       _("cannot get RSS for domain"));
> +        return -1;

Returning -1 changes semantics of this function. Previously it was
tolerant to errors and now it isn't. For instance, if this function was
called on a FreeBSD machine (yes, QEMU driver can be enabled there) then
despite fetching mem stats earlier through monitor (not visible in the
context) an error is now returned which in turn makes whole
virDomainMemoryStats() API fail.

The 'return -1' should be replaced with virResetLastError().

Having said that, now there will be an error reported in the logs every
time the API is called. I wonder what we can do about it. Previously it
was "just" a warning.

Michal