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