From 77f72a86159beccc6cfff3392a2aca78485729be Mon Sep 17 00:00:00 2001 From: Laine Stump Date: Wed, 21 Aug 2019 16:42:41 -0400 Subject: [PATCH] conf: new "managed" attribute for target dev of MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Although 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 element. This attribute is only valid for (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: ... This patch just adds managed to the grammar and parser for , 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 Reviewed-by: Daniel P. Berrangé --- docs/formatdomain.html.in | 48 ++++++++++++++---- docs/schemas/domaincommon.rng | 5 ++ src/conf/domain_conf.c | 49 +++++++++++++++---- src/conf/domain_conf.h | 1 + .../net-eth-unmanaged-tap.xml | 35 +++++++++++++ .../net-eth-unmanaged-tap.xml | 40 +++++++++++++++ tests/qemuxml2xmltest.c | 1 + 7 files changed, 159 insertions(+), 20 deletions(-) create mode 100644 tests/qemuxml2argvdata/net-eth-unmanaged-tap.xml create mode 100644 tests/qemuxml2xmloutdata/net-eth-unmanaged-tap.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index fcb7c59c00..86a5261e47 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5511,24 +5511,52 @@
Generic ethernet connection

- 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 - <target> 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). +

+

+ The name of the tap device can optionally be specified with + the dev attribute of the + <target> 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 dev is a + pre-existing device, and the managed attribute of + the target 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. Since 5.7.0 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. +

+

+ After creating/opening the tap device, an optional shell script + (given in the path attribute of + the <script> element) will be run; this can + be used to do whatever extra host network integration is + required. 

 ...
 <devices>
-  <interface type='ethernet'/>
-  ...
   <interface type='ethernet'>
-    <target dev='vnet7'/>
     <script path='/etc/qemu-ifup-mynet'/>
   </interface>
+  ...
+  <interface type='ethernet'>
+    <target dev='mytap1' managed='no'/>
+    <model type='virtio'/>
+  </interface>
 </devices>
 ...
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index c48f8c4f56..cae3be639e 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2885,6 +2885,11 @@ + + + + + diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ddc72d3cf9..7f49c8253f 100644 --- 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, "\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; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c00d2b4953..af80c2b7ab 100644 --- 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 () */ + int managed_tap; /* enum virTristateBool - ABSENT == YES */ virNetDevIPInfo hostIP; char *ifname_guest_actual; char *ifname_guest; diff --git a/tests/qemuxml2argvdata/net-eth-unmanaged-tap.xml b/tests/qemuxml2argvdata/net-eth-unmanaged-tap.xml new file mode 100644 index 0000000000..7f5a0c217b --- /dev/null +++ b/tests/qemuxml2argvdata/net-eth-unmanaged-tap.xml @@ -0,0 +1,35 @@ + + QEMUGuest1 + c7a5fdbd-edaf-9455-926a-d65c16db1809 + 219100 + 219100 + 1 + + hvm + + + + destroy + restart + destroy + + /usr/bin/qemu-system-i686 + + + + +
+ + + + + + + + + + + + + + diff --git a/tests/qemuxml2xmloutdata/net-eth-unmanaged-tap.xml b/tests/qemuxml2xmloutdata/net-eth-unmanaged-tap.xml new file mode 100644 index 0000000000..cdff179932 --- /dev/null +++ b/tests/qemuxml2xmloutdata/net-eth-unmanaged-tap.xml @@ -0,0 +1,40 @@ + + QEMUGuest1 + c7a5fdbd-edaf-9455-926a-d65c16db1809 + 219100 + 219100 + 1 + + hvm + + + + destroy + restart + destroy + + /usr/bin/qemu-system-i686 + + + + +
+ + +
+ + +
+ + + + + + +
+ + + + + + diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index c4bc05e679..d5c66d8791 100644 --- 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); -- GitLab