This flag was originally created to indicate that either 1) the build
platform wasn't linux, 2) the build platform was linux, but the kernel
was too old to have macvtap support; since there was already a switch
there, the ability to also disable it in case 3) the kernel supported
macvtap but the user didn't want it, was added in. I don't think that
(3) was ever an intentional goal, just something that grew naturally
out of having the flag there in the first place (unless possibly the
original author wanted a way to quickly disable their new code in case
it caused regressions elsewhere).

Now that the check for (2) has been removed, WITH_MACVTAP is just
checking (1) and (3), but (3) is pointless (since it adds almost
nothing extra in size to the code). We can therfore eliminate
the WITH_MACVTAP flag, as it is equivalent to __linux__.

*However*, macvtap/macvlan devices are created using netlink messages,
 and any netlink interaction in libvirt requires libnl. So what we
 *really* need is to check WITH_LIBNL (which itself implies __linux__,
 as libnl is only useful/available on Linux).

Signed-off-by: Laine Stump <laine@redhat.com>
---
 libvirt.spec.in             |  1 -
 meson.build                 | 10 ----------
 src/util/virnetdevmacvlan.c |  6 +++---
 3 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index c4a7c30737..aa2bc84be9 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1167,7 +1167,6 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y' %{_specdir}/%{name}.spec)
            -Dyajl=enabled \
            %{?arg_sanlock} \
            -Dlibpcap=enabled \
-           -Dmacvtap=enabled \
            -Daudit=enabled \
            -Ddtrace=enabled \
            %{?arg_firewalld} \
diff --git a/meson.build b/meson.build
index fe08a45b46..a6b6f2d2ee 100644
--- a/meson.build
+++ b/meson.build
@@ -1159,16 +1159,6 @@ libxml_dep = dependency('libxml-2.0', version: '>=' + libxml_version)
 cc = meson.get_compiler('c')
 m_dep = cc.find_library('m', required : false)
 
-if host_machine.system() == 'linux'
-  if not get_option('macvtap').disabled()
-    conf.set('WITH_MACVTAP', 1)
-  endif
-else
-  if get_option('macvtap').enabled()
-    error('macvtap is not supported on this platform.')
-  endif
-endif
-
 netcf_version = '0.1.8'
 netcf_dep = dependency('netcf', version: '>=' + netcf_version, required: get_option('netcf'))
 if netcf_dep.found()
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index 889cbb2293..25eb53c5da 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -40,7 +40,7 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,
               "passthrough",
 );
 
-#if WITH_MACVTAP
+#if defined(WITH_LIBNL)
 # include <fcntl.h>
 
 # include <net/if.h>
@@ -1051,7 +1051,7 @@ int virNetDevMacVLanRestartWithVPortProfile(const char *cr_ifname,
 
 }
 
-#else /* ! WITH_MACVTAP */
+#else /* ! WITH_LIBNL */
 bool virNetDevMacVLanIsMacvtap(const char *ifname G_GNUC_UNUSED)
 {
     virReportSystemError(ENOSYS, "%s",
@@ -1157,4 +1157,4 @@ void virNetDevMacVLanReserveName(const char *name G_GNUC_UNUSED)
     virReportSystemError(ENOSYS, "%s",
                          _("Cannot create macvlan devices on this platform"));
 }
-#endif /* ! WITH_MACVTAP */
+#endif /* ! WITH_LIBNL */
-- 
2.26.2

On Wed, Sep 30, 2020 at 07:14:41PM -0400, Laine Stump wrote:
> This flag was originally created to indicate that either 1) the build
> platform wasn't linux, 2) the build platform was linux, but the kernel
> was too old to have macvtap support; since there was already a switch
> there, the ability to also disable it in case 3) the kernel supported
> macvtap but the user didn't want it, was added in. I don't think that
> (3) was ever an intentional goal, just something that grew naturally
> out of having the flag there in the first place (unless possibly the
> original author wanted a way to quickly disable their new code in case
> it caused regressions elsewhere).
> 
> Now that the check for (2) has been removed, WITH_MACVTAP is just
> checking (1) and (3), but (3) is pointless (since it adds almost
> nothing extra in size to the code). We can therfore eliminate
> the WITH_MACVTAP flag, as it is equivalent to __linux__.
> 
> *However*, macvtap/macvlan devices are created using netlink messages,
>  and any netlink interaction in libvirt requires libnl. So what we
>  *really* need is to check WITH_LIBNL (which itself implies __linux__,
>  as libnl is only useful/available on Linux).
> 
> Signed-off-by: Laine Stump <laine@redhat.com>
> ---
>  libvirt.spec.in             |  1 -
>  meson.build                 | 10 ----------
>  src/util/virnetdevmacvlan.c |  6 +++---
>  3 files changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index c4a7c30737..aa2bc84be9 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -1167,7 +1167,6 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y' %{_specdir}/%{name}.spec)
>             -Dyajl=enabled \
>             %{?arg_sanlock} \
>             -Dlibpcap=enabled \
> -           -Dmacvtap=enabled \
>             -Daudit=enabled \
>             -Ddtrace=enabled \
>             %{?arg_firewalld} \
> diff --git a/meson.build b/meson.build
> index fe08a45b46..a6b6f2d2ee 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1159,16 +1159,6 @@ libxml_dep = dependency('libxml-2.0', version: '>=' + libxml_version)
>  cc = meson.get_compiler('c')
>  m_dep = cc.find_library('m', required : false)
>  
> -if host_machine.system() == 'linux'
> -  if not get_option('macvtap').disabled()
> -    conf.set('WITH_MACVTAP', 1)
> -  endif
> -else
> -  if get_option('macvtap').enabled()
> -    error('macvtap is not supported on this platform.')
> -  endif
> -endif

Missing change to meson_options.txt to remove 'macvtap' as well.

Pavel

On a Wednesday in 2020, Laine Stump wrote:
>This flag was originally created to indicate that either 1) the build
>platform wasn't linux, 2) the build platform was linux, but the kernel
>was too old to have macvtap support; since there was already a switch
>there, the ability to also disable it in case 3) the kernel supported
>macvtap but the user didn't want it, was added in. I don't think that
>(3) was ever an intentional goal, just something that grew naturally
>out of having the flag there in the first place (unless possibly the
>original author wanted a way to quickly disable their new code in case
>it caused regressions elsewhere).
>
>Now that the check for (2) has been removed, WITH_MACVTAP is just
>checking (1) and (3), but (3) is pointless (since it adds almost
>nothing extra in size to the code). We can therfore eliminate
>the WITH_MACVTAP flag, as it is equivalent to __linux__.
>
>*However*, macvtap/macvlan devices are created using netlink messages,
> and any netlink interaction in libvirt requires libnl. So what we
> *really* need is to check WITH_LIBNL (which itself implies __linux__,
> as libnl is only useful/available on Linux).
>

The indentation is off here.

Jano

>Signed-off-by: Laine Stump <laine@redhat.com>
>---
> libvirt.spec.in             |  1 -
> meson.build                 | 10 ----------
> src/util/virnetdevmacvlan.c |  6 +++---
> 3 files changed, 3 insertions(+), 14 deletions(-)