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

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 <ani(a)anisinha.ca&gt; >> --- >> 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?