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;

The virRandomGenerateWWN() is used solely by nodedev driver to autogenerate WWNN and WWNP when parsing a nodedev XML. Now, the idea was (at least during monolithic daemon) that depending on which hypervisor driver called the nodedev XML parsing (and virRandomGenerateWWN() under the hood) the corresponding OUI is used (e.g. "001a4a" for the QEMU driver). But in era of split daemons things are not that easy. We do not know which hypervisor driver called us. And there might be no hypervisor driver at all - users are allowed to connect to individual drivers directly (e.g. "nodedev:///system"). In this case, we can't use proper OUI. Well, do the next best thing: pick one. By a fair roll of dice the one used by the QEMU driver (QUMRANET_OUI) was chosen. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/util/virrandom.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/util/virrandom.c b/src/util/virrandom.c index 73c5832a05..7606dd1684 100644 --- a/src/util/virrandom.c +++ b/src/util/virrandom.c @@ -138,7 +138,13 @@ virRandomGenerateWWN(char **wwn, return -1; } - if (STREQ(virt_type, "QEMU")) { + /* In case of split daemon we don't really see the hypervisor + * driver that just re-routed the nodedev driver API. There + * might not be any hypervisor driver even. Yet, we have to + * pick OUI. Pick "QEMU". */ + + if (STREQ(virt_type, "QEMU") || + STREQ(virt_type, "nodedev")) { oui = QUMRANET_OUI; } else if (STREQ(virt_type, "Xen") || STREQ(virt_type, "xenlight")) {