Re: [PATCH-RFC-V2] qemu: Add network bandwidth setting for ethernet interfaces

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 18.09.2014 01:33, Anirban Chakraborty wrote:
V2:
Consolidate calls to virNetDevBandwidthSet
Clear bandwidth settings when the interface is detached or domain destroyed

V1:
Ethernet interfaces in libvirt currently do not support bandwidth setting.
For example, following xml file for an interface will not apply these
settings to corresponding qdiscs.

     <interface type="ethernet">
       <mac address="02:36:1d:18:2a:e4"/>
       <model type="virtio"/>
       <script path=""/>
       <target dev="tap361d182a-e4"/>
       <bandwidth>
         <inbound average="984" peak="1024" burst="64"/>
         <outbound average="2000" peak="2048" burst="128"/>
       </bandwidth>
     </interface>

Signed-off-by: Anirban Chakraborty <abchak@xxxxxxxxxxx>
---
  src/conf/domain_conf.h      |  7 +++++++
  src/lxc/lxc_process.c       | 27 ++++++++++++++-------------
  src/qemu/qemu_command.c     |  9 ++++-----
  src/qemu/qemu_driver.c      | 21 +++++++++++++++++++++
  src/qemu/qemu_hotplug.c     | 13 +++++++++++++
  src/util/virnetdevmacvlan.c | 10 ----------
  src/util/virnetdevmacvlan.h |  1 -
  7 files changed, 59 insertions(+), 29 deletions(-)

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 640a4c5..3c950f1 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -829,6 +829,13 @@ typedef enum {
      VIR_DOMAIN_NET_TYPE_LAST
  } virDomainNetType;

+/* check bandwidth configuration for a network interface */
+#define NETDEVIF_SUPPORT_BANDWIDTH(type) \
+   ((type == VIR_DOMAIN_NET_TYPE_ETHERNET || \
+     type == VIR_DOMAIN_NET_TYPE_NETWORK  || \
+     type == VIR_DOMAIN_NET_TYPE_BRIDGE   || \
+     type == VIR_DOMAIN_NET_TYPE_DIRECT) ? true : false)
+

I'd rather turn this into a function (possibly inline function).

  /* the backend driver used for virtio interfaces */
  typedef enum {
      VIR_DOMAIN_NET_BACKEND_TYPE_DEFAULT, /* prefer kernel, fall back to user */
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index ed30c37..5ef91e8 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -274,11 +274,6 @@ char *virLXCProcessSetupInterfaceBridged(virConnectPtr conn,
      if (virNetDevSetOnline(parentVeth, true) < 0)
          goto cleanup;

-    if (virNetDevBandwidthSet(net->ifname,
-                              virDomainNetGetActualBandwidth(net),
-                              false) < 0)
-        goto cleanup;
-

Well, the virLXCProcessSetupInterfaceBridged is used elsewhere in the code too. For instance after this patch the QoS is not applied on interfaces hotplugged into an LXC container.

      if (net->filter &&
          virDomainConfNWFilterInstantiate(conn, vm->uuid, net) < 0)
          goto cleanup;
@@ -300,6 +295,7 @@ char *virLXCProcessSetupInterfaceDirect(virConnectPtr conn,
      virNetDevBandwidthPtr bw;
      virNetDevVPortProfilePtr prof;
      virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver);
+    const char *linkdev = virDomainNetGetActualDirectDev(net);

      /* XXX how todo bandwidth controls ?
       * Since the 'net-ifname' is about to be moved to a different
@@ -329,16 +325,15 @@ char *virLXCProcessSetupInterfaceDirect(virConnectPtr conn,

      if (virNetDevMacVLanCreateWithVPortProfile(
              net->ifname, &net->mac,
-            virDomainNetGetActualDirectDev(net),
+            linkdev,
              virDomainNetGetActualDirectMode(net),
              false, def->uuid,
-            virDomainNetGetActualVirtPortProfile(net),
+            prof,
              &res_ifname,
              VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
              cfg->stateDir,
-            virDomainNetGetActualBandwidth(net), 0) < 0)
+            0) < 0)
          goto cleanup;
-
      ret = res_ifname;

   cleanup:
@@ -368,6 +363,7 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn,
      int ret = -1;
      size_t i;
      size_t niface = 0;
+    int actualType;

      for (i = 0; i < def->nnets; i++) {
          char *veth = NULL;
@@ -381,7 +377,8 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn,
          if (VIR_EXPAND_N(*veths, *nveths, 1) < 0)
              goto cleanup;

-        switch (virDomainNetGetActualType(def->nets[i])) {
+        actualType = virDomainNetGetActualType(def->nets[i]);
+        switch (actualType) {
          case VIR_DOMAIN_NET_TYPE_NETWORK: {
              virNetworkPtr network;
              char *brname = NULL;
@@ -444,11 +441,15 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn,
          case VIR_DOMAIN_NET_TYPE_LAST:
              virReportError(VIR_ERR_INTERNAL_ERROR,
                             _("Unsupported network type %s"),
-                           virDomainNetTypeToString(
-                               virDomainNetGetActualType(def->nets[i])
-                               ));
+                           virDomainNetTypeToString(actualType));
              goto cleanup;
          }
+        /* set network bandwidth */
+        if (NETDEVIF_SUPPORT_BANDWIDTH(actualType) && virNetDevBandwidthSet(
+                def->nets[i]->ifname, virDomainNetGetActualBandwidth(
+                def->nets[i]),
+                false) < 0)

I must say this formatting looks very strange to me in libvirt connotations.

+           goto cleanup;

          (*veths)[(*nveths)-1] = veth;

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f2e6e5a..404c51b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -191,7 +191,6 @@ qemuPhysIfaceConnect(virDomainDefPtr def,
          virDomainNetGetActualVirtPortProfile(net),
          &res_ifname,
          vmop, cfg->stateDir,
-        virDomainNetGetActualBandwidth(net),
          macvlan_create_flags);
      if (rc >= 0) {
          virDomainAuditNetDevice(def, net, res_ifname, true);
@@ -371,10 +370,6 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
                                    &net->mac) < 0)
          goto cleanup;

-    if (virNetDevBandwidthSet(net->ifname,
-                              virDomainNetGetActualBandwidth(net),
-                              false) < 0)
-        goto cleanup;

      if (net->filter &&
          virDomainConfNWFilterInstantiate(conn, def->uuid, net) < 0) {
@@ -7313,6 +7308,10 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
          if (tapfd[0] < 0)
              goto cleanup;
      }
+    /* Set bandwidth  */
+    if (NETDEVIF_SUPPORT_BANDWIDTH(actualType) && virNetDevBandwidthSet(
+            net->ifname, virDomainNetGetActualBandwidth(net), false) < 0)
+        goto cleanup;

      if ((actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
           actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5fd89a3..f5a5cbe 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -186,6 +186,9 @@ qemuVMFilterRebuild(virDomainObjListIterator iter, void *data)
      return virDomainObjListForEach(qemu_driver->domains, iter, data);
  }

+static void
+qemuDomainClearNetBandwidth(virDomainObjPtr vm);
+
  static virNWFilterCallbackDriver qemuCallbackDriver = {
      .name = QEMU_DRIVER_NAME,
      .vmFilterRebuild = qemuVMFilterRebuild,
@@ -2196,6 +2199,9 @@ qemuDomainDestroyFlags(virDomainPtr dom,
      if (virDomainDestroyFlagsEnsureACL(dom->conn, vm->def) < 0)
          goto cleanup;

+    /* Clear network bandwidth settings */
+    qemuDomainClearNetBandwidth(vm);
+


No. This clears QoS only if the destroy API is called. However, domain may terminate by other means where the QoS should be cleared too, e.g. 'sudo poweroff' caled from within the domain. So this call needs to go into qemuProcessStop.

      qemuDomainSetFakeReboot(driver, vm, false);


@@ -17952,6 +17958,21 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
      return ret;
  }

+/* Clear the bandwidth setting of all the network interfaces of a vm */
+static void
+qemuDomainClearNetBandwidth(virDomainObjPtr vm)
+{
+    size_t i;
+    int actualType;
+
+    for (i = 0; i < vm->def->nnets; i++) {
+        virDomainNetDefPtr net = vm->def->nets[i];
+        actualType = virDomainNetGetActualType(net);
+        if (NETDEVIF_SUPPORT_BANDWIDTH(actualType))
+            virNetDevBandwidthClear(net->ifname);
+    }
+}
+

Okay. Previously QoS was set on TAP devices only. So there was no need to call virNetDevBandwidthClear as the TAP devices were removed with qemu process tearing down. However, this clears just qemu domains and left LXC containers uncleared.

  static virDriver qemuDriver = {
      .no = VIR_DRV_QEMU,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 7bc19cd..acf48af 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -947,6 +947,10 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
          if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, vhostfd, &vhostfdSize) < 0)
              goto cleanup;
      }
+    /* Set bandwidth  */
+    if (NETDEVIF_SUPPORT_BANDWIDTH(actualType) && virNetDevBandwidthSet(net->ifname,
+            virDomainNetGetActualBandwidth(net), false) < 0)
+        goto cleanup;

      for (i = 0; i < tapfdSize; i++) {
          if (virSecurityManagerSetTapFDLabel(driver->securityManager,
@@ -3481,6 +3485,7 @@ qemuDomainDetachNetDevice(virConnectPtr conn,
      virDomainNetDefPtr detach = NULL;
      qemuDomainObjPrivatePtr priv = vm->privateData;
      int rc;
+    int actualType;

      if ((detachidx = virDomainNetFindIdx(vm->def, dev->data.net)) < 0)
          goto cleanup;
@@ -3517,6 +3522,14 @@ qemuDomainDetachNetDevice(virConnectPtr conn,
          }
      }

+    actualType = virDomainNetGetActualType(detach);
+    if (NETDEVIF_SUPPORT_BANDWIDTH(actualType) &&
+        (virNetDevBandwidthClear(detach->ifname) < 0)) {
+        virReportError(VIR_ERR_OPERATION_FAILED,
+                       _("cannot clear bandwidth setting for device : %s"),
+                       detach->ifname);
+        goto cleanup;
+    }
      qemuDomainMarkDeviceForRemoval(vm, &detach->info);

      qemuDomainObjEnterMonitor(driver, vm);

Michal

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]