cockpit-machines still carries libvirt host interface management dependencies

[hw-claudio]

Hello,

as we noticed when trying to use cockpit-machines in more minimal OS builds, the now very old cockpit commit (pre-split of cockpit-machines):

commit 3af2e19fb64bcfe7d4bd277e68c03de2817f85b8 Author: Simon Kobyda 42733240+skobyda@users.noreply.github.com Date: Thu Aug 1 13:14:50 2019 +0200

machines: Introduce interfaces into store (#12465)

Closes #12465

introduces functionality to get, add and update Host network interfaces using libvirt. Nowadays the trend seems to be to use the OS dedicated network tools to manage host networks, not libvirt.

Is this functionality really used in cockpit-machine ? Initial testing would suggest that this feature is not really needed, and forces an unneeded chain of dependencies for modular OS where modular libvirt is chosen.

Any thoughts about removing the call in common.js:458 : interfaceGetAll({ connectionName }),

?

Any aspects that I missed? Thanks!

[jfehlig]

It's my first look at the code, but AFAICT cockpit-machines doesn't do anything with the information it gathers from 'ListInterfaces', and subsequent 'GetXMLDesc' for each interface. It gets stored, but not updated or retrieved. Did I overlook the use of this information? Can someone describe how it is used?

IMO, cockpit-machines should remove the use of libvirt's virInterface* APIs. virt-manager long ago dropped their use. The libvirt community would like to deprecate them

https://listman.redhat.com/archives/libvir-list/2020-December/msg00183.html

The only interface backend common among all distros is the udev backend, which only implements read operations and likely returns inconsistent results across distros (and even distro versions).

At SUSE, we'd like to take advantage of libvirt's modularization efforts and provide a minimal installation that does not include the virtinterfaced daemon. Removing the use of these APIs in cockpit-machines, or making their use configurable, would make it more friendly with minimal (but sufficient) libvirt installations.

BTW, the virInterface APIs are all about managing network interface devices on the host. As Claudio mentioned, distro tools (NetworkManager, netplan, etc) are better suited for that. Does cockpit already support the creation and management of host eth, vlan, bond, bridge, etc devices? If so, that's where libvirt's virInterface APIs should be used. But again, I'd suggest dropping their use altogether since they provide no practical value.

[martinpitt]

Thanks @hw-claudio for pointing out! I remember that I already tried to remove it once, back then it was unsuccessful. PR #1779 just landed which removes the central bit (thanks @Lunarequest !). I'll work on a PR to clean it up some more.

[martinpitt]

See PR #1782 , that should get rid of everything.

[JanZerebecki]

I'm not 100% certain, but a libvirt without a udev driver might also not return devices that were not defined manually from the nodedev api. Can someone with a libvirt compiled without udev confirm? I don't have one handy. ( https://libvirt.org/hvsupport.html#virNodeDeviceDriver implies it is all provided by udev, but @jfehlig said that the SR-IOV support still works without udev, so perhaps the function call would still succeed, but without any system devices that were not manually defined in libvirt. )

I see various references to that outside of what https://github.com/cockpit-project/cockpit-machines/pull/1782 touches. Some of those might not be affected, but some look like they are relied on to enumerate all devices on a host.

[jfehlig]

I'm not 100% certain, but a libvirt without a udev driver might also not return devices that were not defined manually from the nodedev api.

There is no udev driver in libvirt.

Can someone with a libvirt compiled without udev confirm?

There is no proposal to build libvirt without udev support.

[JanZerebecki]

Sorry, no idea why I though that was also disabled in Micro 6.0, after checking again, yes the udev driver is enabled (meson argument -Dudev=enabled).

Forget what I said before.