conf: new "managed" attribute for target dev of <interface type='ethernet'>

Although <interface type='ethernet'> has always been able to use an existing tap device, this is just a coincidence due to the fact that the same ioctl is used to create a new tap device or get a handle to an existing device. Even then, once we have the handle to the device, we still insist on doing extra setup to it (setting the MAC address and IFF_UP). That *might* be okay if libvirtd is running as a privileged process, but if libvirtd is running as an unprivileged user, those attempted modifications to the tap device will fail (yes, even if the tap is set to be owned by the user running libvirtd). We could avoid this if we knew that the device already existed, but as stated above, an existing device and new device are both accessed in the same manner, and anyway, we need to preserve existing behavior for those who are already using pre-existing devices with privileged libvirtd (and allowing/expecting libvirt to configure the pre-existing device). In order to cleanly support the idea of using a pre-existing and pre-configured tap device, this patch introduces a new optional attribute "managed" for the interface <target> element. This attribute is only valid for <interface type='ethernet'> (since all other interface types have mandatory config that doesn't apply in the case where we expect the tap device to be setup before we get it). The syntax would look something like this: <interface type='ethernet'> <target dev='mytap0' managed='no'/> ... </interface> This patch just adds managed to the grammar and parser for <target>, but has no functionality behind it. (NB: when managed='no' (the default when not specified is 'yes'), the target dev is always a name explicitly provided, so we don't auto-remove it from the config just because it starts with "vnet" (VIR_NET_GENERATED_TAP_PREFIX); this makes it possible to use the same pattern of names that libvirt itself uses when it automatically creates the tap devices.) Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2024-12-22 13:45:38 +00:00 · 2019-08-21 16:42:41 -04:00 · 2019-08-21 16:42:41 -04:00 · 77f72a8615
commit 77f72a8615
parent 33d02dfca6
7 changed files with 159 additions and 20 deletions
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@ -5511,23 +5511,51 @@
    <h5><a id="elementsNICSEthernet">Generic ethernet connection</a></h5>

    <p>
-      Provides a means for the administrator to execute an arbitrary script
-      to connect the guest's network to the LAN. The guest will have a tun
-      device created with a name of vnetN, which can also be overridden with the
-      &lt;target&gt; element. After creating the tun device a shell script will
-      be run which is expected to do whatever host network integration is
-      required. By default this script is called /etc/qemu-ifup but can be
-      overridden.
+      Provides a means to use a new or existing tap device (or veth
+      device pair, depening on the needs of the hypervisor driver)
+      that is partially or wholly setup external to libvirt (either
+      prior to the guest starting, or while the guest is being started
+      via an optional script specified in the config).
+    </p>
+    <p>
+      The name of the tap device can optionally be specified with
+      the <code>dev</code> attribute of the
+      <code>&lt;target&gt;</code> element. If no target dev is
+      specified, libvirt will create a new standard tap device with a
+      name of the pattern "vnetN", where "N" is replaced with a
+      number. If a target dev is specified and that device doesn't
+      exist, then a new standard tap device will be created with the
+      exact dev name given. If the specified target dev does exist,
+      then that existing device will be used. Usually some basic setup
+      of the device is done by libvirt, including setting a MAC
+      address, and the IFF_UP flag, but if the <code>dev</code> is a
+      pre-existing device, and the <code>managed</code> attribute of
+      the <code>target</code> element is also set to "no" (the default
+      value is "yes"), even this basic setup will not be performed -
+      libvirt will simply pass the device on to the hypervisor with no
+      setup at all. <span class="since">Since 5.7.0</span> Using
+      managed='no' with a pre-created tap device is useful because
+      it permits a virtual machine managed by an unprivileged libvirtd
+      to have emulated network devices based on tap devices.
+    </p>
+    <p>
+      After creating/opening the tap device, an optional shell script
+      (given in the <code>path</code> attribute of
+      the <code>&lt;script&gt;</code> element) will be run; this can
+      be used to do whatever extra host network integration is
+      required.
    </p>

 <pre>
 ...
 &lt;devices&gt;
-  &lt;interface type='ethernet'/&gt;
+  &lt;interface type='ethernet'&gt;
+    &lt;script path='/etc/qemu-ifup-mynet'/&gt;
+  &lt;/interface&gt;
  ...
  &lt;interface type='ethernet'&gt;
-    &lt;target dev='vnet7'/&gt;
-    &lt;script path='/etc/qemu-ifup-mynet'/&gt;
+    &lt;target dev='mytap1' managed='no'/&gt;
+    &lt;model type='virtio'/&gt;
  &lt;/interface&gt;
 &lt;/devices&gt;
 ...</pre>
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@ -2885,6 +2885,11 @@
          <attribute name="dev">
            <ref name="deviceName"/>
          </attribute>
+          <optional>
+            <attribute name="managed">
+              <ref name="virYesNo"/>
+            </attribute>
+          </optional>
          <empty/>
        </element>
      </optional>
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@ -6112,6 +6112,14 @@ virDomainNetDefValidate(const virDomainNetDef *net)
                       virDomainNetTypeToString(net->type));
        return -1;
    }
+    if (net->managed_tap == VIR_TRISTATE_BOOL_NO &&
+        net->type != VIR_DOMAIN_NET_TYPE_ETHERNET) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("unmanaged target dev is not supported on "
+                         "interfaces of type '%s'"),
+                       virDomainNetTypeToString(net->type));
+        return -1;
+    }
    return 0;
 }

@ -11421,6 +11429,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
    VIR_AUTOFREE(char *) bridge = NULL;
    VIR_AUTOFREE(char *) dev = NULL;
    VIR_AUTOFREE(char *) ifname = NULL;
+    VIR_AUTOFREE(char *) managed_tap = NULL;
    VIR_AUTOFREE(char *) ifname_guest = NULL;
    VIR_AUTOFREE(char *) ifname_guest_actual = NULL;
    VIR_AUTOFREE(char *) script = NULL;
@ -11583,13 +11592,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
            } else if (!ifname &&
                       virXMLNodeNameEqual(cur, "target")) {
                ifname = virXMLPropString(cur, "dev");
-                if (ifname &&
-                    (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
-                    (STRPREFIX(ifname, VIR_NET_GENERATED_TAP_PREFIX) ||
-                     (prefix && STRPREFIX(ifname, prefix)))) {
-                    /* An auto-generated target name, blank it out */
-                    VIR_FREE(ifname);
-                }
+                managed_tap = virXMLPropString(cur, "managed");
            } else if ((!ifname_guest || !ifname_guest_actual) &&
                       virXMLNodeNameEqual(cur, "guest")) {
                ifname_guest = virXMLPropString(cur, "dev");
@ -11923,6 +11926,27 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
                                   ctxt, &def->guestIP) < 0)
        goto error;

+    if (managed_tap) {
+        if (STREQ(managed_tap, "no")) {
+            def->managed_tap = VIR_TRISTATE_BOOL_NO;
+        } else if (STREQ(managed_tap, "yes")) {
+            def->managed_tap = VIR_TRISTATE_BOOL_YES;
+        } else {
+            virReportError(VIR_ERR_XML_ERROR,
+                           _("invalid 'managed' value '%s'"),
+                           managed_tap);
+            goto error;
+        }
+    }
+
+    if (def->managed_tap != VIR_TRISTATE_BOOL_NO && ifname &&
+        (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
+        (STRPREFIX(ifname, VIR_NET_GENERATED_TAP_PREFIX) ||
+         (prefix && STRPREFIX(ifname, prefix)))) {
+        /* An auto-generated target name, blank it out */
+        VIR_FREE(ifname);
+    }
+
    if (script != NULL)
        VIR_STEAL_PTR(def->script, script);
    if (domain_name != NULL)
@ -25550,12 +25574,17 @@ virDomainNetDefFormat(virBufferPtr buf,
    virBufferEscapeString(buf, "<backenddomain name='%s'/>\n", def->domain_name);

    if (def->ifname &&
-        !((flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE) &&
-          (STRPREFIX(def->ifname, VIR_NET_GENERATED_TAP_PREFIX) ||
-           (prefix && STRPREFIX(def->ifname, prefix))))) {
+        (def->managed_tap == VIR_TRISTATE_BOOL_NO ||
+         !((flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE) &&
+           (STRPREFIX(def->ifname, VIR_NET_GENERATED_TAP_PREFIX) ||
+            (prefix && STRPREFIX(def->ifname, prefix)))))) {
        /* Skip auto-generated target names for inactive config. */
        virBufferEscapeString(&attrBuf, " dev='%s'", def->ifname);
    }
+    if (def->managed_tap != VIR_TRISTATE_BOOL_ABSENT) {
+        virBufferAsprintf(&attrBuf, " managed='%s'",
+                          virTristateBoolTypeToString(def->managed_tap));
+    }

    if (virXMLFormatElement(buf, "target", &attrBuf, NULL) < 0)
        return -1;
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@ -1004,6 +1004,7 @@ struct _virDomainNetDef {
    char *script;
    char *domain_name; /* backend domain name */
    char *ifname; /* interface name on the host (<target dev='x'/>) */
+    int managed_tap; /* enum virTristateBool - ABSENT == YES */
    virNetDevIPInfo hostIP;
    char *ifname_guest_actual;
    char *ifname_guest;
--- a/tests/qemuxml2argvdata/net-eth-unmanaged-tap.xml
+++ b/tests/qemuxml2argvdata/net-eth-unmanaged-tap.xml
@ -0,0 +1,35 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219100</memory>
+  <currentMemory unit='KiB'>219100</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-i686</emulator>
+    <disk type='block' device='disk'>
+      <driver name='qemu' type='raw'/>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <controller type='usb' index='0'/>
+    <controller type='ide' index='0'/>
+    <controller type='pci' index='0' model='pci-root'/>
+    <interface type='ethernet'>
+      <mac address='fe:11:22:33:44:55'/>
+      <target dev='mytap0' managed='no'/>
+      <model type='virtio'/>
+    </interface>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <memballoon model='none'/>
+  </devices>
+</domain>
--- a/tests/qemuxml2xmloutdata/net-eth-unmanaged-tap.xml
+++ b/tests/qemuxml2xmloutdata/net-eth-unmanaged-tap.xml
@ -0,0 +1,40 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219100</memory>
+  <currentMemory unit='KiB'>219100</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-i686</emulator>
+    <disk type='block' device='disk'>
+      <driver name='qemu' type='raw'/>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <controller type='usb' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+    </controller>
+    <controller type='ide' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'/>
+    <interface type='ethernet'>
+      <mac address='fe:11:22:33:44:55'/>
+      <target dev='mytap0' managed='no'/>
+      <model type='virtio'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+    </interface>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <memballoon model='none'/>
+  </devices>
+</domain>
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@ -408,6 +408,7 @@ mymain(void)
    DO_TEST("net-eth", NONE);
    DO_TEST("net-eth-ifname", NONE);
    DO_TEST("net-eth-hostip", NONE);
+    DO_TEST("net-eth-unmanaged-tap", NONE);
    DO_TEST("net-virtio-network-portgroup", NONE);
    DO_TEST("net-virtio-rxtxqueuesize", NONE);
    DO_TEST("net-hostdev", NONE);