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

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



On Fri, Sep 05, 2014 at 11:42:58PM +0000, Anirban Chakraborty wrote:


On 9/5/14, 1:31 AM, "Martin Kletzander" <mkletzan redhat com> wrote:

On Thu, Sep 04, 2014 at 03:02:54PM -0700, Anirban Chakraborty wrote:
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>
-----

This patch fixes the behavior.

Although this doesn't confuse git, it might confuse something else.
Leaving it without '----' is ok.

Please review it and if it appears ok, please apply.

thanks,
Anirban Chakraborty


No need to have this in the commit message, it's kept in the log then.


Signed-off-by: Anirban Chakraborty <abchak juniper net>
---
src/qemu/qemu_command.c | 5 +++++
src/qemu/qemu_hotplug.c | 3 +++
2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2184caa..258c6a7 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7251,6 +7251,11 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
        if (tapfd[0] < 0)
            goto cleanup;
    }
+	/* Configure network bandwidth for ethernet type network interfaces */
+	if (actualType == VIR_DOMAIN_NET_TYPE_ETHERNET)
+		if (virNetDevBandwidthSet(net->ifname,
+			virDomainNetGetActualBandwidth(net), false) < 0)
+			goto cleanup;


In libvirt, we indent by spaces.  This would be caught by running
'make syntax-check'.  Anyway, running 'make syntax-check check' is a
good practice to follow before formatting the patches.

Got it. Thanks for the pointer.


    if ((actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
         actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index a364c52..aeb53c5 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -940,6 +940,9 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
        if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, vhostfd,
&vhostfdSize) < 0)
            goto cleanup;
    } else if (actualType == VIR_DOMAIN_NET_TYPE_ETHERNET) {
+		if (virNetDevBandwidthSet(net->ifname,
+				virDomainNetGetActualBandwidth(net), false) < 0)
+			goto cleanup;

Same here.

We need to clear the bandwidth settings when shutting down the domain
or unplugging the device.

If I am not mistaken, currently, the bandwidth setting has been cleared
only for bridged devices (via
networkShutdownNetworkVirtual‹>virNetDevBandwidthClear ). Are you saying
that we should explicitly clear the bandwidth settings for unplugging and
shut down events? As far as my understanding, for both of these events,
the tap interface gets deleted, so there is no need to cleanup qdisc
settings of these devices.


It doesn't, actually.  <interface type='ethernet'><source
dev="tap123"/> ... makes use of the device tap123 that already exists
and leaves it in the system.  When I start and stop the domain with
the bandwidth set, it is kept in there.  If I were to start another
domain without any bandwidth setting (or just use the device somehow),
the speed would be affected.

Probably taking Laine's approach would be cleaner and easier (it could
also take care of this automagically).


Looking forward to v2,

I am working on it right now and will send it out soon. Thanks for the
feedback.

Anirban

Attachment: signature.asc
Description: Digital signature


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