Re: [libvirt] [PATCHv4 4/4] blkiotune: add qemu support for blkiotune.device_weight

On Mon, Nov 14, 2011 at 09:30:02PM -0700, Eric Blake wrote: > From: Hu Tao <hutao(a)cn.fujitsu.com&gt; > > 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; }