4:13 a.m.
Thanks to Peter who made me test this more and find some corner
cases. And even take a look on Coverity output.
Michal Privoznik (3):
networkBandwidthUpdate: Don't blindly dereference pointers
networkBandwidthGenericChecks: Drop useless check
cmdAttachInterface: Fully implement @floor support
src/network/bridge_driver.c | 7 ++-----
tools/virsh-domain.c | 8 ++++++--
2 files changed, 8 insertions(+), 7 deletions(-)
--
2.4.6
4:13 a.m.
New subject: [libvirt] [PATCH 1/3] networkBandwidthUpdate: Don't blindly dereference pointers
It may happen that an interface don't have any bandwidth set and
a new one is to be set. In that case, @ifaceBand will be NULL.
This will cause troubles later in the code when deciding what to
do.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/network/bridge_driver.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 666078c..f57c400 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -5048,8 +5048,8 @@ networkBandwidthUpdate(virDomainNetDefPtr iface,
/* Okay, there are three possible scenarios: */
- if (ifaceBand->in && ifaceBand->in->floor &&
- newBandwidth->in && newBandwidth->in->floor) {
+ if (ifaceBand && ifaceBand->in && ifaceBand->in->floor
&&
+ newBandwidth && newBandwidth->in &&
newBandwidth->in->floor) {
/* Either we just need to update @floor .. */
if (virNetDevBandwidthUpdateRate(network->def->bridge,
--
2.4.6
4:13 a.m.
New subject: [libvirt] [PATCH 2/3] networkBandwidthGenericChecks: Drop useless check
There's a check right at the beginning of the function that
shortcuts if the function was called over all NULL arguments.
However, this was meant just as a fool-proof check so that we
don't crash if function is used in a bad manner. Anyway, it makes
Coverity unhappy as it then thinks any of the arguments could be
NULL. Well, with the current state of the code it can't.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/network/bridge_driver.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index f57c400..c343e5b 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -4960,9 +4960,6 @@ networkBandwidthGenericChecks(virDomainNetDefPtr iface,
virNetDevBandwidthPtr ifaceBand;
unsigned long long old_floor, new_floor;
- if (!iface && !newBandwidth)
- return false;
-
if (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_NETWORK) {
/* This is not an interface that's plugged into a network.
* We don't care. Thus from our POV bandwidth change is allowed. */
--
2.4.6
6:03 a.m.
New subject: [libvirt] [PATCH 2/3] networkBandwidthGenericChecks: Drop useless check
On 08/12/2015 05:13 AM, Michal Privoznik wrote:
There's a check right at the beginning of the function that
shortcuts if the function was called over all NULL arguments.
However, this was meant just as a fool-proof check so that we
don't crash if function is used in a bad manner. Anyway, it makes
Coverity unhappy as it then thinks any of the arguments could be
NULL. Well, with the current state of the code it can't.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/network/bridge_driver.c | 3 ---
1 file changed, 3 deletions(-)
ACK - it was something I pointed out in my review though ;-)
(http://www.redhat.com/archives/libvir-list/2015-August/msg00374.html).
Hence why I pointed out to use ATTRIBUTE_NONNULL for both args
John
diff --git a/src/network/bridge_driver.c
b/src/network/bridge_driver.c
index f57c400..c343e5b 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -4960,9 +4960,6 @@ networkBandwidthGenericChecks(virDomainNetDefPtr iface,
virNetDevBandwidthPtr ifaceBand;
unsigned long long old_floor, new_floor;
- if (!iface && !newBandwidth)
- return false;
-
if (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_NETWORK) {
/* This is not an interface that's plugged into a network.
* We don't care. Thus from our POV bandwidth change is allowed. */
4:13 a.m.
New subject: [libvirt] [PATCH 3/3] cmdAttachInterface: Fully implement @floor support
In my previous commit d7f5c88961b52 I tried to introduce support
for inbound.floor. But the code change was incomplete. This is
the change needed to fully enable the feature.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/virsh-domain.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 9f54691..c8b0e76 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1023,12 +1023,16 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
if (inboundStr || outboundStr) {
virBufferAddLit(&buf, "<bandwidth>\n");
virBufferAdjustIndent(&buf, 2);
- if (inboundStr && inbound.average > 0) {
- virBufferAsprintf(&buf, "<inbound average='%llu'",
inbound.average);
+ if (inboundStr && (inbound.average || inbound.floor)) {
+ virBufferAddLit(&buf, "<inbound");
+ if (inbound.average > 0)
+ virBufferAsprintf(&buf, " average='%llu'",
inbound.average);
if (inbound.peak > 0)
virBufferAsprintf(&buf, " peak='%llu'",
inbound.peak);
if (inbound.burst > 0)
virBufferAsprintf(&buf, " burst='%llu'",
inbound.burst);
+ if (inbound.floor > 0)
+ virBufferAsprintf(&buf, " floor='%llu'",
inbound.floor);
virBufferAddLit(&buf, "/>\n");
}
if (outboundStr && outbound.average > 0) {
--
2.4.6
6:13 a.m.
On 08/12/2015 05:13 AM, Michal Privoznik wrote:
Thanks to Peter who made me test this more and find some corner
cases. And even take a look on Coverity output.
Michal Privoznik (3):
networkBandwidthUpdate: Don't blindly dereference pointers
networkBandwidthGenericChecks: Drop useless check
cmdAttachInterface: Fully implement @floor support
src/network/bridge_driver.c | 7 ++-----
tools/virsh-domain.c | 8 ++++++--
2 files changed, 8 insertions(+), 7 deletions(-)
ACK 1 & 3 too - (1 hasn't come to an inbox near me yet, but I see it in
the archive).
John
3425
days inactive
3425
days old
5 comments
2 participants
participants (2)
-
John Ferlan -
Michal Privoznik