On Tue, 11 Jan 2022, Michal Prívozník wrote:
> 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 <a...@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.
Does v4 help?
