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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
