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

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.