On Tue, Jul 18, 2023 at 17:27:35 +0200, Michal Privoznik wrote:
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@redhat.com> --- 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")) {
I at first wanted to suggest to simply drop the code selecting the prefix based on the hypervisor type, but this would introduce a behaviour change for any existing monolithic deployment. Since this simply didn't work with modular daemons until now I guess we can argue to change the behaviour in the way you propose. Two other solution I can see in this case are: - use our own OUI (but we don't have one) - try to plumb the hypervisor name if possible through a new API (complex, unclear benefit, doesn't solve all cases anywyas) From my side, please drop the commit message bit about it being chosen by a dice roll. I don't really buy that. Additionally please allow others to chime in (don't push it right after): Reviewed-by: Peter Krempa <pkrempa@redhat.com>