Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cockpit-machines still carries libvirt host interface management dependencies #1777

Closed
hw-claudio opened this issue Aug 21, 2024 · 6 comments · Fixed by #1782
Closed

cockpit-machines still carries libvirt host interface management dependencies #1777

hw-claudio opened this issue Aug 21, 2024 · 6 comments · Fixed by #1782
Assignees

Comments

@hw-claudio
Copy link

hw-claudio commented Aug 21, 2024

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 [email protected]
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!

@hw-claudio hw-claudio changed the title cockpit-machine still carries libvirt host interface management dependencies cockpit-machines still carries libvirt host interface management dependencies Aug 21, 2024
@jfehlig
Copy link

jfehlig commented Aug 21, 2024

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
Copy link
Member

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 added a commit to martinpitt/cockpit-machines that referenced this issue Aug 26, 2024
Nothing uses this any more.

Fixes cockpit-project#1777
@martinpitt martinpitt self-assigned this Aug 26, 2024
martinpitt added a commit to martinpitt/cockpit-machines that referenced this issue Aug 26, 2024
Nothing uses this any more.

Fixes cockpit-project#1777
@martinpitt
Copy link
Member

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

martinpitt added a commit to martinpitt/cockpit-machines that referenced this issue Aug 26, 2024
Nothing uses this any more.

Fixes cockpit-project#1777
martinpitt added a commit to martinpitt/cockpit-machines that referenced this issue Aug 26, 2024
Nothing uses this any more.

`HostVmsList` already didn't expect an `interfaces` property any more,
so drop specifying it.

Fixes cockpit-project#1777
martinpitt added a commit to martinpitt/cockpit-machines that referenced this issue Aug 26, 2024
Nothing uses this any more.

`HostVmsList` already didn't expect an `interfaces` property any more,
so drop specifying it.

Fixes cockpit-project#1777
martinpitt added a commit to martinpitt/cockpit-machines that referenced this issue Aug 26, 2024
Nothing uses this any more.

`HostVmsList` already didn't expect an `interfaces` property any more,
so drop specifying it.

Fixes cockpit-project#1777
martinpitt added a commit to martinpitt/cockpit-machines that referenced this issue Aug 27, 2024
virtinterfaced is being deprecated [1], and at least some distros want
to get rid of the dependency.

Fixes cockpit-project#1777

[1] https://listman.redhat.com/archives/libvir-list/2020-December/msg00183.html
@JanZerebecki
Copy link

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 #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
Copy link

jfehlig commented Aug 30, 2024

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
Copy link

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.

Lunarequest pushed a commit to Lunarequest/cockpit-machines that referenced this issue Sep 23, 2024
virtinterfaced is being deprecated [1], and at least some distros want
to get rid of the dependency.

Fixes cockpit-project#1777

[1] https://listman.redhat.com/archives/libvir-list/2020-December/msg00183.html
martinpitt added a commit to martinpitt/cockpit-machines that referenced this issue Sep 27, 2024
virtinterfaced is being deprecated [1], and at least some distros want
to get rid of the dependency.

Replace it with reading `ip --json a` instead  to get all the interface
names.

Fixes cockpit-project#1777

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

Co-Authored-By: Martin Pitt <[email protected]>
martinpitt added a commit to martinpitt/cockpit-machines that referenced this issue Oct 1, 2024
virtinterfaced is being deprecated [1], and at least some distros want
to get rid of the dependency.

Replace it with reading `ip --json a` instead  to get all the interface
names.

Fixes cockpit-project#1777

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

Co-Authored-By: Martin Pitt <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants