Nir Soffer has posted comments on this change.
Change subject: sampling: use constants for counter bounds
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.ovirt.org/#/c/24194/1/vdsm/sampling.py
File vdsm/sampling.py:
Line 43: _THP_STATE_PATH =
'/sys/kernel/mm/redhat_transparent_hugepage/enabled'
Line 44:
Line 45: JIFFIES_BOUND = 2 ** 32
Line 46: NETSTATS_BOUND = 2 ** 32
Line 47:
BOUND is not the term that one would expect and is also not clear - are you talking about
upper bound or lower bound?. The common term for this is MAX everywhere else, e.g
http://pubs.opengroup.org/onlinepubs/009695399/basedefs/limits.h.html.
Regarding JIFFIES, the documentation why using 2**32 is correct when the samples may be
2**64 should be here and not in the commit message. Otherwise one has to search the
history of the file to find the documentation. This is specially important here because
using the modulo operator with negative numbers does not work like any other language and
is highly surprising. I guess that 90% of people reading this code cannot explain why it
is right.
It would be also useful to tell the reader that using x % 2**32 instead of x % 2**64 on
64bit machines is correct when 0 > x > 2**32 - 2**64 or 0 < x < 2**32
(hopefully I got it right).
Line 48:
Line 49: class InterfaceSample:
Line 50: """
Line 51: A network interface sample.
--
To view, visit
http://gerrit.ovirt.org/24194
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I706000106c3bc31edf8541c980bce1f49464ebf8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes