4:38 p.m.
From: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com>
Since 2.6.39, kernel changed the minimum weight of device blkio.
We need to follow them according to different kernel version.
Chen Hanxiao (2):
[libvirt][PATCH]cgroup: introduce kernel version check function for
cgroup
[libvirt][PATCH]blkio: change the minimum weight according to kernel
version
src/util/vircgroup.c | 76 ++++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 65 insertions(+), 11 deletions(-)
--
1.8.2.1
4:38 p.m.
New subject: [libvirt] [PATCH 1/2]cgroup: introduce kernel version check function for cgroup
From: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com>
Some cgroup value range may change
in the further kernel.
Introduce kernel version check function for cgroup.
This will be helpful to determine the proper values.
Signed-off-by: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com>
---
src/util/vircgroup.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index e99caf5..498bc20 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -38,6 +38,7 @@
#include <sys/types.h>
#include <signal.h>
#include <dirent.h>
+#include <sys/utsname.h>
#define __VIR_CGROUP_ALLOW_INCLUDE_PRIV_H__
#include "vircgrouppriv.h"
@@ -74,6 +75,37 @@ typedef enum {
*/
} virCgroupFlags;
+static int virCgroupGetKernelVersion(unsigned long *kernelVersion)
+{
+ struct utsname ver;
+
+ uname(&ver);
+
+ if (virParseVersionString(ver.release, kernelVersion, true) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown release: %s"),
ver.release);
+ return -1;
+ }
+
+ return 0;
+}
+
+static bool virCgroupVersionCheck(const char * dstVersion)
+{
+ 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;
+}
+
+
#ifdef VIR_CGROUP_SUPPORTED
bool
--
1.8.2.1
11:49 a.m.
New subject: [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(a)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
2:24 p.m.
New subject: [libvirt] [PATCH 1/2]cgroup: introduce kernel version check function for cgroup
-----Original Message-----
From: Eric Blake [mailto:eblake@redhat.com]
Sent: Wednesday, October 09, 2013 11:49 AM
To: Chen Hanxiao
Cc: libvir-list(a)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(a)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(¤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;
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
4:38 p.m.
New subject: [libvirt] [PATCH 2/2]blkio: change the minimum weight according to kernel version
From: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com>
kernel had changed the minimum weight of device blkio from
100 to 10 in commit df457f845e5449be2e7d96668791f789b3770ac7.
So we should to use new blkio weight range after 2.6.39.
libvirt should follow kernel according to
kernel version.
Signed-off-by: Gao feng <gaofeng(a)cn.fujitsu.com>
Signed-off-by: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com>
---
src/util/vircgroup.c | 44 +++++++++++++++++++++++++++++++++-----------
1 file changed, 33 insertions(+), 11 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 498bc20..fa3b67e 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -57,6 +57,8 @@
#define VIR_FROM_THIS VIR_FROM_CGROUP
+#define BLKIO_WEIGHT_DEVICE_RANGE_CHANGED_VERSION "2.6.39"
+
#if defined(__linux__) && defined(HAVE_GETMNTENT_R) && \
defined(_DIRENT_HAVE_D_TYPE) && defined(_SC_CLK_TCK)
# define VIR_CGROUP_SUPPORTED
@@ -1820,11 +1822,20 @@ virCgroupPathOfController(virCgroupPtr group,
int
virCgroupSetBlkioWeight(virCgroupPtr group, unsigned int weight)
{
- if (weight > 1000 || weight < 100) {
- virReportError(VIR_ERR_INVALID_ARG,
- _("weight '%u' must be in range (100, 1000)"),
- weight);
- return -1;
+ if (virCgroupVersionCheck(BLKIO_WEIGHT_DEVICE_RANGE_CHANGED_VERSION)) {
+ if (weight > 1000 || weight < 10) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("weight '%u' must be in range (10,
1000)"),
+ weight);
+ return -1;
+ }
+ } else {
+ if (weight > 1000 || weight < 100) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("weight '%u' must be in range (100,
1000)"),
+ weight);
+ return -1;
+ }
}
return virCgroupSetValueU64(group,
@@ -1861,7 +1872,8 @@ virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight)
*
* @group: The cgroup to change io device weight device for
* @path: The device with a weight to alter
- * @weight: The new device weight (100-1000), or 0 to clear
+ * @weight: The new device weight (100-1000) or
+ * (10-1000) after kernel 2.6.39, or 0 to clear
*
* device_weight is treated as a write-only parameter, so
* there isn't a getter counterpart.
@@ -1877,11 +1889,21 @@ virCgroupSetBlkioDeviceWeight(virCgroupPtr group,
struct stat sb;
int ret;
- if (weight && (weight > 1000 || weight < 100)) {
- virReportError(VIR_ERR_INVALID_ARG,
- _("weight '%u' must be in range (100, 1000)"),
- weight);
- return -1;
+ if (virCgroupVersionCheck(BLKIO_WEIGHT_DEVICE_RANGE_CHANGED_VERSION)) {
+ if (weight &&
+ (weight > 1000 || weight < 10)) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("weight '%u' must be in range (10,
1000)"),
+ weight);
+ return -1;
+ }
+ } else {
+ if (weight && (weight > 1000 || weight < 100)) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("weight '%u' must be in range (100,
1000)"),
+ weight);
+ return -1;
+ }
}
if (stat(path, &sb) < 0) {
--
1.8.2.1
11:52 a.m.
New subject: [libvirt] [PATCH 2/2]blkio: change the minimum weight according to kernel version
On 10/08/2013 02:38 AM, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com>
kernel had changed the minimum weight of device blkio from
100 to 10 in commit df457f845e5449be2e7d96668791f789b3770ac7.
So we should to use new blkio weight range after 2.6.39.
Why not do it unconditionally, and then just have graceful error
detection when on older kernels when the input is rejected by the kernel
as out of range? That would be feature-based instead of version-check
based, which makes it more reliable even if the feature of the kernel is
backported to something that uname reports as having an older version
number.
libvirt should follow kernel according to
kernel version.
Signed-off-by: Gao feng <gaofeng(a)cn.fujitsu.com>
Signed-off-by: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com>
---
src/util/vircgroup.c | 44 +++++++++++++++++++++++++++++++++-----------
1 file changed, 33 insertions(+), 11 deletions(-)
Incomplete. You also need to touch virsh.pod and probably
formatdomain.html.in to document the larger range, and mention that
different connections may have different ranges (due to different kernel
versions on the other ends of those two connections).
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
2:30 p.m.
New subject: [libvirt] [PATCH 2/2]blkio: change the minimum weight according to kernel version
-----Original Message-----
From: Eric Blake [mailto:eblake@redhat.com]
Sent: Wednesday, October 09, 2013 11:52 AM
To: Chen Hanxiao
Cc: libvir-list(a)redhat.com
Subject: Re: [libvirt] [PATCH 2/2]blkio: change the minimum weight according to
kernel version
On 10/08/2013 02:38 AM, Chen Hanxiao wrote:
> From: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com>
>
> kernel had changed the minimum weight of device blkio from
> 100 to 10 in commit df457f845e5449be2e7d96668791f789b3770ac7.
> So we should to use new blkio weight range after 2.6.39.
Why not do it unconditionally, and then just have graceful error
detection when on older kernels when the input is rejected by the kernel
as out of range? That would be feature-based instead of version-check
based, which makes it more reliable even if the feature of the kernel is
backported to something that uname reports as having an older version
number.
How about just add the range to the doc and tell users that the range was changed.
Since the kernel will check whether the value is valid or not,
how do you think we leave this to kernel and check the errno?
That means libvirt did not check the input value ranges, send it to kernel and catch the
error.
Then we could not care how kernel would change its range.
Thanks
>
> libvirt should follow kernel according to
> kernel version.
>
> Signed-off-by: Gao feng <gaofeng(a)cn.fujitsu.com>
> Signed-off-by: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com>
> ---
> src/util/vircgroup.c | 44 +++++++++++++++++++++++++++++++++-----------
> 1 file changed, 33 insertions(+), 11 deletions(-)
Incomplete. You also need to touch virsh.pod and probably
formatdomain.html.in to document the larger range, and mention that
different connections may have different ranges (due to different kernel
versions on the other ends of those two connections).
Thanks. I will add these in the next patch.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4041
days inactive
4042
days old
6 comments
2 participants
participants (2)
-
Chen Hanxiao -
Eric Blake