On 10/08/2013 02:38 AM, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
Some cgroup value range may change
s/range/ranges/
in the further kernel.
s/the further kernel/future kernels/
Introduce kernel version check function for cgroup. This will be helpful to determine the proper values.
I'm half-inclined to NACK. Version checks are lousy, as features may be backported to a distro-style kernel that reports an older version number. Is there any way you can probe for an actual feature, rather than relying on brittle version number checks?
+ +static bool virCgroupVersionCheck(const char * dstVersion)
Style: no space after '*' in pointer type declarations.
+{ + unsigned long currentVersion; + unsigned long version; + + if (virCgroupGetKernelVersion(¤tVersion) < 0) { + return -1; + } + + if (virParseVersionString(dstVersion, &version, true) < 0) { + return -1; + } + + return (((long long)currentVersion - (long long)version) >= 0) ? true : false;
The casts are pointless. Either we're on a 64-bit platform where it adds no precision, or we're on a 32-bit platform where the extra precision slows us down; but either way the version number already fits within a long. Style: 'bool_expr ? true: false' is wasteful; just write 'bool_expr'. Thus, this would be: return currentVersion >= version; if we still determine we need this function. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org