On Tue, Jan 18, 2022 at 5:15 PM Michal Privoznik <mpriv...@redhat.com> wrote:
> This reverts commit 938382b60ae5bd1f83b5cb09e1ce68b9a88f679a. > > Turns out, the commit did more harm than good. It changed > semantics on some public APIs. For instance, while > qemuDomainGetInfo() previously did not returned an error it does > now. While the calls to virProcessGetStatInfo() is guarded with > virDomainObjIsActive() it doesn't necessarily mean that QEMU's > PID is still alive. QEMU might be gone but we just haven't > realized it (e.g. because the eof handler thread is waiting for a > job). Doh! > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2041610 > Signed-off-by: Michal Privoznik <mpriv...@redhat.com> > --- > src/ch/ch_driver.c | 2 ++ > src/qemu/qemu_driver.c | 7 ++++++- > src/util/virprocess.c | 8 ++------ > 3 files changed, 10 insertions(+), 7 deletions(-) > > diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c > index 3cbc668489..53e0872207 100644 > --- a/src/ch/ch_driver.c > +++ b/src/ch/ch_driver.c > @@ -1073,6 +1073,8 @@ chDomainHelperGetVcpus(virDomainObj *vm, > if (virProcessGetStatInfo(&vcpuinfo->cpuTime, > &vcpuinfo->cpu, NULL, > vm->pid, vcpupid) < 0) { > + virReportSystemError(errno, "%s", > + _("cannot get vCPU placement & pCPU > time")); > return -1; > } > } > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index e150b86cef..373cd62536 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -1357,6 +1357,8 @@ qemuDomainHelperGetVcpus(virDomainObj *vm, > if (virProcessGetStatInfo(&vcpuinfo->cpuTime, > &vcpuinfo->cpu, NULL, > vm->pid, vcpupid) < 0) { > + virReportSystemError(errno, "%s", > + _("cannot get vCPU placement & pCPU > time")); > return -1; > } > } > @@ -2517,6 +2519,8 @@ qemuDomainGetInfo(virDomainPtr dom, > if (virDomainObjIsActive(vm)) { > if (virProcessGetStatInfo(&(info->cpuTime), NULL, NULL, > vm->pid, 0) < 0) { > + virReportError(VIR_ERR_OPERATION_FAILED, "%s", > + _("cannot read cputime for domain")); > goto cleanup; > } > } > @@ -10524,7 +10528,8 @@ qemuDomainMemoryStatsInternal(virQEMUDriver > *driver, > } > > if (virProcessGetStatInfo(NULL, NULL, &rss, vm->pid, 0) < 0) { > - virResetLastError(); > + virReportError(VIR_ERR_OPERATION_FAILED, "%s", > + _("cannot get RSS for domain")); > } else { > stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_RSS; > stats[ret].val = rss; > diff --git a/src/util/virprocess.c b/src/util/virprocess.c > index 85d8c8e747..b559a4257e 100644 > --- a/src/util/virprocess.c > +++ b/src/util/virprocess.c > @@ -1784,10 +1784,7 @@ virProcessGetStatInfo(unsigned long long *cpuTime, > virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_STIME], NULL, 10, > &systime) < 0 || > virStrToLong_l(proc_stat[VIR_PROCESS_STAT_RSS], NULL, 10, &rss) < > 0 || > virStrToLong_i(proc_stat[VIR_PROCESS_STAT_PROCESSOR], NULL, 10, > &cpu) < 0) { > - virReportError(VIR_ERR_INTERNAL_ERROR, > - _("cannot parse process status data for pid > '%d/%d'"), > - (int) pid, (int) tid); > - return -1; > + VIR_WARN("cannot parse process status data"); > } > > /* We got jiffies > @@ -1884,8 +1881,7 @@ virProcessGetStatInfo(unsigned long long *cpuTime > G_GNUC_UNUSED, > pid_t pid G_GNUC_UNUSED, > pid_t tid G_GNUC_UNUSED) > { > - virReportSystemError(ENOSYS, "%s", > - _("Process statistics data is not supported on > this platform")); > + errno = ENOSYS; > return -1; > } > > -- > 2.34.1 > >