libvirt: use kvm-ok from PATH

[hedrok]

Docs (see docs/install/linux.md) state that kvm-ok should be in PATH and it seems logical, but /usr/sbin/kvm-ok is hardcoded instead, which may not be the case for userspace script or just not available under OS (e.g. NixOS).

Replace hardcoded path with which kvm-ok to check if it is available and just with call kvm-ok to run it from PATH.

[ipspace]

Nice catch, thanks a million. However, I'm worried about existing installations that do not have /usr/sbin in the path. How about we go with which kvm-ok || stat /usr/sbin/kvm-ok and kvm-ok || /usr/sbin/kvm-ok?

[hedrok]

I've tried and unfortunately it doesn't work. I've even tried || true - obviously there is no shell interpretation for these commands...

(networklab) vyos ~/.../netlab$ netlab up -v
...
Checking virtualization provider installation
run_command executing: which kvm-ok '||' true
[FATAL]   KVM is not installed or does not include kvm-ok utility

Yes, netsim/cli/external_commands.py splits string using shlex and gives it to subprocess.run as a list:

  if isinstance(cmd,str):
    try:
      cmd = shlex.split(cmd)
...
  try:
    result = subprocess.run(
                cmd,
...

I see no options to run it with shell interpretation.
I agree with your concern, but I'm totally unfamiliar with netlab codebase, just fixed usual for NixOS user problem. Do you have any suggestions how to both support /usr/sbin/kvm-ok when it is not in PATH and kvm-ok in PATH?

Thanks for quick reply!

[ipspace]

I've tried and unfortunately it doesn't work. I've even tried || true - obviously there is no shell interpretation for these commands...

The craziest thing is that it works for me :(

Anyway, what if we fix the documentation instead (because most people use Ubuntu anyway):

• Document that kvm-ok has to be in /usr/sbin
• Document how to disable the check
• Optionally, add a pointer to the documentation to the error message

[hedrok]

The craziest thing is that it works for me :(

Exteremely interesting how :)
If you have time to investigate, please write.

[I've updated PR, rest of this comment is outdated, see next one]
I've just ran Ubuntu in VM and:

vagrant@ubuntu2204:~$ echo $PATH
/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin

So as "most people use Ubuntu anyway" we can merge this PR without changes?.. :)

For "most people" it will work as "/usr/sbin" is in PATH.
For some of not "most people" like me it will start working.
For those of not "most people" who have kvm-ok in /usr/sbin and /usr/sbin not in PATH, it would be unpleasent of course, but it is easy to fix situation: add /usr/sbin to PATH or add link to kvm-ok somewhere in PATH. Plus do we know whether such people really exist?..

[hedrok]

I've updated PR with simple sh -c "which kvm-ok || stat /usr/sbin/kvm-ok" - tested that it works in both scenarios:

1. When there is /usr/sbin/kvm-ok, but usr/sbin is not in PATH.
2. When there is kvm-ok in PATH.

I think/hope that relying on presence of sh is ok.