On 12.12.2014 17:08, Vasiliy Tolstov wrote:
If user not specify network type ethernet, assume that user
needs simple tap device created with libvirt.
This patch does not need to run external script to create tap device or
add root to qemu process. Also libvirt runs script after device creating,
if user provide it.
Signed-off-by: Vasiliy Tolstov <v.tolstov(a)selfip.ru>
---
src/qemu/qemu_command.c | 157 ++++++++++++++++++++++++++++++++----------------
src/qemu/qemu_hotplug.c | 10 +--
src/qemu/qemu_process.c | 4 ++
3 files changed, 111 insertions(+), 60 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 48bdf4e..98c2ba1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -278,9 +278,9 @@ static int qemuCreateInBridgePortWithHelper(virQEMUDriverConfigPtr
cfg,
}
/* qemuNetworkIfaceConnect - *only* called if actualType is
- * VIR_DOMAIN_NET_TYPE_NETWORK or VIR_DOMAIN_NET_TYPE_BRIDGE (i.e. if
- * the connection is made with a tap device connecting to a bridge
- * device)
+ * VIR_DOMAIN_NET_TYPE_NETWORK, VIR_DOMAIN_NET_TYPE_BRIDGE or
+ * VIR_DOMAIN_NET_TYPE_ETHERNET (i.e. if the connection is
+ * made with a tap device)
*/
int
qemuNetworkIfaceConnect(virDomainDefPtr def,
@@ -321,49 +321,61 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR;
}
- if (cfg->privileged) {
- if (virNetDevTapCreateInBridgePort(brname, &net->ifname,
&net->mac,
- def->uuid, tunpath, tapfd, *tapfdSize,
- virDomainNetGetActualVirtPortProfile(net),
- virDomainNetGetActualVlan(net),
- tap_create_flags) < 0) {
+ if (actualType == VIR_DOMAIN_NET_TYPE_ETHERNET) {
I wonder how this can even work. I mean, not shown in the context, but a few lines above
there's check for @net's bridge name. If there's none (and there really is
none for ETHERNET), an error is thrown and control returns immediately after that.
+ if (virNetDevTapCreate(&net->ifname, tunpath, tapfd,
*tapfdSize,
+ tap_create_flags) < 0) {
virDomainAuditNetDevice(def, net, tunpath, false);
goto cleanup;
}
- if (virDomainNetGetActualBridgeMACTableManager(net)
- == VIR_NETWORK_BRIDGE_MAC_TABLE_MANAGER_LIBVIRT) {
- /* libvirt is managing the FDB of the bridge this device
- * is attaching to, so we need to turn off learning and
- * unicast_flood on the device to prevent the kernel from
- * adding any FDB entries for it, then add an fdb entry
- * outselves, using the MAC address from the interface
- * config.
- */
- if (virNetDevBridgePortSetLearning(brname, net->ifname, false) < 0)
- goto cleanup;
- if (virNetDevBridgePortSetUnicastFlood(brname, net->ifname, false) <
0)
- goto cleanup;
- if (virNetDevBridgeFDBAdd(&net->mac, net->ifname,
- VIR_NETDEVBRIDGE_FDB_FLAG_MASTER |
- VIR_NETDEVBRIDGE_FDB_FLAG_TEMP) < 0)
+ if (net->script) {
+ if (qemuExecuteEthernetScript(net->ifname, net->script) < 0)
And here I wonder how this can even compile. qemuExecuteEthernetScript() is undefined here
as you declare it a few hundred lines below.
goto cleanup;
}
- } else {
- if (qemuCreateInBridgePortWithHelper(cfg, brname,
- &net->ifname,
- tapfd, tap_create_flags) < 0) {
- virDomainAuditNetDevice(def, net, tunpath, false);
+ if (virNetDevSetOnline(net->ifname, !!(tap_create_flags &
VIR_NETDEV_TAP_CREATE_IFUP)) < 0)
goto cleanup;
+ } else {
+ if (cfg->privileged) {
+ if (virNetDevTapCreateInBridgePort(brname, &net->ifname,
&net->mac,
+ def->uuid, tunpath, tapfd,
*tapfdSize,
+
virDomainNetGetActualVirtPortProfile(net),
+ virDomainNetGetActualVlan(net),
+ tap_create_flags) < 0) {
+ virDomainAuditNetDevice(def, net, tunpath, false);
+ goto cleanup;
+ }
+ if (virDomainNetGetActualBridgeMACTableManager(net)
+ == VIR_NETWORK_BRIDGE_MAC_TABLE_MANAGER_LIBVIRT) {
+ /* libvirt is managing the FDB of the bridge this device
+ * is attaching to, so we need to turn off learning and
+ * unicast_flood on the device to prevent the kernel from
+ * adding any FDB entries for it, then add an fdb entry
+ * outselves, using the MAC address from the interface
+ * config.
+ */
+ if (virNetDevBridgePortSetLearning(brname, net->ifname, false) <
0)
+ goto cleanup;
+ if (virNetDevBridgePortSetUnicastFlood(brname, net->ifname, false)
< 0)
+ goto cleanup;
+ if (virNetDevBridgeFDBAdd(&net->mac, net->ifname,
+ VIR_NETDEVBRIDGE_FDB_FLAG_MASTER |
+ VIR_NETDEVBRIDGE_FDB_FLAG_TEMP) < 0)
+ goto cleanup;
+ }
+ } else {
+ if (qemuCreateInBridgePortWithHelper(cfg, brname,
+ &net->ifname,
+ tapfd, tap_create_flags) < 0) {
+ virDomainAuditNetDevice(def, net, tunpath, false);
+ goto cleanup;
+ }
+ /* qemuCreateInBridgePortWithHelper can only create a single FD */
+ if (*tapfdSize > 1) {
+ VIR_WARN("Ignoring multiqueue network request");
+ *tapfdSize = 1;
+ }
}
- /* qemuCreateInBridgePortWithHelper can only create a single FD */
- if (*tapfdSize > 1) {
- VIR_WARN("Ignoring multiqueue network request");
- *tapfdSize = 1;
- }
+ virDomainAuditNetDevice(def, net, tunpath, true);
}
-
- virDomainAuditNetDevice(def, net, tunpath, true);
-
if (cfg->macFilter &&
ebtablesAddForwardAllowIn(driver->ebtables,
net->ifname,
@@ -510,6 +522,40 @@ qemuOpenVhostNet(virDomainDefPtr def,
return -1;
}
+/**
+ * qemuExecuteEthernetScript:
+ * @ifname: the interface name
+ * @script: the script name
+
+ * This function executes script for new tap device created by libvirt.
+ *
+ * Returns 0 in case of success or -1 on failure
+ */
+static int qemuExecuteEthernetScript(const char *ifname, const char *script)
+{
+ virCommandPtr cmd;
+ int ret;
+
+ cmd = virCommandNew(script);
+ virCommandAddArgFormat(cmd, "%s", ifname);
+ virCommandClearCaps(cmd);
+#ifdef CAP_NET_ADMIN
+ virCommandAllowCap(cmd, CAP_NET_ADMIN);
+#endif
+ virCommandAddEnvPassCommon(cmd);
+
+ if (virCommandRun(cmd, NULL) < 0) {
+ ret = -1;
+ } else {
+ ret = 0;
+ }
+
+ cleanup:
This ^^ label is not used anywhere.
+ virCommandFree(cmd);
+ return ret;
+}
+
+
int
qemuNetworkPrepareDevices(virDomainDefPtr def)
{
@@ -4537,18 +4583,23 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
break;
case VIR_DOMAIN_NET_TYPE_ETHERNET:
- virBufferAddLit(&buf, "tap");
- if (net->ifname) {
- virBufferAsprintf(&buf, "%cifname=%s", type_sep,
net->ifname);
- type_sep = ',';
- }
- if (net->script) {
- virBufferAsprintf(&buf, "%cscript=%s", type_sep,
- net->script);
- type_sep = ',';
- }
- is_tap = true;
- break;
+ virBufferAddLit(&buf, "tap");
+ type_sep = ',';
This is not needed yet. I prefer the style that's used in other cases.
+ /* for one tapfd 'fd=' shall be used,
+ * for more than one 'fds=' is the right choice */
+ if (tapfdSize == 1) {
+ virBufferAsprintf(&buf, "%cfd=%s", type_sep, tapfd[0]);
+ } else {
+ virBufferAsprintf(&buf, "%cfds=", type_sep);
+ for (i = 0; i < tapfdSize; i++) {
+ if (i)
+ virBufferAddChar(&buf, ':');
+ virBufferAdd(&buf, tapfd[i], -1);
+ }
+ }
+ type_sep = ',';
+ is_tap = true;
+ break;
case VIR_DOMAIN_NET_TYPE_CLIENT:
virBufferAsprintf(&buf, "socket%cconnect=%s:%d",
@@ -7369,7 +7420,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
/* Currently nothing besides TAP devices supports multiqueue. */
if (net->driver.virtio.queues > 0 &&
!(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
- actualType == VIR_DOMAIN_NET_TYPE_BRIDGE)) {
+ actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
+ actualType == VIR_DOMAIN_NET_TYPE_ETHERNET)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Multiqueue network is not supported for: %s"),
virDomainNetTypeToString(actualType));
@@ -7377,7 +7429,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
}
if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
- actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {
+ actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
+ actualType == VIR_DOMAIN_NET_TYPE_ETHERNET) {
tapfdSize = net->driver.virtio.queues;
if (!tapfdSize)
tapfdSize = 1;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 8c0642e..6d6f6c4 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -904,7 +904,8 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
}
if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
- actualType == VIR_DOMAIN_NET_TYPE_NETWORK) {
+ actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
+ actualType == VIR_DOMAIN_NET_TYPE_ETHERNET) {
tapfdSize = vhostfdSize = net->driver.virtio.queues;
if (!tapfdSize)
tapfdSize = vhostfdSize = 1;
@@ -935,13 +936,6 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
iface_connected = true;
if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, vhostfd,
&vhostfdSize) < 0)
goto cleanup;
- } else if (actualType == VIR_DOMAIN_NET_TYPE_ETHERNET) {
- vhostfdSize = 1;
- if (VIR_ALLOC(vhostfd) < 0)
- goto cleanup;
- *vhostfd = -1;
- if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, vhostfd,
&vhostfdSize) < 0)
- goto cleanup;
}
/* Set device online immediately */
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index ab4df9b..1ca10b3 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5071,6 +5071,10 @@ void qemuProcessStop(virQEMUDriverPtr driver,
cfg->stateDir));
VIR_FREE(net->ifname);
break;
+ case VIR_DOMAIN_NET_TYPE_ETHERNET:
+ ignore_value(virNetDevTapDelete(net->ifname, net->backend.tap));
+ VIR_FREE(net->ifname);
+ break;
I think this call should be conditional. Otherwise I'm getting SIGSEGV:
==21845== 1 errors in context 1 of 24:
==21845== Thread 6:
==21845== Invalid read of size 1
==21845== at 0x4C2D282: strlen (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==21845== by 0x5312BE8: virStrcpy (virstring.c:543)
==21845== by 0x52F356C: virNetDevTapDelete (virnetdevtap.c:344)
==21845== by 0x1E05B3E7: qemuProcessStop (qemu_process.c:5075)
==21845== by 0x1E05A7D9: qemuProcessStart (qemu_process.c:4855)
==21845== by 0x1E0A26F2: qemuDomainObjStart (qemu_driver.c:6668)
==21845== by 0x1E0A2940: qemuDomainCreateWithFlags (qemu_driver.c:6723)
==21845== by 0x1E0A29A9: qemuDomainCreate (qemu_driver.c:6742)
==21845== by 0x53E7F07: virDomainCreate (libvirt-domain.c:6787)
==21845== by 0x125C0C: remoteDispatchDomainCreate (remote_dispatch.h:3177)
==21845== by 0x125B39: remoteDispatchDomainCreateHelper (remote_dispatch.h:3155)
==21845== by 0x545E0FD: virNetServerProgramDispatchCall (virnetserverprogram.c:437)
==21845== Address 0x0 is not stack'd, malloc'd or (recently) free'd
case VIR_DOMAIN_NET_TYPE_BRIDGE:
case VIR_DOMAIN_NET_TYPE_NETWORK:
#ifdef VIR_NETDEV_TAP_REQUIRE_MANUAL_CLEANUP
And I'm getting a few of 'make check' errors too.
Michal