Lack of this one function (which is called for each active tap device
every time libvirtd is started) is the one thing preventing a
"WITHOUT_LIBNL" build of libvirt from being useful. With this
alternate implementation, guests using standard tap devices will work
properly.

Signed-off-by: Laine Stump <laine@redhat.com>
---
 src/util/virnetdev.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 5221bada7b..c43823c747 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -915,9 +915,30 @@ virNetDevGetMaster(const char *ifname, char **master)
     return 0;
 }
 
+#elif defined(__linux__)
 
-#else
+/* libnl isn't available, so we can't use netlink.
+ * Fall back to using sysfs
+ */
+int
+virNetDevGetMaster(const char *ifname, char **master)
+{
+    g_autofree char *path = NULL;
+    g_autofree char *canonical = NULL;
+
+    if (virNetDevSysfsFile(&path, ifname, "master") < 0)
+        return -1;
 
+    if (!(canonical = virFileCanonicalizePath(path)))
+        return -1;
+
+    *master = g_path_get_basename(canonical);
+
+    VIR_DEBUG("IFLA_MASTER for %s is %s", ifname, *master ? *master : "(none)");
+    return 0;
+}
+
+#else
 
 int
 virNetDevGetMaster(const char *ifname G_GNUC_UNUSED,
-- 
2.26.2

On 10/1/20 1:14 AM, Laine Stump wrote:
> Lack of this one function (which is called for each active tap device
> every time libvirtd is started) is the one thing preventing a
> "WITHOUT_LIBNL" build of libvirt from being useful. With this
> alternate implementation, guests using standard tap devices will work
> properly.
> 
> Signed-off-by: Laine Stump <laine@redhat.com>
> ---
>   src/util/virnetdev.c | 23 ++++++++++++++++++++++-
>   1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
> index 5221bada7b..c43823c747 100644
> --- a/src/util/virnetdev.c
> +++ b/src/util/virnetdev.c
> @@ -915,9 +915,30 @@ virNetDevGetMaster(const char *ifname, char **master)
>       return 0;
>   }
>   
> +#elif defined(__linux__)
>   
> -#else
> +/* libnl isn't available, so we can't use netlink.
> + * Fall back to using sysfs
> + */
> +int
> +virNetDevGetMaster(const char *ifname, char **master)
> +{
> +    g_autofree char *path = NULL;
> +    g_autofree char *canonical = NULL;
> +
> +    if (virNetDevSysfsFile(&path, ifname, "master") < 0)
> +        return -1;
>   
> +    if (!(canonical = virFileCanonicalizePath(path)))
> +        return -1;
> +
> +    *master = g_path_get_basename(canonical);
> +
> +    VIR_DEBUG("IFLA_MASTER for %s is %s", ifname, *master ? *master : "(none)");
> +    return 0;
> +}
> +
> +#else
>   
>   int
>   virNetDevGetMaster(const char *ifname G_GNUC_UNUSED,
> 

Whoa, building without LIBNL should fail, n'est-ce pas? I'm not saying 
your patch is wrong, it's just that I'm surprised we haven't caught this 
earlier.

Michal

On 10/1/20 4:06 AM, Michal Prívozník wrote:
> On 10/1/20 1:14 AM, Laine Stump wrote:
>> Lack of this one function (which is called for each active tap device
>> every time libvirtd is started) is the one thing preventing a
>> "WITHOUT_LIBNL" build of libvirt from being useful. With this
>> alternate implementation, guests using standard tap devices will work
>> properly.
>>
>> Signed-off-by: Laine Stump <laine@redhat.com>
>> ---
>>   src/util/virnetdev.c | 23 ++++++++++++++++++++++-
>>   1 file changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
>> index 5221bada7b..c43823c747 100644
>> --- a/src/util/virnetdev.c
>> +++ b/src/util/virnetdev.c
>> @@ -915,9 +915,30 @@ virNetDevGetMaster(const char *ifname, char 
>> **master)
>>       return 0;
>>   }
>> +#elif defined(__linux__)
>> -#else
>> +/* libnl isn't available, so we can't use netlink.
>> + * Fall back to using sysfs
>> + */
>> +int
>> +virNetDevGetMaster(const char *ifname, char **master)
>> +{
>> +    g_autofree char *path = NULL;
>> +    g_autofree char *canonical = NULL;
>> +
>> +    if (virNetDevSysfsFile(&path, ifname, "master") < 0)
>> +        return -1;
>> +    if (!(canonical = virFileCanonicalizePath(path)))
>> +        return -1;
>> +
>> +    *master = g_path_get_basename(canonical);
>> +
>> +    VIR_DEBUG("IFLA_MASTER for %s is %s", ifname, *master ? *master : 
>> "(none)");
>> +    return 0;
>> +}
>> +
>> +#else
>>   int
>>   virNetDevGetMaster(const char *ifname G_GNUC_UNUSED,
>>
> 
> Whoa, building without LIBNL should fail, n'est-ce pas? I'm not saying 
> your patch is wrong, it's just that I'm surprised we haven't caught this 
> earlier.

Yes, at two different levels:

1) A standard configure would try to enable "macvtap" and "virtualport" 
by default, and the checks for both of those would generate an error 
(saying "libnl is required for this feature") if libnl3-devel wasn't 
installed.

2) since commit 8708ca01 in 2017, and ending with the patch just prior 
to this one, if you removed libnl3-devel from your system, and then had 
--without-macvtap --without-virtualport (or whatever the flags were on 
the configure commandline - I've forgotten) then make would fail with a 
compile error in virnetdev.c.

But nobody ever does either of those (because "Why would you?"). I guess 
I should add the above details to the commit logs, now that I've taken 
the time to look it up and try it :-)

Earlier patches in this series eliminate the virtualport and macvtap 
flags, and patch 7 fixes the compile error, so finally a build without 
libnl can succeed. It was only after I did that, and tried installing a 
build w/o libnl and firing it up that I saw the errors (well, I had 
suspected that's what would happen after a manual inspection of places 
we were using netlink, but installing and running it verified the 
suspicion).