[Qemu-devel] [PATCH v3] tap: close fd conditionally when error occured

[Jay Zhou]

If netdev_add tap,id=net0,...,vhost=on failed in net_init_tap_one(),
the followed up device_add virtio-net-pci,netdev=net0 will fail
too, prints:

   TUNSETOFFLOAD ioctl() failed: Bad file descriptor TUNSETOFFLOAD
   ioctl() failed: Bad file descriptor

The reason is that the fd of tap is closed when error occured after
calling net_init_tap_one().

The fd should be closed when calling net_init_tap_one failed:
   - if tap_set_sndbuf() failed
   - if tap_set_sndbuf() succeeded but vhost failed to open or
     initialize with vhostforce flag on
The fd should not be closed just because vhost failed to open or
initialize but without vhostforce flag. So the followed up
device_add can fall back to userspace virtio successfully.

Suggested-by: Michael S. Tsirkin <address@hidden>
Suggested-by: Igor Mammedov <address@hidden>
Suggested-by: Jason Wang <address@hidden>
Signed-off-by: Jay Zhou <address@hidden>
---
 net/tap.c | 44 ++++++++++++++++++++++++++++++++------------
 1 file changed, 32 insertions(+), 12 deletions(-)

diff --git a/net/tap.c b/net/tap.c
index 979e622..c46ab14 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -651,7 +651,7 @@ static void net_init_tap_one(const NetdevTapOptions *tap, 
NetClientState *peer,
     tap_set_sndbuf(s->fd, tap, &err);
     if (err) {
         error_propagate(errp, err);
-        return;
+        goto error;
     }
 
     if (tap->has_fd || tap->has_fds) {
@@ -686,15 +686,26 @@ static void net_init_tap_one(const NetdevTapOptions *tap, 
NetClientState *peer,
         if (vhostfdname) {
             vhostfd = monitor_fd_param(cur_mon, vhostfdname, &err);
             if (vhostfd == -1) {
-                error_propagate(errp, err);
-                return;
+                if (tap->has_vhostforce && tap->vhostforce) {
+                    error_propagate(errp, err);
+                    goto error;
+                } else {
+                    warn_report_err(err);
+                    return;
+                }
             }
         } else {
             vhostfd = open("/dev/vhost-net", O_RDWR);
             if (vhostfd < 0) {
-                error_setg_errno(errp, errno,
-                                 "tap: open vhost char device failed");
-                return;
+                if (tap->has_vhostforce && tap->vhostforce) {
+                    error_setg_errno(errp, errno,
+                                     "tap: open vhost char device failed");
+                    goto error;
+                } else {
+                    warn_report("tap: open vhost char device failed: %s",
+                                strerror(errno));
+                    return;
+                }
             }
             fcntl(vhostfd, F_SETFL, O_NONBLOCK);
         }
@@ -702,12 +713,23 @@ static void net_init_tap_one(const NetdevTapOptions *tap, 
NetClientState *peer,
 
         s->vhost_net = vhost_net_init(&options);
         if (!s->vhost_net) {
-            error_setg(errp,
-                       "vhost-net requested but could not be initialized");
-            return;
+            if (tap->has_vhostforce && tap->vhostforce) {
+                error_setg(errp,
+                           "vhost-net requested but could not be initialized");
+                goto error;
+            } else {
+                warn_report("vhost-net requested but could not be 
initialized");
+                return;
+            }
         }
     } else if (vhostfdname) {
-        error_setg(errp, "vhostfd(s)= is not valid without vhost");
+        warn_report("vhostfd(s)= is not valid without vhost");
+        return;
+    }
+
+error:
+    if (!tap->has_fd && !tap->has_fds) {
+        close(fd);
     }
 }
 
@@ -877,7 +899,6 @@ free_fail:
                          vnet_hdr, fd, &err);
         if (err) {
             error_propagate(errp, err);
-            close(fd);
             return -1;
         }
     } else {
@@ -916,7 +937,6 @@ free_fail:
                              vhostfdname, vnet_hdr, fd, &err);
             if (err) {
                 error_propagate(errp, err);
-                close(fd);
                 return -1;
             }
         }
-- 
1.8.3.1