On 10/6/14, 3:43 AM, "Michal Privoznik" <mprivozn@xxxxxxxxxx> wrote:
>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).
Will do.
>
>> /* 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.
Thanks for pointing it out. I am taking care of it in my next patch.
>
>> 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.
Yeah, this was not right, for some reason it escaped my attention.
>
>> + 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.
Got it, thanks for catching it.
>
>> 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.
I¹ll take care of it in my next patch. Thanks.
Anirban
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list