On Tue, Nov 15, 2011 at 03:22:03PM +0800, Hu Tao wrote:
On Mon, Nov 14, 2011 at 09:30:02PM -0700, Eric Blake wrote:
From: Hu Tao <hutao@cn.fujitsu.com>
Implement setting/getting per-device blkio weights in qemu, using the cgroups blkio.weight_device tunable. --- src/libvirt_private.syms | 1 + src/qemu/qemu_cgroup.c | 20 ++ src/qemu/qemu_driver.c | 218 +++++++++++++++++++- src/util/cgroup.c | 51 +++++ src/util/cgroup.h | 4 + .../qemuxml2argv-blkiotune-device.args | 4 + tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c | 1 + 8 files changed, 295 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkiotune-device.args
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d78fd53..3575fe0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -89,6 +89,7 @@ virCgroupKillRecursive; virCgroupMounted; virCgroupPathOfController; virCgroupRemove; +virCgroupSetBlkioDeviceWeight; virCgroupSetBlkioWeight; virCgroupSetCpuShares; virCgroupSetCpuCfsPeriod; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 2a10bd2..eda4c66 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -312,6 +312,26 @@ int qemuSetupCgroup(struct qemud_driver *driver, } }
+ if (vm->def->blkio.ndevices) { + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) { + for (i = 0; i < vm->def->blkio.ndevices; i++) { + virBlkioDeviceWeightPtr dw = &vm->def->blkio.devices[i]; + rc = virCgroupSetBlkioDeviceWeight(cgroup, dw->path, + dw->weight); + if (rc != 0) { + virReportSystemError(-rc, + _("Unable to set io device weight " + "for domain %s"), + vm->def->name); + goto cleanup; + } + } + } else { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Block I/O tuning is not available on this host")); + } + } + if (vm->def->mem.hard_limit != 0 || vm->def->mem.soft_limit != 0 || vm->def->mem.swap_hard_limit != 0) { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b0ce115..43f4041 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -112,7 +112,7 @@ # define KVM_CAP_NR_VCPUS 9 /* returns max vcpus per vm */ #endif
-#define QEMU_NB_BLKIO_PARAM 1 +#define QEMU_NB_BLKIO_PARAM 2
static void processWatchdogEvent(void *data, void *opaque);
@@ -5885,6 +5885,105 @@ cleanup: return ret; }
+/* deviceWeightStr in the form of /device/path,weight,/device/path,weight + * for example, /dev/disk/by-path/pci-0000:00:1f.2-scsi-0:0:0:0,800 + */ +static int +parseBlkioWeightDeviceStr(char *deviceWeightStr, + virBlkioDeviceWeightPtr *dw, int *size, + virCgroupPtr cgroup) +{ + char *temp; + int ndevices = 0; + int nsep = 0; + int i; + virBlkioDeviceWeightPtr result = NULL; + + temp = deviceWeightStr; + while (temp) { + temp = strchr(temp, ','); + if (temp) { + temp++; + nsep++; + } + } + + /* A valid string must have even number of fields, hence an odd + * number of commas. */ + if (!(nsep & 1)) + goto error; + + ndevices = (nsep + 1) / 2; + + if (VIR_ALLOC_N(result, ndevices) < 0) { + virReportOOMError(); + return -1; + } + + i = 0; + temp = deviceWeightStr; + while (temp) { + char *p = temp; + + /* device path */ + p = strchr(p, ','); + if (!p) + goto error; + + result[i].path = strndup(temp, p - temp); + if (!result[i].path) { + virReportOOMError(); + goto cleanup; + } + + /* weight */ + temp = p + 1; + + if (virStrToLong_ui(temp, &p, 10, &result[i].weight) < 0) + goto error; + + if (cgroup) { + int rc = virCgroupSetBlkioDeviceWeight(cgroup, + result[i].path, + result[i].weight);
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):
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 43f4041..275d5c8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5890,8 +5890,7 @@ cleanup: */ static int parseBlkioWeightDeviceStr(char *deviceWeightStr, - virBlkioDeviceWeightPtr *dw, int *size, - virCgroupPtr cgroup) + virBlkioDeviceWeightPtr *dw, int *size) { char *temp; int ndevices = 0; @@ -5942,24 +5941,8 @@ parseBlkioWeightDeviceStr(char *deviceWeightStr, if (virStrToLong_ui(temp, &p, 10, &result[i].weight) < 0) goto error;
- if (cgroup) { - int rc = virCgroupSetBlkioDeviceWeight(cgroup, - result[i].path, - result[i].weight); - if (rc < 0) { - virReportSystemError(-rc, - _("Unable to set io device weight " - "for path %s"), - result[i].path); - goto cleanup; - } - } + i++;
- /* 0-weight entries only affect cgroups, they don't stick in xml */ - if (result[i].weight) - i++; - else - VIR_FREE(result[i].path); if (*p == '\0') break; else if (*p != ',') @@ -6087,7 +6070,23 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom,
if (parseBlkioWeightDeviceStr(params[i].value.s, &devices, - &ndevices, group) < 0) { + &ndevices) < 0) { + ret = -1; + continue; + } + for (i = 0; i < ndevices; i++) { + rc = virCgroupSetBlkioDeviceWeight(group, + devices[i].path, + devices[i].weight); + if (rc < 0) { + virReportSystemError(-rc, + _("Unable to set io device weight " + "for path %s"), + devices[i].path); + break; + } + } + if (i != ndevices) { ret = -1; continue; } @@ -6140,7 +6139,7 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom, } if (parseBlkioWeightDeviceStr(params[i].value.s, &devices, - &ndevices, NULL) < 0) { + &ndevices) < 0) { ret = -1; continue; }
Yes, IMHO it is better to do the setting of values, after parsing is fully complete. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|