[PATCH 4/5] net/tap: refactor and improve net_init_tap_one() - Vladimir Sementsov-Ogievskiy

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
To: qemu-devel@nongnu.org
Cc: den@openvz.org, jasowang@redhat.com, vsementsov@virtuozzo.com
Subject: [PATCH 4/5] net/tap: refactor and improve net_init_tap_one()
Date: Mon, 21 Dec 2020 22:06:08 +0300	[thread overview]
Message-ID: <20201221190609.93768-5-vsementsov@virtuozzo.com> (raw)
In-Reply-To: <20201221190609.93768-1-vsementsov@virtuozzo.com>

First, 46d4d36d0bf2b24 changed the logic of net_init_tap_one() so
that failure of vhost initialization is ignored (still, warning
printed) when vhostforce is not set.

Then 894022e616016fe8 updated net_init_tap_one() to use
qemu_try_set_nonblock() instead of qemu_set_nonblock(), to not crash,
but it didn't consider the failure-ignoring logic and used wrong fd for
error message.

Let's refactor the function net_init_tap_one() splitting out vhost-net
initialization and fixing all the issues.

Error message change: we need to fix fd anyway. Also, don't want to
pass name to the new function only for error path, so, drop it. As a
pay, add more information on vhost fd itself.

Fixes: 894022e616016fe81745753f14adfbd680a1c7ee
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/net/vhost_net.h |   3 --
 net/tap.c               | 103 +++++++++++++++++++++-------------------
 2 files changed, 53 insertions(+), 53 deletions(-)

diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
index 172b0051d8..c00871cd1b 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -4,9 +4,6 @@
 #include "net/net.h"
 #include "hw/virtio/vhost-backend.h"
 
-#define VHOST_NET_INIT_FAILED \
-    "vhost-net requested but could not be initialized"
-
 struct vhost_net;
 typedef struct vhost_net VHostNetState;
 
diff --git a/net/tap.c b/net/tap.c
index 33d749c7b6..89ea04862b 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -652,15 +652,61 @@ static int net_tap_init(const NetdevTapOptions *tap, int *vnet_hdr,
 
 #define MAX_TAP_QUEUES 1024
 
+static int tap_init_vhost_net(TAPState *s, const NetdevTapOptions *tap,
+                              const char *vhostfdname, Error **errp)
+{
+    VhostNetOptions options;
+    int vhostfd;
+    int ret;
+
+    options.backend_type = VHOST_BACKEND_TYPE_KERNEL;
+    options.net_backend = &s->nc;
+    if (tap->has_poll_us) {
+        options.busyloop_timeout = tap->poll_us;
+    } else {
+        options.busyloop_timeout = 0;
+    }
+
+    if (vhostfdname) {
+        vhostfd = monitor_fd_param(monitor_cur(), vhostfdname, errp);
+        if (vhostfd < 0) {
+            return -1;
+        }
+
+        ret = qemu_try_set_nonblock(vhostfd);
+        if (ret < 0) {
+            error_setg_errno(errp, -ret,
+                             "tap: Can't use vhost file descriptor %s(%d)",
+                             vhostfdname, vhostfd);
+            return -1;
+        }
+    } else {
+        vhostfd = open("/dev/vhost-net", O_RDWR);
+        if (vhostfd < 0) {
+            error_setg_errno(errp, errno, "tap: open vhost char device failed");
+            return -1;
+        }
+        qemu_set_nonblock(vhostfd);
+    }
+
+    options.opaque = (void *)(uintptr_t)vhostfd;
+
+    s->vhost_net = vhost_net_init(&options);
+    if (!s->vhost_net) {
+        error_setg(errp, "vhost-net requested but could not be initialized");
+        return -1;
+    }
+
+    return 0;
+}
+
 static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
                              const char *model, const char *name,
                              const char *ifname, const char *script,
                              const char *downscript, const char *vhostfdname,
                              int vnet_hdr, int fd, Error **errp)
 {
-    Error *err = NULL;
     TAPState *s = net_tap_fd_init(peer, model, name, fd, vnet_hdr);
-    int vhostfd;
     int ret;
 
     ret = tap_set_sndbuf(s->fd, tap, errp);
@@ -687,58 +733,15 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
 
     if (tap->has_vhost ? tap->vhost :
         vhostfdname || (tap->has_vhostforce && tap->vhostforce)) {
-        VhostNetOptions options;
-
-        options.backend_type = VHOST_BACKEND_TYPE_KERNEL;
-        options.net_backend = &s->nc;
-        if (tap->has_poll_us) {
-            options.busyloop_timeout = tap->poll_us;
-        } else {
-            options.busyloop_timeout = 0;
-        }
-
-        if (vhostfdname) {
-            int ret;
-
-            vhostfd = monitor_fd_param(monitor_cur(), vhostfdname, &err);
-            if (vhostfd == -1) {
-                if (tap->has_vhostforce && tap->vhostforce) {
-                    error_propagate(errp, err);
-                } else {
-                    warn_report_err(err);
-                }
-                return;
-            }
-            ret = qemu_try_set_nonblock(vhostfd);
-            if (ret < 0) {
-                error_setg_errno(errp, -ret, "%s: Can't use file descriptor %d",
-                                 name, fd);
-                return;
-            }
-        } else {
-            vhostfd = open("/dev/vhost-net", O_RDWR);
-            if (vhostfd < 0) {
-                if (tap->has_vhostforce && tap->vhostforce) {
-                    error_setg_errno(errp, errno,
-                                     "tap: open vhost char device failed");
-                } else {
-                    warn_report("tap: open vhost char device failed: %s",
-                                strerror(errno));
-                }
-                return;
-            }
-            qemu_set_nonblock(vhostfd);
-        }
-        options.opaque = (void *)(uintptr_t)vhostfd;
+        Error *err = NULL;
 
-        s->vhost_net = vhost_net_init(&options);
-        if (!s->vhost_net) {
+        ret = tap_init_vhost_net(s, tap, vhostfdname, &err);
+        if (ret < 0) {
             if (tap->has_vhostforce && tap->vhostforce) {
-                error_setg(errp, VHOST_NET_INIT_FAILED);
+                error_propagate(errp, err);
             } else {
-                warn_report(VHOST_NET_INIT_FAILED);
+                warn_report_err(err);
             }
-            return;
         }
     } else if (vhostfdname) {
         error_setg(errp, "vhostfd(s)= is not valid without vhost");
-- 
2.28.0


