Re: [PATCH 1/5] virsh: Make cmdVersion() work with split daemon

On Tue, Jul 18, 2023 at 17:27:33 +0200, Michal Privoznik wrote: > When virsh connects to a non-hypervisor daemon directly (e.g. > "nodedev:///system") and user executes 'version' they are met > with an error message. This is because cmdVersion() calls > virConnectGetVersion() which fails, hence the error. > > The reason for virConnectGetVersion() fail is simple - it's > documented as: > > Get the version level of the Hypervisor running. > > Well, there's no hypervisor in non-hypervisor daemons and thus it > doesn't make sense to provide an implementation in each driver's > virConnectDriver.hypervisorDriver table (just like we do for > other APIs, e.g. nodeConnectIsSecure()). > > Given all of this, just make cmdVersion() deal with the error in > a non-fatal fashion. > > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > --- > tools/virsh-host.c | 26 ++++++++++++-------------- > 1 file changed, 12 insertions(+), 14 deletions(-) > > diff --git a/tools/virsh-host.c b/tools/virsh-host.c > index 0bda327cae..35e6a2eb98 100644 > --- a/tools/virsh-host.c > +++ b/tools/virsh-host.c > @@ -1447,21 +1447,19 @@ cmdVersion(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) > vshPrint(ctl, _("Using API: %1$s %2$d.%3$d.%4$d\n"), hvType, > major, minor, rel); > > - if (virConnectGetVersion(priv->conn, &hvVersion) < 0) { > - vshError(ctl, "%s", _("failed to get the hypervisor version")); > - return false; > - } > - if (hvVersion == 0) { > - vshPrint(ctl, > - _("Cannot extract running %1$s hypervisor version\n"), hvType); > - } else { > - major = hvVersion / 1000000; > - hvVersion %= 1000000; > - minor = hvVersion / 1000; > - rel = hvVersion % 1000; > + if (virConnectGetVersion(priv->conn, &hvVersion) >= 0) { > + if (hvVersion == 0) { > + vshPrint(ctl, > + _("Cannot extract running %1$s hypervisor version\n"), hvType); > + } else { > + major = hvVersion / 1000000; > + hvVersion %= 1000000; > + minor = hvVersion / 1000; > + rel = hvVersion % 1000; > > - vshPrint(ctl, _("Running hypervisor: %1$s %2$d.%3$d.%4$d\n"), > - hvType, major, minor, rel); > + vshPrint(ctl, _("Running hypervisor: %1$s %2$d.%3$d.%4$d\n"), > + hvType, major, minor, rel); > + } > } Ideally you'd also add an else branch with 'vshResetLibvirtError(); but the other call to virConnectGetLibVersion() doesn't do that so ... whatever ... I guess :)

> > if (vshCommandOptBool(cmd, "daemon")) { > -- > 2.41.0 > Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt;