[libvirt] [PATCH] blkio: change the minimum weight from 100 to 10

kernel had changed the minimum weight of device blkio from 100 to 10 in commit df457f845e5449be2e7d96668791f789b3770ac7. commit df457f845e5449be2e7d96668791f789b3770ac7 Author: Justin TerAvest <teravest@google.com> Date: Tue Mar 8 19:45:00 2011 +0100 blk-cgroup: Lower minimum weight from 100 to 10. We've found that we still get good, useful isolation at weights this low. I'd like to adjust the minimum so that any other changes can take these values into account. Signed-off-by: Justin TerAvest <teravest@google.com> Acked-by: Vivek Goyal <vgoyal@redhat.com> Signed-off-by: Jens Axboe <jaxboe@fusionio.com> libvirt should comport with kernel. Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- docs/formatdomain.html.in | 4 ++-- src/util/vircgroup.c | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 83d551a..541acb3 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -766,7 +766,7 @@ defaults. <span class="since">Since 0.8.8</span></dd> <dt><code>weight</code></dt> <dd> The optional <code>weight</code> element is the overall I/O - weight of the guest. The value should be in the range [100, + weight of the guest. The value should be in the range [10, 1000].</dd> <dt><code>device</code></dt> <dd>The domain may have multiple <code>device</code> elements @@ -783,7 +783,7 @@ Each <code>device</code> element has two mandatory sub-elements, <code>path</code> describing the absolute path of the device, and <code>weight</code> giving - the relative weight of that device, in the range [100, + the relative weight of that device, in the range [10, 1000]. <span class="since">Since 0.9.8</span></dd> </dl> diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 16458a3..6e1fe6c 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1784,9 +1784,9 @@ virCgroupPathOfController(virCgroupPtr group, int virCgroupSetBlkioWeight(virCgroupPtr group, unsigned int weight) { - if (weight > 1000 || weight < 100) { + if (weight > 1000 || weight < 10) { virReportError(VIR_ERR_INVALID_ARG, - _("weight '%u' must be in range (100, 1000)"), + _("weight '%u' must be in range (10, 1000)"), weight); return -1; } @@ -1825,7 +1825,7 @@ 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 (10-1000), or 0 to clear * * device_weight is treated as a write-only parameter, so * there isn't a getter counterpart. @@ -1841,9 +1841,9 @@ virCgroupSetBlkioDeviceWeight(virCgroupPtr group, struct stat sb; int ret; - if (weight && (weight > 1000 || weight < 100)) { + if (weight && (weight > 1000 || weight < 10)) { virReportError(VIR_ERR_INVALID_ARG, - _("weight '%u' must be in range (100, 1000)"), + _("weight '%u' must be in range (10, 1000)"), weight); return -1; } -- 1.8.3.1

On 08/18/2013 11:59 PM, Gao feng wrote:
kernel had changed the minimum weight of device blkio from 100 to 10 in commit df457f845e5449be2e7d96668791f789b3770ac7.
commit df457f845e5449be2e7d96668791f789b3770ac7 Author: Justin TerAvest <teravest@google.com> Date: Tue Mar 8 19:45:00 2011 +0100
blk-cgroup: Lower minimum weight from 100 to 10.
We've found that we still get good, useful isolation at weights this low. I'd like to adjust the minimum so that any other changes can take these values into account.
Signed-off-by: Justin TerAvest <teravest@google.com> Acked-by: Vivek Goyal <vgoyal@redhat.com> Signed-off-by: Jens Axboe <jaxboe@fusionio.com>
libvirt should comport with kernel.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- docs/formatdomain.html.in | 4 ++-- src/util/vircgroup.c | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-)
What happens when running a newer libvirt with an older kernel? Or in other words, what error message do you get if you pass a limit lower than the current kernel can support? I want to make sure the message looks sane to an end-user before accepting this patch.
- if (weight && (weight > 1000 || weight < 100)) { + if (weight && (weight > 1000 || weight < 10)) { virReportError(VIR_ERR_INVALID_ARG, - _("weight '%u' must be in range (100, 1000)"), + _("weight '%u' must be in range (10, 1000)"), weight); return -1;
In other words, I suspect this code needs to be beefed up to actually probe whether the kernel accepted the change, rather than blindly doing the filter ourselves and hoping that it was correct. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 08/20/2013 01:23 AM, Eric Blake wrote:
On 08/18/2013 11:59 PM, Gao feng wrote:
kernel had changed the minimum weight of device blkio from 100 to 10 in commit df457f845e5449be2e7d96668791f789b3770ac7.
commit df457f845e5449be2e7d96668791f789b3770ac7 Author: Justin TerAvest <teravest@google.com> Date: Tue Mar 8 19:45:00 2011 +0100
blk-cgroup: Lower minimum weight from 100 to 10.
We've found that we still get good, useful isolation at weights this low. I'd like to adjust the minimum so that any other changes can take these values into account.
Signed-off-by: Justin TerAvest <teravest@google.com> Acked-by: Vivek Goyal <vgoyal@redhat.com> Signed-off-by: Jens Axboe <jaxboe@fusionio.com>
libvirt should comport with kernel.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- docs/formatdomain.html.in | 4 ++-- src/util/vircgroup.c | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-)
What happens when running a newer libvirt with an older kernel? Or in other words, what error message do you get if you pass a limit lower than the current kernel can support? I want to make sure the message looks sane to an end-user before accepting this patch.
- if (weight && (weight > 1000 || weight < 100)) { + if (weight && (weight > 1000 || weight < 10)) { virReportError(VIR_ERR_INVALID_ARG, - _("weight '%u' must be in range (100, 1000)"), + _("weight '%u' must be in range (10, 1000)"), weight); return -1;
In other words, I suspect this code needs to be beefed up to actually probe whether the kernel accepted the change, rather than blindly doing the filter ourselves and hoping that it was correct.
Hmm, I haven't considered this problem. we should report different error messages and check different min values base on different kernel. Thanks!
participants (2)
-
Eric Blake
-
Gao feng