[PATCH 1/5] util: fix tap device name auto-generation for FreeBSD

Laine Stump posted 5 patches 5 years, 1 month ago
[PATCH 1/5] util: fix tap device name auto-generation for FreeBSD
Posted by Laine Stump 5 years, 1 month ago
The Linux implementation of virNetDevCreate() no longer requires a
template ifname (e.g. "vnet%d") when it is called, but just generates
a new name if ifname is empty. The FreeBSD implementation requires
that the caller actually fill in a template ifname, and will fail if
ifname is empty. Since we want to eliminate all the special code in
callers that is setting the template name, we need to make the
behavior of the FreeBSD virNetDevCreate() match the behavior of the
Linux virNetDevCreate().

The simplest way to do this is to use the new virNetDevGenerateName()
function - if ifname is empty it generates a new name with the proper
prefix, and if it's not empty, it leaves it alone.

Signed-off-by: Laine Stump <laine@redhat.com>
---
 src/util/virnetdevtap.c | 35 ++++++-----------------------------
 1 file changed, 6 insertions(+), 29 deletions(-)

diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index 88ad321627..cca2f614fe 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -308,7 +308,6 @@ int virNetDevTapCreate(char **ifname,
     int s;
     struct ifreq ifr;
     int ret = -1;
-    char *newifname = NULL;
 
     if (tapfdSize > 1) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -316,6 +315,12 @@ int virNetDevTapCreate(char **ifname,
         goto cleanup;
     }
 
+    /* auto-generate an unused name for the new device (this
+     * is NOP if a name has been provided)
+     */
+    if (virNetDevGenerateName(ifname, VIR_NET_DEV_GEN_NAME_VNET) < 0)
+        return -1;
+    
     /* As FreeBSD determines interface type by name,
      * we have to create 'tap' interface first and
      * then rename it to 'vnet'
@@ -329,34 +334,6 @@ int virNetDevTapCreate(char **ifname,
         goto cleanup;
     }
 
-    /* In case we were given exact interface name (e.g. 'vnetN'),
-     * we just rename to it. If we have format string like
-     * 'vnet%d', we need to find the first available name that
-     * matches this pattern
-     */
-    if (strstr(*ifname, "%d") != NULL) {
-        size_t i;
-        for (i = 0; i <= IF_MAXUNIT; i++) {
-            g_autofree char *newname = NULL;
-
-            newname = g_strdup_printf(*ifname, i);
-
-            if (virNetDevExists(newname) == 0) {
-                newifname = g_steal_pointer(&newname);
-                break;
-            }
-        }
-        if (newifname) {
-            VIR_FREE(*ifname);
-            *ifname = newifname;
-        } else {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("Failed to generate new name for interface %s"),
-                           ifr.ifr_name);
-            goto cleanup;
-        }
-    }
-
     if (tapfd) {
         g_autofree char *dev_path = NULL;
         dev_path = g_strdup_printf("/dev/%s", ifr.ifr_name);
-- 
2.28.0

Re: [PATCH 1/5] util: fix tap device name auto-generation for FreeBSD
Posted by Michal Privoznik 5 years, 1 month ago
On 12/16/20 2:27 AM, Laine Stump wrote:
> The Linux implementation of virNetDevCreate() no longer requires a
> template ifname (e.g. "vnet%d") when it is called, but just generates
> a new name if ifname is empty. The FreeBSD implementation requires
> that the caller actually fill in a template ifname, and will fail if
> ifname is empty. Since we want to eliminate all the special code in
> callers that is setting the template name, we need to make the
> behavior of the FreeBSD virNetDevCreate() match the behavior of the
> Linux virNetDevCreate().
> 
> The simplest way to do this is to use the new virNetDevGenerateName()
> function - if ifname is empty it generates a new name with the proper
> prefix, and if it's not empty, it leaves it alone.
> 
> Signed-off-by: Laine Stump <laine@redhat.com>
> ---
>   src/util/virnetdevtap.c | 35 ++++++-----------------------------
>   1 file changed, 6 insertions(+), 29 deletions(-)
> 
> diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
> index 88ad321627..cca2f614fe 100644
> --- a/src/util/virnetdevtap.c
> +++ b/src/util/virnetdevtap.c
> @@ -308,7 +308,6 @@ int virNetDevTapCreate(char **ifname,
>       int s;
>       struct ifreq ifr;
>       int ret = -1;
> -    char *newifname = NULL;
>   
>       if (tapfdSize > 1) {
>           virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> @@ -316,6 +315,12 @@ int virNetDevTapCreate(char **ifname,
>           goto cleanup;
>       }
>   
> +    /* auto-generate an unused name for the new device (this
> +     * is NOP if a name has been provided)
> +     */
> +    if (virNetDevGenerateName(ifname, VIR_NET_DEV_GEN_NAME_VNET) < 0)
> +        return -1;
> +

This line ^^ contains some trailing spaces.

>       /* As FreeBSD determines interface type by name,
>        * we have to create 'tap' interface first and
>        * then rename it to 'vnet'

Michal