5:45 a.m.
Hello Ondřej Svoboda,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/24152
to review the following change.
Change subject: vm iface statistics: never report negative rates
......................................................................
vm iface statistics: never report negative rates
When Linux tx/rx couters overflow their 32 bit limit, they wrap back to
zero. This causes our reported rxRate/txRate to become negative for one
nample. 51073875161d9b932b7
Bug-Url: https://bugzilla.redhat.com/1062166
Change-Id: I40daeb12172a7f54f135b3018405ef228983fa93
Signed-off-by: Dan Kenigsberg <danken(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/23598
Tested-by: Ondřej Svoboda <osvoboda(a)redhat.com>
---
M vdsm/vm.py
1 file changed, 4 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/52/24152/1
diff --git a/vdsm/vm.py b/vdsm/vm.py
index bf64195..d88d356 100644
--- a/vdsm/vm.py
+++ b/vdsm/vm.py
@@ -609,10 +609,12 @@
ifStats['txDropped'] = str(eInfo[nic.name][7])
ifRxBytes = (100.0 *
- (eInfo[nic.name][0] - sInfo[nic.name][0]) /
+ (eInfo[nic.name][0] - sInfo[nic.name][0]) %
+ 2 ** 32 /
sampleInterval / ifSpeed / self.MBPS_TO_BPS)
ifTxBytes = (100.0 *
- (eInfo[nic.name][4] - sInfo[nic.name][4]) /
+ (eInfo[nic.name][4] - sInfo[nic.name][4]) %
+ 2 ** 32 /
sampleInterval / ifSpeed / self.MBPS_TO_BPS)
ifStats['rxRate'] = '%.1f' % ifRxBytes
--
To view, visit http://gerrit.ovirt.org/24152
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I40daeb12172a7f54f135b3018405ef228983fa93
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.4
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Ondřej Svoboda <osvoboda(a)redhat.com>
11:18 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: vm iface statistics: never report negative rates
......................................................................
Patch Set 1: Verified+1
verified in master
--
To view, visit http://gerrit.ovirt.org/24152
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I40daeb12172a7f54f135b3018405ef228983fa93
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.4
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Assaf Muller <amuller(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Ondřej Svoboda <osvoboda(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
6:15 a.m.
Yaniv Bronhaim has submitted this change and it was merged.
Change subject: vm iface statistics: never report negative rates
......................................................................
vm iface statistics: never report negative rates
When Linux tx/rx couters overflow their 32 bit limit, they wrap back to
zero. This causes our reported rxRate/txRate to become negative for one
nample. 51073875161d9b932b7
Bug-Url: https://bugzilla.redhat.com/1062166
Change-Id: I40daeb12172a7f54f135b3018405ef228983fa93
Signed-off-by: Dan Kenigsberg <danken(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/23598
Tested-by: Ondřej Svoboda <osvoboda(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/24152
Reviewed-by: Antoni Segura Puimedon <asegurap(a)redhat.com>
Reviewed-by: Yaniv Bronhaim <ybronhei(a)redhat.com>
---
M vdsm/vm.py
1 file changed, 4 insertions(+), 2 deletions(-)
Approvals:
Yaniv Bronhaim: Looks good to me, approved
Antoni Segura Puimedon: Looks good to me, but someone else must approve
Dan Kenigsberg: Verified
--
To view, visit http://gerrit.ovirt.org/24152
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I40daeb12172a7f54f135b3018405ef228983fa93
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.4
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Assaf Muller <amuller(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Ondřej Svoboda <osvoboda(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
6:14 a.m.
Yaniv Bronhaim has posted comments on this change.
Change subject: vm iface statistics: never report negative rates
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.ovirt.org/24152
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I40daeb12172a7f54f135b3018405ef228983fa93
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.4
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Assaf Muller <amuller(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Ondřej Svoboda <osvoboda(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
6:30 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: vm iface statistics: never report negative rates
......................................................................
Patch Set 1:
... and it's good that you have not noticed that this is a backport, as your comments
are correct and welcome.
--
To view, visit http://gerrit.ovirt.org/24152
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I40daeb12172a7f54f135b3018405ef228983fa93
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.4
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Assaf Muller <amuller(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Ondřej Svoboda <osvoboda(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
4:50 a.m.
Nir Soffer has posted comments on this change.
Change subject: vm iface statistics: never report negative rates
......................................................................
Patch Set 1: -Code-Review
I did not notice that this is a backport, hopefully this will be improved upstream.
--
To view, visit http://gerrit.ovirt.org/24152
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I40daeb12172a7f54f135b3018405ef228983fa93
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.4
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Assaf Muller <amuller(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Ondřej Svoboda <osvoboda(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
4:35 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: vm iface statistics: never report negative rates
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.ovirt.org/#/c/24152/1/vdsm/vm.py
File vdsm/vm.py:
Line 609: ifStats['txDropped'] = str(eInfo[nic.name][7])
Line 610:
Line 611: ifRxBytes = (100.0 *
Line 612: (eInfo[nic.name][0] - sInfo[nic.name][0]) %
Line 613: 2 ** 32 /
Use constant and explain why the maximum value is 2**32. Can we get
this va
You comment is correct, and I have a partial answer in
http://gerrit.ovirt.org/24194 .
Since this patch is an ovirt-3.4 backport of a bug fix, would you agree to ack it, and
continue the discussion on the master branch?
Line 614: sampleInterval / ifSpeed / self.MBPS_TO_BPS)
Line 615: ifTxBytes = (100.0 *
Line 616: (eInfo[nic.name][4] - sInfo[nic.name][4]) %
Line 617: 2 ** 32 /
--
To view, visit http://gerrit.ovirt.org/24152
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I40daeb12172a7f54f135b3018405ef228983fa93
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.4
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Assaf Muller <amuller(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Ondřej Svoboda <osvoboda(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
3:48 p.m.
Nir Soffer has posted comments on this change.
Change subject: vm iface statistics: never report negative rates
......................................................................
Patch Set 1: Code-Review-1
(1 comment)
Change may be correct, but introduce duplicated undocumented magic number. Not clear where
this number comes from, if we can get it from the system, and if this number can change in
future version, breaking the proposed change.
http://gerrit.ovirt.org/#/c/24152/1/vdsm/vm.py
File vdsm/vm.py:
Line 609: ifStats['txDropped'] = str(eInfo[nic.name][7])
Line 610:
Line 611: ifRxBytes = (100.0 *
Line 612: (eInfo[nic.name][0] - sInfo[nic.name][0]) %
Line 613: 2 ** 32 /
Use constant and explain why the maximum value is 2**32. Can we get this value from the
system instead of using a hardcoded value?
Line 614: sampleInterval / ifSpeed / self.MBPS_TO_BPS)
Line 615: ifTxBytes = (100.0 *
Line 616: (eInfo[nic.name][4] - sInfo[nic.name][4]) %
Line 617: 2 ** 32 /
--
To view, visit http://gerrit.ovirt.org/24152
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I40daeb12172a7f54f135b3018405ef228983fa93
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.4
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Assaf Muller <amuller(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Ondřej Svoboda <osvoboda(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
11:01 a.m.
Antoni Segura Puimedon has posted comments on this change.
Change subject: vm iface statistics: never report negative rates
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit http://gerrit.ovirt.org/24152
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I40daeb12172a7f54f135b3018405ef228983fa93
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.4
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Assaf Muller <amuller(a)redhat.com>
Gerrit-Reviewer: Ondřej Svoboda <osvoboda(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
3788
days inactive
3793
days old
vdsm-patches@lists.fedorahosted.org
8 comments
4 participants
participants (4)
-
asegurap＠redhat.com -
Dan Kenigsberg -
Nir Soffer -
ybronhei＠redhat.com