Re: [libvirt] [PATCH] Clear bandwidth settings for a shutoff domain using domiftune

2014-08-12 Thread jiahu 


On 08/11/2014 06:05 PM, Michal Privoznik wrote:

On 11.08.2014 08:41, Jianwei Hu wrote:
qemu: To clear bandwidth settings for a shutoff domain by using 
domiftune.


After applying this patch, we can use virsh domiftune command to 
clear inbound

or/and outbound setting for a shutoff domain.

for example:
virsh domiftune $domain $interface 0 0


Thanks for catching this.



Please refer to below virsh help message:

man virsh:

To clear inbound or outbound settings, use --inbound or --outbound 
respectfully with average value of zero.

---
  src/qemu/qemu_driver.c | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 82a82aa..7db2e9c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9983,11 +9983,17 @@ qemuDomainSetInterfaceParameters(virDomainPtr 
dom,

  VIR_FREE(persistentNet-bandwidth-in);
  persistentNet-bandwidth-in = bandwidth-in;
  bandwidth-in = NULL;
+} else {
+VIR_FREE(persistentNet-bandwidth-in);
+persistentNet-bandwidth-in = 0;


We like NULL for pointer more than 0. Moreover, there's no need to 
explicitly set pointer freed to NULL as the VIR_FREE() macro does that 
already for you (in fact virFree() function does that, whatever).



  }
  if (bandwidth-out) {
  VIR_FREE(persistentNet-bandwidth-out);
  persistentNet-bandwidth-out = bandwidth-out;
  bandwidth-out = NULL;
+} else {
+VIR_FREE(persistentNet-bandwidth-out);
+persistentNet-bandwidth-out = 0;
  }
  }




But the fix isn't quite right. For instance:

virsh # domiftune dummy 52:54:00:89:3a:c2 --config
inbound.average: 10
inbound.peak   : 0
inbound.burst  : 0
outbound.average: 10
outbound.peak  : 0
outbound.burst : 0

virsh # domiftune dummy 52:54:00:89:3a:c2 --config 100

virsh # domiftune dummy 52:54:00:89:3a:c2 --config
inbound.average: 100
inbound.peak   : 0
inbound.burst  : 0
outbound.average: 0
outbound.peak  : 0
outbound.burst : 0

The bandwidth is cleared unconditionally. What we really need is:

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 82a82aa..2c3f179 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9983,11 +9983,15 @@ qemuDomainSetInterfaceParameters(virDomainPtr 
dom,

 VIR_FREE(persistentNet-bandwidth-in);
 persistentNet-bandwidth-in = bandwidth-in;
 bandwidth-in = NULL;
+} else  if (inboundSpecified) {
+VIR_FREE(persistentNet-bandwidth-in);
 }
 if (bandwidth-out) {
 VIR_FREE(persistentNet-bandwidth-out);
 persistentNet-bandwidth-out = bandwidth-out;
 bandwidth-out = NULL;
+} else if (outboundSpecified) {
+VIR_FREE(persistentNet-bandwidth-out);
 }
 }

I'm fixing this patch though and pushing. ACK.

Thanks for your advice and correction.


Michal


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

[libvirt] [PATCH] Clear bandwidth settings for a shutoff domain using domiftune

2014-08-11 Thread Jianwei Hu 
qemu: To clear bandwidth settings for a shutoff domain by using domiftune.

After applying this patch, we can use virsh domiftune command to clear inbound
or/and outbound setting for a shutoff domain.

for example:
virsh domiftune $domain $interface 0 0

Please refer to below virsh help message:

man virsh:

To clear inbound or outbound settings, use --inbound or --outbound respectfully 
with average value of zero.
---
 src/qemu/qemu_driver.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 82a82aa..7db2e9c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9983,11 +9983,17 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
 VIR_FREE(persistentNet-bandwidth-in);
 persistentNet-bandwidth-in = bandwidth-in;
 bandwidth-in = NULL;
+} else {
+VIR_FREE(persistentNet-bandwidth-in);
+persistentNet-bandwidth-in = 0;
 }
 if (bandwidth-out) {
 VIR_FREE(persistentNet-bandwidth-out);
 persistentNet-bandwidth-out = bandwidth-out;
 bandwidth-out = NULL;
+} else {
+VIR_FREE(persistentNet-bandwidth-out);
+persistentNet-bandwidth-out = 0;
 }
 }
 
-- 
1.8.3.1

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

Re: [libvirt] [PATCH] Clear bandwidth settings for a shutoff domain using domiftune

2014-08-11 Thread Michal Privoznik 

On 11.08.2014 08:41, Jianwei Hu wrote:

qemu: To clear bandwidth settings for a shutoff domain by using domiftune.

After applying this patch, we can use virsh domiftune command to clear inbound
or/and outbound setting for a shutoff domain.

for example:
virsh domiftune $domain $interface 0 0


Thanks for catching this.



Please refer to below virsh help message:

man virsh:

To clear inbound or outbound settings, use --inbound or --outbound respectfully 
with average value of zero.
---
  src/qemu/qemu_driver.c | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 82a82aa..7db2e9c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9983,11 +9983,17 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
  VIR_FREE(persistentNet-bandwidth-in);
  persistentNet-bandwidth-in = bandwidth-in;
  bandwidth-in = NULL;
+} else {
+VIR_FREE(persistentNet-bandwidth-in);
+persistentNet-bandwidth-in = 0;


We like NULL for pointer more than 0. Moreover, there's no need to 
explicitly set pointer freed to NULL as the VIR_FREE() macro does that 
already for you (in fact virFree() function does that, whatever).



  }
  if (bandwidth-out) {
  VIR_FREE(persistentNet-bandwidth-out);
  persistentNet-bandwidth-out = bandwidth-out;
  bandwidth-out = NULL;
+} else {
+VIR_FREE(persistentNet-bandwidth-out);
+persistentNet-bandwidth-out = 0;
  }
  }




But the fix isn't quite right. For instance:

virsh # domiftune dummy 52:54:00:89:3a:c2 --config
inbound.average: 10
inbound.peak   : 0
inbound.burst  : 0
outbound.average: 10
outbound.peak  : 0
outbound.burst : 0

virsh # domiftune dummy 52:54:00:89:3a:c2 --config 100

virsh # domiftune dummy 52:54:00:89:3a:c2 --config
inbound.average: 100
inbound.peak   : 0
inbound.burst  : 0
outbound.average: 0
outbound.peak  : 0
outbound.burst : 0

The bandwidth is cleared unconditionally. What we really need is:

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 82a82aa..2c3f179 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9983,11 +9983,15 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
 VIR_FREE(persistentNet-bandwidth-in);
 persistentNet-bandwidth-in = bandwidth-in;
 bandwidth-in = NULL;
+} else  if (inboundSpecified) {
+VIR_FREE(persistentNet-bandwidth-in);
 }
 if (bandwidth-out) {
 VIR_FREE(persistentNet-bandwidth-out);
 persistentNet-bandwidth-out = bandwidth-out;
 bandwidth-out = NULL;
+} else if (outboundSpecified) {
+VIR_FREE(persistentNet-bandwidth-out);
 }
 }

I'm fixing this patch though and pushing. ACK.

Michal

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