8:11 p.m.
On Tue, Nov 29, 2011 at 11:50:56AM -0700, Eric Blake wrote:
On 11/15/2011 12:22 AM, Hu Tao wrote:
> On Mon, Nov 14, 2011 at 09:30:02PM -0700, Eric Blake wrote:
>> From: Hu Tao <hutao(a)cn.fujitsu.com>
>>
>> Implement setting/getting per-device blkio weights in qemu,
>> using the cgroups blkio.weight_device tunable.
>
> Does more than the function name says. Would it be better to just do the
> parsing here, and set the cgroup values after parsing(see my comment to
> patch 3 for filtering out 0-weight when writing to xml):
ACK to your improvements for setting, but you forgot to do 0-weight
filtering from the getting side:
diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index a8fea6c..105bdde 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -6270,9 +6270,14 @@ static int
qemuDomainGetBlkioParameters(virDomainPtr dom,
case 1: /* blkiotune.device_weight */
if (vm->def->blkio.ndevices > 0) {
virBuffer buf = VIR_BUFFER_INITIALIZER;
+ bool comma = false;
for (j = 0; j < vm->def->blkio.ndevices; j++) {
- if (j)
+ if (!vm->def->blkio.devices[j].weight)
+ continue;
+ if (comma)
virBufferAddChar(&buf, ',');
+ else
+ comma = true;
virBufferAsprintf(&buf, "%s,%u",
vm->def->blkio.devices[j].path,
vm->def->blkio.devices[j].weight);
@@ -6282,7 +6287,8 @@ static int
qemuDomainGetBlkioParameters(virDomainPtr dom,
goto cleanup;
}
param->value.s = virBufferContentAndReset(&buf);
- } else {
+ }
+ if (!param->value.s) {
param->value.s = strdup("");
if (!param->value.s) {
virReportOOMError();
Thanks. I also found we need to do filtering at other places. Since this
is already pushed, here is another patch:
From b2009f582641b61c450744061ce34faa7a5f8eb7 Mon Sep 17 00:00:00 2001
From: Hu Tao <hutao(a)cn.fujitsu.com>
Date: Wed, 30 Nov 2011 10:06:19 +0800
Subject: [PATCH] filter blkio 0-device-weight at two oter places
filter 0-device-weight when:
- getting blkio parameters with --config
- starting up a domain
When testing with blkio, I found these issues:
(dom is down)
virsh blkiotune dom --device-weights /dev/sda,300,/dev/sdb,500
virsh blkiotune dom --device-weights /dev/sda,300,/dev/sdb,0
virsh blkiotune dom
weight : 800
device_weight : /dev/sda,200,/dev/sdb,0
# issue 1: shows 0 device weight of /dev/sdb that may confuse user
(continued)
virsh start dom
# issue 2: If /dev/sdb doesn't exist, libvirt refuses to bring the
# dom up because it wants to set the device weight to 0 of a
# non-existing device. Since 0 means no weight-limit, we really don't
# have to set it.
---
src/qemu/qemu_cgroup.c | 2 ++
src/qemu/qemu_driver.c | 5 ++++-
2 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index e5ca8f3..d663798 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -317,6 +317,8 @@ int qemuSetupCgroup(struct qemud_driver *driver,
if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) {
for (i = 0; i < vm->def->blkio.ndevices; i++) {
virBlkioDeviceWeightPtr dw = &vm->def->blkio.devices[i];
+ if (!dw->weight)
+ continue;
rc = virCgroupSetBlkioDeviceWeight(cgroup, dw->path,
dw->weight);
if (rc != 0) {
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 81d11c2..6818449 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6370,6 +6370,8 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom,
if (persistentDef->blkio.ndevices > 0) {
virBuffer buf = VIR_BUFFER_INITIALIZER;
for (j = 0; j < persistentDef->blkio.ndevices; j++) {
+ if (!persistentDef->blkio.devices[j].weight)
+ continue;
if (j)
virBufferAddChar(&buf, ',');
virBufferAsprintf(&buf, "%s,%u",
@@ -6381,7 +6383,8 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom,
goto cleanup;
}
param->value.s = virBufferContentAndReset(&buf);
- } else {
+ }
+ if (!param->value.s) {
param->value.s = strdup("");
if (!param->value.s) {
virReportOOMError();
--
1.7.3.1
--
Thanks,
Hu Tao