[PATCH 0/4] Fix vcpu stats present for inactive VMs and allow virsh completion while daemon is down
Two things that annoyed me while I tried to reply to a mail. Peter Krempa (4): qemuDomainGetStatsCpuProc: Don't fetch stats for inactive VMs virProcessGetStatInfo: Improve debug message vsh: Suppress attempts to write to stderr when it was closed in 'cmdComplete' vsh: cmdComplete: Don't exit when connecting to the daemon fails src/qemu/qemu_driver.c | 3 +++ src/util/virprocess.c | 4 ++-- tools/vsh.c | 12 ++++++++++-- tools/vsh.h | 2 ++ 4 files changed, 17 insertions(+), 4 deletions(-) -- 2.54.0
From: Peter Krempa <pkrempa@redhat.com> CPU stats for inactive VM make no sense. In this case it's especially misleading because 'vm->pid' of an inactive VM is '0' so virProcessGetStat returns stats for virtqemud itself. Fixes: 044b8744d65f8571038f85685b3c4b241162977b Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d227ac58cd..529e9fe3be 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17156,6 +17156,9 @@ qemuDomainGetStatsCpuProc(virDomainObj *vm, unsigned long long userTime = 0; unsigned long long sysTime = 0; + if (!virDomainObjIsActive(vm)) + return; + if (virProcessGetStatInfo(&cpuTime, &userTime, &sysTime, NULL, NULL, vm->pid, 0) < 0) { /* ignore error */ -- 2.54.0
From: Peter Krempa <pkrempa@redhat.com> Label the 'pid'/'tid' field. Use proper typecast also for 'tid'. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virprocess.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virprocess.c b/src/util/virprocess.c index e06ed94b0d..97843d6f75 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -1808,8 +1808,8 @@ virProcessGetStatInfo(unsigned long long *cpuTime, *vm_rss = rss * pagesize; - VIR_DEBUG("Got status for %d/%d user=%llu sys=%llu cpu=%d rss=%lld", - (int) pid, tid, utime, stime, cpu, rss); + VIR_DEBUG("Got process stats for pid=%d tid=%d: user=%llu sys=%llu cpu=%d rss=%lld", + (int) pid, (int) tid, utime, stime, cpu, rss); return 0; } -- 2.54.0
From: Peter Krempa <pkrempa@redhat.com> The completer closes stderr to suppress anything poluting the shell when completion would cause any errors. Since 'virshReconnect' would call 'vshError' on connection failure this causes vshError to be killed by SIGPIPE and not provide any completions if the connection is not possible. To avoid this add a flag called 'stderr_closed' to vshControl and use it to suppress output in 'vshPrintStderr'. Keep only the log. Prior to this patch, attempt to run completion on a host with all daemons shut down would result in: # virsh complete -- "start" "--doma" ; echo $? 141 # With this patch the completion will still fail but the return code will be 1. Further patch will allow completion. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/vsh.c | 5 +++++ tools/vsh.h | 2 ++ 2 files changed, 7 insertions(+) diff --git a/tools/vsh.c b/tools/vsh.c index 74016c0043..c84d17a332 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -2145,6 +2145,9 @@ vshPrintStderr(vshControl *ctl, if (ctl) vshOutputLogFile(ctl, level, str); + if (ctl->stderr_closed) + return; + /* Most output is to stdout, but if someone ran virsh 2>&1, then * printing to stderr will not interleave correctly with stdout * unless we flush between every transition between streams. */ @@ -3577,6 +3580,8 @@ cmdComplete(vshControl *ctl, const vshCmd *cmd) if (!ctl->imode) { if (virOnce(&vshCmdCompleteCloseStdinStderrOnce, vshCmdCompleteCloseStdinStderr) < 0) return false; + + ctl->stderr_closed = true; } if (!(hooks && hooks->connHandler && hooks->connHandler(ctl))) diff --git a/tools/vsh.h b/tools/vsh.h index bd2494e899..1c5a69b04e 100644 --- a/tools/vsh.h +++ b/tools/vsh.h @@ -225,6 +225,8 @@ struct _vshControl { const vshClientHooks *hooks;/* mandatory client specific hooks */ void *privData; /* client specific data */ + + bool stderr_closed; /* stderr was closed for the 'complete' command */ }; typedef void * -- 2.54.0
On a Wednesday in 2026, Peter Krempa via Devel wrote:
From: Peter Krempa <pkrempa@redhat.com>
The completer closes stderr to suppress anything poluting the shell when
*polluting
completion would cause any errors.
Since 'virshReconnect' would call 'vshError' on connection failure this causes vshError to be killed by SIGPIPE and not provide any completions if the connection is not possible.
To avoid this add a flag called 'stderr_closed' to vshControl and use it to suppress output in 'vshPrintStderr'. Keep only the log.
Prior to this patch, attempt to run completion on a host with all daemons shut down would result in:
# virsh complete -- "start" "--doma" ; echo $? 141 #
With this patch the completion will still fail but the return code will be 1. Further patch will allow completion.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/vsh.c | 5 +++++ tools/vsh.h | 2 ++ 2 files changed, 7 insertions(+)
Jano
From: Peter Krempa <pkrempa@redhat.com> Invoke the 'connHandler' without checking return value. 'virsh complete' can provide useful completions even when the daemon connection is broken. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/vsh.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index c84d17a332..ccf2f21d64 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -3584,8 +3584,11 @@ cmdComplete(vshControl *ctl, const vshCmd *cmd) ctl->stderr_closed = true; } - if (!(hooks && hooks->connHandler && hooks->connHandler(ctl))) - return false; + /* Attempt connecting so that we can also do completions based on e.g. + * object names work. Failure to connect is not fatal because we want + * at least argument completion */ + if (hooks && hooks->connHandler) + ignore_value(hooks->connHandler(ctl)); vshReadlineInit(ctl); -- 2.54.0
On a Wednesday in 2026, Peter Krempa via Devel wrote:
Two things that annoyed me while I tried to reply to a mail.
Peter Krempa (4): qemuDomainGetStatsCpuProc: Don't fetch stats for inactive VMs virProcessGetStatInfo: Improve debug message vsh: Suppress attempts to write to stderr when it was closed in 'cmdComplete' vsh: cmdComplete: Don't exit when connecting to the daemon fails
src/qemu/qemu_driver.c | 3 +++ src/util/virprocess.c | 4 ++-- tools/vsh.c | 12 ++++++++++-- tools/vsh.h | 2 ++ 4 files changed, 17 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa