[libvirt] [PATCH 1/2]cgroup: introduce kernel version check function for cgroup

[Chen Hanxiao]

> -----Original Message-----
> From: Eric Blake [mailto:eblake at redhat.com]
> Sent: Wednesday, October 09, 2013 11:49 AM
> To: Chen Hanxiao
> Cc: libvir-list at redhat.com
> Subject: Re: [libvirt] [PATCH 1/2]cgroup: introduce kernel version check function
> for cgroup
> 
> On 10/08/2013 02:38 AM, Chen Hanxiao wrote:
> > From: Chen Hanxiao <chenhanxiao at 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?
> 

Maybe we could leave this job to kernel.

> > +
> > +static bool virCgroupVersionCheck(const char * dstVersion)
> 
> Style: no space after '*' in pointer type declarations.
> 
> > +{
> > +    unsigned long currentVersion;
> > +    unsigned long version;
> > +
> > +    if (virCgroupGetKernelVersion(&currentVersion) < 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;

Thanks for your explanation.

> 
> if we still determine we need this function.
> 
> --
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org