[libvirt PATCH v2 0/2] stop using netcf for the interface driver backend
V1 here: https://www.redhat.com/archives/libvir-list/2021-January/msg00922.html A short version of the cover letter from V1: this is a followup to my proposal to stop using netcf for the interface driver backend in Fedora/RHEL/CentOS builds and use the udev backend instead (it turns out Ubuntu already disabled netcf in 2018). Changes in V2: * removed the patch that made the default netcf=disabled even when netcf-devel was found on the host. If someone has netcf-devel installed and still wants to build without netcf, then can add "-Dnetcf=disabled" on the meson commandline. * Made the specfile changes more intelligent: * instead of hardcoding -Dnetcf=disabled, we now have a variable %{with_netcf} that is set to 1 for current Fedora (< 34) and current RHEL (< 9) but will be set to 0 for future Fedora/RHEL. This way the behavior on current OS releases will remain the same even for future libvirt. * it is possible to for netcf support off even in current/older OS releases by adding "--without netcf" to the rpmbuild commandline. I think at this point I would be comfortable pushing these patches, unless someone has misgivings about it... Laine Stump (2): build: support explicitly disabling netcf rpm: disable netcf for the interface driver in rpm build on new targets libvirt.spec.in | 22 +++++++++++++++++----- meson.build | 10 ++++++---- 2 files changed, 23 insertions(+), 9 deletions(-) -- 2.29.2
placing "-Dnetcf=disabled" on the meson commandline was ignored, meaning that even with that option the build would get WITH_NETCF if the netcf-devel package was found - the only way to disable it was to uninstall netcf-devel. This patch adds the small bit of logic to check the netcf meson commandline option (in addition to whether netcf-devel is installed) before defining WITH_NETCF. Signed-off-by: Laine Stump <laine@redhat.com> --- meson.build | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/meson.build b/meson.build index d8a63baac9..bfdffb16d2 100644 --- a/meson.build +++ b/meson.build @@ -1155,8 +1155,10 @@ libm_dep = cc.find_library('m', required : false) netcf_version = '0.1.8' netcf_dep = dependency('netcf', version: '>=' + netcf_version, required: get_option('netcf')) -if netcf_dep.found() - conf.set('WITH_NETCF', 1) +if not get_option('netcf').disabled() + if netcf_dep.found() + conf.set('WITH_NETCF', 1) + endif endif have_gnu_gettext_tools = false @@ -1550,7 +1552,7 @@ elif get_option('driver_hyperv').enabled() error('openwsman is required for the Hyper-V driver') endif -if not get_option('driver_interface').disabled() and conf.has('WITH_LIBVIRTD') and (udev_dep.found() or netcf_dep.found()) +if not get_option('driver_interface').disabled() and conf.has('WITH_LIBVIRTD') and (udev_dep.found() or conf.has('WITH_NETCF')) conf.set('WITH_INTERFACE', 1) elif get_option('driver_interface').enabled() error('Requested the Interface driver without netcf or udev and libvirtd support') @@ -2355,7 +2357,7 @@ libs_summary = { 'libssh': libssh_dep.found(), 'libssh2': libssh2_dep.found(), 'libutil': libutil_dep.found(), - 'netcf': netcf_dep.found(), + 'netcf': conf.has('WITH_NETCF'), 'NLS': have_gnu_gettext_tools, 'numactl': numactl_dep.found(), 'openwsman': openwsman_dep.found(), -- 2.29.2
libvirt.spec currently adds a hardcoded -Dnetcf=enabled to the meson commandline, so just setting the default in the meson.build file won't have any effect for rpm builds - it will be overridden. This patch changes the meson commandline in the spec file from hardcoded -Dnetcf=enabled to %{arg_netcf}, which is itself set according to the value of %{with_netcf}; and *that* is normally set according to the distro release of the build target (1 for Fedora >= 34 and RHEL >= 9, 0 otherwise), but can be manually overridden by adding "-without netcf" to the rpmbuild commandline. Along with being used to determine what arg to pass to meson, %{with_netcf} is also checked when deciding on whether or not to add netcf build time / install time dependencies ("Requires: netcf-libs" and "BuildRequires: netcf-devel") Signed-off-by: Laine Stump <laine@redhat.com> --- libvirt.spec.in | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index b5892987cf..f7c350db41 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -101,6 +101,7 @@ %define with_sanlock 0 %define with_numad 0 %define with_firewalld_zone 0 +%define with_netcf 0 %define with_libssh2 0 %define with_wireshark 0 %define with_libssh 0 @@ -145,6 +146,10 @@ %define with_firewalld_zone 0%{!?_without_firewalld_zone:1} %endif +%if 0%{?fedora} < 34 || 0%{?rhel} < 9 + %define with_netcf 0%{!?_without_netcf:1} +%endif + # fuse is used to provide virtualized /proc for LXC %if %{with_lxc} @@ -358,8 +363,9 @@ BuildRequires: fuse-devel >= 2.8.6 %if %{with_libssh2} BuildRequires: libssh2-devel >= 1.3.0 %endif - +%if %{with_netcf} BuildRequires: netcf-devel >= 0.2.2 +%endif %if %{with_esx} BuildRequires: libcurl-devel %endif @@ -528,13 +534,13 @@ capabilities. Summary: Interface driver plugin for the libvirtd daemon Requires: libvirt-daemon = %{version}-%{release} Requires: libvirt-libs = %{version}-%{release} +%if %{with_netcf} Requires: netcf-libs >= 0.2.2 +%endif %description daemon-driver-interface The interface driver plugin for the libvirtd daemon, providing -an implementation of the network interface APIs using the -netcf library - +an implementation of the network interface APIs using udev. %package daemon-driver-secret Summary: Secret driver plugin for the libvirtd daemon @@ -1100,6 +1106,12 @@ exit 1 %define arg_firewalld_zone -Dfirewalld_zone=disabled %endif +%if %{with_netcf} + %define arg_netcf -Dnetcf=enabled +%else + %define arg_netcf -Dnetcf=disabled +%endif + %if %{with_wireshark} %define arg_wireshark -Dwireshark_dissector=enabled %else @@ -1170,7 +1182,7 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y' %{_specdir}/%{name}.spec) %{?arg_numad} \ -Dcapng=enabled \ %{?arg_fuse} \ - -Dnetcf=enabled \ + %{?arg_netcf} \ -Dselinux=enabled \ %{?arg_selinux_mount} \ -Dapparmor=disabled \ -- 2.29.2
On Sun, Jan 24, 2021 at 01:44:26AM -0500, Laine Stump wrote:
libvirt.spec currently adds a hardcoded -Dnetcf=enabled to the meson commandline, so just setting the default in the meson.build file won't have any effect for rpm builds - it will be overridden.
This patch changes the meson commandline in the spec file from hardcoded -Dnetcf=enabled to %{arg_netcf}, which is itself set according to the value of %{with_netcf}; and *that* is normally set according to the distro release of the build target (1 for Fedora >= 34 and RHEL >= 9, 0 otherwise), but can be manually overridden by adding "-without netcf" to the rpmbuild commandline.
Along with being used to determine what arg to pass to meson, %{with_netcf} is also checked when deciding on whether or not to add netcf build time / install time dependencies ("Requires: netcf-libs" and "BuildRequires: netcf-devel")
Signed-off-by: Laine Stump <laine@redhat.com> --- libvirt.spec.in | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in index b5892987cf..f7c350db41 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -101,6 +101,7 @@ %define with_sanlock 0 %define with_numad 0 %define with_firewalld_zone 0 +%define with_netcf 0 %define with_libssh2 0 %define with_wireshark 0 %define with_libssh 0 @@ -145,6 +146,10 @@ %define with_firewalld_zone 0%{!?_without_firewalld_zone:1} %endif
+%if 0%{?fedora} < 34 || 0%{?rhel} < 9 + %define with_netcf 0%{!?_without_netcf:1} +%endif +
# fuse is used to provide virtualized /proc for LXC %if %{with_lxc} @@ -358,8 +363,9 @@ BuildRequires: fuse-devel >= 2.8.6 %if %{with_libssh2} BuildRequires: libssh2-devel >= 1.3.0 %endif - +%if %{with_netcf} BuildRequires: netcf-devel >= 0.2.2 +%endif %if %{with_esx} BuildRequires: libcurl-devel %endif @@ -528,13 +534,13 @@ capabilities. Summary: Interface driver plugin for the libvirtd daemon Requires: libvirt-daemon = %{version}-%{release} Requires: libvirt-libs = %{version}-%{release} +%if %{with_netcf} Requires: netcf-libs >= 0.2.2 +%endif
%description daemon-driver-interface The interface driver plugin for the libvirtd daemon, providing -an implementation of the network interface APIs using the -netcf library - +an implementation of the network interface APIs using udev.
s/using udev// because it may still be netcf on some distro versions Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 1/29/21 12:06 PM, Daniel P. Berrangé wrote:
On Sun, Jan 24, 2021 at 01:44:26AM -0500, Laine Stump wrote:
libvirt.spec currently adds a hardcoded -Dnetcf=enabled to the meson commandline, so just setting the default in the meson.build file won't have any effect for rpm builds - it will be overridden.
This patch changes the meson commandline in the spec file from hardcoded -Dnetcf=enabled to %{arg_netcf}, which is itself set according to the value of %{with_netcf}; and *that* is normally set according to the distro release of the build target (1 for Fedora >= 34 and RHEL >= 9, 0 otherwise), but can be manually overridden by adding "-without netcf" to the rpmbuild commandline.
Along with being used to determine what arg to pass to meson, %{with_netcf} is also checked when deciding on whether or not to add netcf build time / install time dependencies ("Requires: netcf-libs" and "BuildRequires: netcf-devel")
Signed-off-by: Laine Stump <laine@redhat.com> --- libvirt.spec.in | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in index b5892987cf..f7c350db41 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -101,6 +101,7 @@ %define with_sanlock 0 %define with_numad 0 %define with_firewalld_zone 0 +%define with_netcf 0 %define with_libssh2 0 %define with_wireshark 0 %define with_libssh 0 @@ -145,6 +146,10 @@ %define with_firewalld_zone 0%{!?_without_firewalld_zone:1} %endif
+%if 0%{?fedora} < 34 || 0%{?rhel} < 9 + %define with_netcf 0%{!?_without_netcf:1} +%endif +
# fuse is used to provide virtualized /proc for LXC %if %{with_lxc} @@ -358,8 +363,9 @@ BuildRequires: fuse-devel >= 2.8.6 %if %{with_libssh2} BuildRequires: libssh2-devel >= 1.3.0 %endif - +%if %{with_netcf} BuildRequires: netcf-devel >= 0.2.2 +%endif %if %{with_esx} BuildRequires: libcurl-devel %endif @@ -528,13 +534,13 @@ capabilities. Summary: Interface driver plugin for the libvirtd daemon Requires: libvirt-daemon = %{version}-%{release} Requires: libvirt-libs = %{version}-%{release} +%if %{with_netcf} Requires: netcf-libs >= 0.2.2 +%endif
%description daemon-driver-interface The interface driver plugin for the libvirtd daemon, providing -an implementation of the network interface APIs using the -netcf library - +an implementation of the network interface APIs using udev.
s/using udev//
because it may still be netcf on some distro versions
Ah, right. I noticed that leftover of V1 after I posted V2, and meant to take it out, but forgot. I'll fix it before I push.
On Sun, Jan 24, 2021 at 1:45 AM Laine Stump <laine@redhat.com> wrote:
V1 here: https://www.redhat.com/archives/libvir-list/2021-January/msg00922.html
A short version of the cover letter from V1: this is a followup to my proposal to stop using netcf for the interface driver backend in Fedora/RHEL/CentOS builds and use the udev backend instead (it turns out Ubuntu already disabled netcf in 2018).
Changes in V2:
* removed the patch that made the default netcf=disabled even when netcf-devel was found on the host. If someone has netcf-devel installed and still wants to build without netcf, then can add "-Dnetcf=disabled" on the meson commandline.
* Made the specfile changes more intelligent:
* instead of hardcoding -Dnetcf=disabled, we now have a variable %{with_netcf} that is set to 1 for current Fedora (< 34) and current RHEL (< 9) but will be set to 0 for future Fedora/RHEL. This way the behavior on current OS releases will remain the same even for future libvirt.
* it is possible to for netcf support off even in current/older OS releases by adding "--without netcf" to the rpmbuild commandline.
I think at this point I would be comfortable pushing these patches, unless someone has misgivings about it...
Laine Stump (2): build: support explicitly disabling netcf rpm: disable netcf for the interface driver in rpm build on new targets
libvirt.spec.in | 22 +++++++++++++++++----- meson.build | 10 ++++++---- 2 files changed, 23 insertions(+), 9 deletions(-)
-- 2.29.2
This looks fine to me, but I'm wondering why libvirt doesn't communicate with NetworkManager for this information? That's a cross distribution method of handling complex network configuration that we basically know will exist and can handle parsing and configuring networks effectively. Otherwise... Reviewed-by: Neal Gompa <ngompa13@gmail.com> -- 真実はいつも一つ！/ Always, there's only one truth!
On 1/24/21 1:59 AM, Neal Gompa wrote:
On Sun, Jan 24, 2021 at 1:45 AM Laine Stump <laine@redhat.com> wrote:
V1 here: https://www.redhat.com/archives/libvir-list/2021-January/msg00922.html
A short version of the cover letter from V1: this is a followup to my proposal to stop using netcf for the interface driver backend in Fedora/RHEL/CentOS builds and use the udev backend instead (it turns out Ubuntu already disabled netcf in 2018).
Changes in V2:
* removed the patch that made the default netcf=disabled even when netcf-devel was found on the host. If someone has netcf-devel installed and still wants to build without netcf, then can add "-Dnetcf=disabled" on the meson commandline.
* Made the specfile changes more intelligent:
* instead of hardcoding -Dnetcf=disabled, we now have a variable %{with_netcf} that is set to 1 for current Fedora (< 34) and current RHEL (< 9) but will be set to 0 for future Fedora/RHEL. This way the behavior on current OS releases will remain the same even for future libvirt.
* it is possible to for netcf support off even in current/older OS releases by adding "--without netcf" to the rpmbuild commandline.
I think at this point I would be comfortable pushing these patches, unless someone has misgivings about it...
Laine Stump (2): build: support explicitly disabling netcf rpm: disable netcf for the interface driver in rpm build on new targets
libvirt.spec.in | 22 +++++++++++++++++----- meson.build | 10 ++++++---- 2 files changed, 23 insertions(+), 9 deletions(-)
-- 2.29.2
This looks fine to me, but I'm wondering why libvirt doesn't communicate with NetworkManager for this information? That's a cross distribution method of handling complex network configuration that we basically know will exist and can handle parsing and configuring networks effectively.
Wanna write a backend driver? Seriously though, we've talked about that in the past, but if it was done, it would be separate from netcf - an alternate backend like the udev backend. But in the end, I don't really think there's much demand for libvirt to configure host interfaces anyway, so it would be a lot of effort for no real gain. The only reason we ever added the virInterface API in the first place was that at the time (late 2008) there was *no* standard API for configuring a new bridge or bond device and attaching ethernets to them (unless you count "directly modify the files in /etc/sysconfig/network-scripts" as an API :-P), and that was a common need for someone provisioning a host to be a compute node for virtualization. Even NetworkManager (which at that time generally was only used on Desktop systems, almost never on servers) had exactly 0 support for bridge or bond devices (I won't claim to know how much of a programmatic API they had at the time, but it would have been a moot point, since they didn't support bridges and bonds at all) At the same time there was a virtualization management application (ovirt) which needed such an API because they needed to be able to provision a new compute node that started out with a bare ethernet (and they wanted a bridge or a bond attached to a bridge). Since they already had a connection into the node via libvirt, they saw that as an opportune place to add such an API. But within a couple years after that, NetworkManager started supporting bridge and bond devices, but still using ifcfg files just like initscripts (and libvirt/netcf) did. The only problem was that they made subtle changes in behavior (one I remember is they changed the necessary ordering of starting bridge devices and attached ethernet devices, and then later *again* changed behavior by automatically starting attached ethernets (or something like that - I just remember it was frustrating to deal with). But at the same time they made behavioral changes that repeatedly broke libvirt's attempts at configuring/managing host interfaces, they also produced a usable API. And the people who were the reason we added netcf/virInterface decided not to use virInterface, but instead to do their own thing (via vdsm, which is a part of ovirt). So we were left with a bunch of code that nobody used, and was constantly breaking due to behavior changes by the ever-more-popular NetworkManager. This of course did nothing to encourage me (or anyone else) to work on it - why toil away to get something to work properly if the fix for the current version of NM was just going to break it on previous versions (and probably prime it for breakage on future versions), especially if there were no users? Anyway, I've digressed quite a ways for no good reason (other than that it's the middle of the night and I'm sleepless again) and am probably becoming incoherent and repetitive, so I'll pull it back in. In the old days there was no good way to configure host interfaces via an API, so we made something. But now there *are* good ways to do that, so there's not much upside to just putting libvirt in as the middleman. If someone wants to try doing it, a NetworkManager backend to the interface driver might be interesting, but I'm not going to spend any time doing it unless someone in a position of authority over me says that I need to, and that hasn't happened :-P BTW, do you ever sleep???
Otherwise...
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Thanks. I'm going to let it sit and fester a few days to see if anyone else has anything to say about it, but I'll definitely remember to add your R-b when/if I push :-)
Unless there are any objections to it, I'm planning to push these two patches later today or tomorrow. (Dan - I've Cc'ed you since you were the only one to respond to V1) After that, I'm wondering if it would be okay to backport it into Fedora Rawhide so that it makes it into Fedora 34 (since it is still enabled by default unless you do an rpm build (as long as netcf-devel is installed, which will be the case on anyone's dev machine), the only way to get more exposure is in Fedora, and since it's so sparsely used (if at all) I don't think having it just in Rawhide is going to be of any practical use). Also, this is disabling netcf for Fedora > 33, and if we fail to actually get the change in before F34 we'll either have to change that, or deal with it changing for virt-preview and anyone's local builds... On 1/24/21 1:44 AM, Laine Stump wrote:
V1 here: https://www.redhat.com/archives/libvir-list/2021-January/msg00922.html
A short version of the cover letter from V1: this is a followup to my proposal to stop using netcf for the interface driver backend in Fedora/RHEL/CentOS builds and use the udev backend instead (it turns out Ubuntu already disabled netcf in 2018).
Changes in V2:
* removed the patch that made the default netcf=disabled even when netcf-devel was found on the host. If someone has netcf-devel installed and still wants to build without netcf, then can add "-Dnetcf=disabled" on the meson commandline.
* Made the specfile changes more intelligent:
* instead of hardcoding -Dnetcf=disabled, we now have a variable %{with_netcf} that is set to 1 for current Fedora (< 34) and current RHEL (< 9) but will be set to 0 for future Fedora/RHEL. This way the behavior on current OS releases will remain the same even for future libvirt.
* it is possible to for netcf support off even in current/older OS releases by adding "--without netcf" to the rpmbuild commandline.
I think at this point I would be comfortable pushing these patches, unless someone has misgivings about it...
Laine Stump (2): build: support explicitly disabling netcf rpm: disable netcf for the interface driver in rpm build on new targets
libvirt.spec.in | 22 +++++++++++++++++----- meson.build | 10 ++++++---- 2 files changed, 23 insertions(+), 9 deletions(-)
On 1/24/21 7:44 AM, Laine Stump wrote:
V1 here: https://www.redhat.com/archives/libvir-list/2021-January/msg00922.html
A short version of the cover letter from V1: this is a followup to my proposal to stop using netcf for the interface driver backend in Fedora/RHEL/CentOS builds and use the udev backend instead (it turns out Ubuntu already disabled netcf in 2018).
Changes in V2:
* removed the patch that made the default netcf=disabled even when netcf-devel was found on the host. If someone has netcf-devel installed and still wants to build without netcf, then can add "-Dnetcf=disabled" on the meson commandline.
* Made the specfile changes more intelligent:
* instead of hardcoding -Dnetcf=disabled, we now have a variable %{with_netcf} that is set to 1 for current Fedora (< 34) and current RHEL (< 9) but will be set to 0 for future Fedora/RHEL. This way the behavior on current OS releases will remain the same even for future libvirt.
* it is possible to for netcf support off even in current/older OS releases by adding "--without netcf" to the rpmbuild commandline.
I think at this point I would be comfortable pushing these patches, unless someone has misgivings about it...
Laine Stump (2): build: support explicitly disabling netcf rpm: disable netcf for the interface driver in rpm build on new targets
libvirt.spec.in | 22 +++++++++++++++++----- meson.build | 10 ++++++---- 2 files changed, 23 insertions(+), 9 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (4)
-
Daniel P. Berrangé -
Laine Stump -
Michal Privoznik -
Neal Gompa