[libvirt] [PATCH-RFC-V2] qemu: Add network bandwidth setting for ethernet interfaces
Michal Privoznik
mprivozn at redhat.com
Mon Oct 6 10:43:05 UTC 2014
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 at juniper.net>
> ---
> 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
More information about the libvir-list
mailing list