qemu: support unmanaged target tap dev for <interface type='ethernet'>

If managed='no', then the tap device must already exist, and setting
of MAC address and online status (IFF_UP) is skipped.

NB: we still set IFF_VNET_HDR and IFF_MULTI_QUEUE as appropriate,
because those bits must be properly set in the TUNSETIFF we use to set
the tap device name of the handle we've opened - if IFF_VNET_HDR has
not been set and we set it the request will be honored even when
running libvirtd unprivileged; if IFF_MULTI_QUEUE is requested to be
different than how it was created, that will result in an error from
the kernel. This means that you don't need to pay attention to
IFF_VNET_HDR when creating the tap devices, but you *do* need to set
IFF_MULTI_QUEUE if you're going to use multiple queues for your tap
device.

NB2: /dev/vhost-net normally has permissions 600, so it can't be
opened by an unprivileged process. This would normally cause a warning
message when using a virtio net device from an unprivileged
libvirtd. I've found that setting the permissions for /dev/vhost-net
permits unprivileged libvirtd to use vhost-net for virtio devices, but
have no idea what sort of security implications that has. I haven't
changed libvrit's code to avoid *attempting* to open /dev/vhost-net -
if you are concerned about the security of opening up permissions of
/dev/vhost-net (probably a good idea at least until we ask someone who
knows about the code) then add <driver name='qemu'/> to the interface
definition and you'll avoid the warning message.

Note that virNetDevTapCreate() is the correct function to call in the
case of an existing device, because the same ioctl() that creates a
new tap device will also open an existing tap device.

Resolves: https://bugzilla.redhat.com/1723367 (partially)

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c
index ad1755dd6a4868e236da69f3683c9644ed92a75a..b82a62f7d327b22847406feabb4559b7b6851899 100644 (file)
--- a/src/qemu/qemu_interface.c
+++ b/src/qemu/qemu_interface.c
@@ -428,36 +428,53 @@ qemuInterfaceEthernetConnect(virDomainDefPtr def,
     if (virDomainNetIsVirtioModel(net))
         tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR;
 
-    if (!net->ifname ||
-        STRPREFIX(net->ifname, VIR_NET_GENERATED_TAP_PREFIX) ||
-        strchr(net->ifname, '%')) {
-        VIR_FREE(net->ifname);
-        if (VIR_STRDUP(net->ifname, VIR_NET_GENERATED_TAP_PREFIX "%d") < 0)
+    if (net->managed_tap == VIR_TRISTATE_BOOL_NO) {
+        if (!net->ifname) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("target dev must be supplied when managed='no'"));
             goto cleanup;
-        /* avoid exposing vnet%d in getXMLDesc or error outputs */
-        template_ifname = true;
-    }
-    if (virNetDevTapCreate(&net->ifname, tunpath, tapfd, tapfdSize,
-                           tap_create_flags) < 0) {
-        goto cleanup;
-    }
+        }
+        if (virNetDevExists(net->ifname) != 1) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("target managed='no' but specified dev doesn't exist"));
+            goto cleanup;
+        }
+        if (virNetDevTapCreate(&net->ifname, tunpath, tapfd, tapfdSize,
+                               tap_create_flags) < 0) {
+            goto cleanup;
+        }
+    } else {
+        if (!net->ifname ||
+            STRPREFIX(net->ifname, VIR_NET_GENERATED_TAP_PREFIX) ||
+            strchr(net->ifname, '%')) {
+            VIR_FREE(net->ifname);
+            if (VIR_STRDUP(net->ifname, VIR_NET_GENERATED_TAP_PREFIX "%d") < 0)
+                goto cleanup;
+            /* avoid exposing vnet%d in getXMLDesc or error outputs */
+            template_ifname = true;
+        }
+        if (virNetDevTapCreate(&net->ifname, tunpath, tapfd, tapfdSize,
+                               tap_create_flags) < 0) {
+            goto cleanup;
+        }
 
-    /* The tap device's MAC address cannot match the MAC address
-     * used by the guest. This results in "received packet on
-     * vnetX with own address as source address" error logs from
-     * the kernel.
-     */
-    virMacAddrSet(&tapmac, &net->mac);
-    if (tapmac.addr[0] == 0xFE)
-        tapmac.addr[0] = 0xFA;
-    else
-        tapmac.addr[0] = 0xFE;
+        /* The tap device's MAC address cannot match the MAC address
+         * used by the guest. This results in "received packet on
+         * vnetX with own address as source address" error logs from
+         * the kernel.
+         */
+        virMacAddrSet(&tapmac, &net->mac);
+        if (tapmac.addr[0] == 0xFE)
+            tapmac.addr[0] = 0xFA;
+        else
+            tapmac.addr[0] = 0xFE;
 
-    if (virNetDevSetMAC(net->ifname, &tapmac) < 0)
-        goto cleanup;
+        if (virNetDevSetMAC(net->ifname, &tapmac) < 0)
+            goto cleanup;
 
-    if (virNetDevSetOnline(net->ifname, true) < 0)
-        goto cleanup;
+        if (virNetDevSetOnline(net->ifname, true) < 0)
+            goto cleanup;
+    }
 
     if (net->script &&
         virNetDevRunEthernetScript(net->ifname, net->script) < 0)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 6a261786f976cd7f4690ca1cf7871a0037f5a9a5..955ba4de4ce2035669feb9c1688a241ef68b9152 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -7559,7 +7559,7 @@ void qemuProcessStop(virQEMUDriverPtr driver,
                              cfg->stateDir));
             break;
         case VIR_DOMAIN_NET_TYPE_ETHERNET:
-            if (net->ifname) {
+            if (net->managed_tap != VIR_TRISTATE_BOOL_NO && net->ifname) {
                 ignore_value(virNetDevTapDelete(net->ifname, net->backend.tap));
                 VIR_FREE(net->ifname);
             }

diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h
index 563b218227aeb72332fbe44b40fc40eb4589841e..6ff98487cb8cd50e3c299009a97d9430a574f7ad 100644 (file)
--- a/src/util/virnetdev.h
+++ b/src/util/virnetdev.h
@@ -150,7 +150,7 @@ int virNetDevSetupControl(const char *ifname,
     ATTRIBUTE_RETURN_CHECK;
 
 int virNetDevExists(const char *brname)
-    ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NOINLINE;
 
 int virNetDevSetOnline(const char *ifname,
                        bool online)

diff --git a/tests/qemuxml2argvdata/net-eth-unmanaged-tap.args b/tests/qemuxml2argvdata/net-eth-unmanaged-tap.args
new file mode 100644 (file)
index 0000000..2bb99e9
--- /dev/null
+++ b/tests/qemuxml2argvdata/net-eth-unmanaged-tap.args
@@ -0,0 +1,32 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-i686 \
+-name QEMUGuest1 \
+-S \
+-machine pc,accel=tcg,usb=off,dump-guest-core=off \
+-m 214 \
+-realtime mlock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-usb \
+-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
+-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
+-netdev tap,fd=3,id=hostnet0,vhost=on,vhostfd=44 \
+-device virtio-net-pci,netdev=hostnet0,id=net0,mac=fe:11:22:33:44:55,bus=pci.0,\
+addr=0x3

diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
index 3f0c1c3fef894a5fd0073540f3ce48cee852645a..a386dd17be6bfec907f65f4b4f1d36c62909ec9c 100644 (file)
--- a/tests/qemuxml2argvmock.c
+++ b/tests/qemuxml2argvmock.c
@@ -149,8 +149,12 @@ virNetDevTapCreate(char **ifname,
     for (i = 0; i < tapfdSize; i++)
         tapfd[i] = STDERR_FILENO + 1 + i;
 
-    VIR_FREE(*ifname);
-    return VIR_STRDUP(*ifname, "vnet0");
+    if (STREQ_NULLABLE(*ifname, "mytap0")) {
+        return 0;
+    } else {
+        VIR_FREE(*ifname);
+        return VIR_STRDUP(*ifname, "vnet0");
+    }
 }
 
 int
@@ -160,6 +164,14 @@ virNetDevSetMAC(const char *ifname ATTRIBUTE_UNUSED,
     return 0;
 }
 
+
+int
+virNetDevExists(const char *ifname)
+{
+    return STREQ_NULLABLE(ifname, "mytap0");
+}
+
+
 int virNetDevIPAddrAdd(const char *ifname ATTRIBUTE_UNUSED,
                        virSocketAddr *addr ATTRIBUTE_UNUSED,
                        virSocketAddr *peer ATTRIBUTE_UNUSED,

diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 5af74ef9be30e33142411af4505b9a620dbf6806..1f2ae5958a3355d01b58bc9991cceb1820bc0d45 100644 (file)
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1318,6 +1318,7 @@ mymain(void)
     DO_TEST("net-eth-ifname", NONE);
     DO_TEST("net-eth-names", NONE);
     DO_TEST("net-eth-hostip", NONE);
+    DO_TEST("net-eth-unmanaged-tap", NONE);
     DO_TEST("net-client", NONE);
     DO_TEST("net-server", NONE);
     DO_TEST("net-many-models", NONE);