That function takes string list and returns first string in that list that starts with the @prefix parameter with that prefix being skipped as the caller knows what it starts with (also for easier manipulation in future). Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/libvirt_private.syms | 1 + src/util/virstring.c | 17 +++++++++++++++++ src/util/virstring.h | 2 ++ 3 files changed, 20 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ff322d61083e..d22bde70e17d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2170,6 +2170,7 @@ virStrdup; virStringArrayHasString; virStringFreeList; virStringFreeListCount; +virStringGetFirstWithPrefix; virStringHasControlChars; virStringIsEmpty; virStringJoin; diff --git a/src/util/virstring.c b/src/util/virstring.c index 5794f968406e..31f77cd1a656 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -238,6 +238,23 @@ virStringArrayHasString(char **strings, const char *needle) return false; } +char * +virStringGetFirstWithPrefix(char **strings, const char *prefix) +{ + size_t i = 0; + + if (!strings) + return NULL; + + while (strings[i]) { + if (STRPREFIX(strings[i], prefix)) + return strings[i] + strlen(prefix); + i++; + } + + return NULL; +} + /* Like strtol, but produce an "int" result, and check more carefully. Return 0 upon success; return -1 to indicate failure. When END_PTR is NULL, the byte after the final valid digit must be NUL. diff --git a/src/util/virstring.h b/src/util/virstring.h index e6dcb32e998c..f65a12652961 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -47,6 +47,8 @@ void virStringFreeListCount(char **strings, size_t count); size_t virStringListLen(const char **strings); bool virStringArrayHasString(char **strings, const char *needle); +char *virStringGetFirstWithPrefix(char **strings, const char *prefix) + ATTRIBUTE_NONNULL(2); char *virArgvToString(const char *const *argv); -- 2.5.0

Since now they were not needed, but I sense they will be in a short while. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/libvirt_private.syms | 5 + src/util/vircgroup.c | 277 ++++++++++++++++++++++++++++++++++++++++++++++- src/util/vircgroup.h | 20 ++++ 3 files changed, 299 insertions(+), 3 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d22bde70e17d..8803777cde80 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1175,6 +1175,11 @@ virCgroupDenyDeviceMajor; virCgroupDenyDevicePath; virCgroupDetectMountsFromFile; virCgroupFree; +virCgroupGetBlkioDeviceReadBps; +virCgroupGetBlkioDeviceReadIops; +virCgroupGetBlkioDeviceWeight; +virCgroupGetBlkioDeviceWriteBps; +virCgroupGetBlkioDeviceWriteIops; virCgroupGetBlkioIoDeviceServiced; virCgroupGetBlkioIoServiced; virCgroupGetBlkioWeight; diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index afa85ded9061..9d527fb8b99a 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -804,6 +804,36 @@ virCgroupGetValueStr(virCgroupPtr group, static int +virCgroupGetValueForBlkDev(virCgroupPtr group, + int controller, + const char *key, + const char *path, + char **value) +{ + char *prefix = NULL; + char *str = NULL; + char **lines = NULL; + int ret = -1; + + if (virCgroupGetValueStr(group, controller, key, &str) < 0) + goto error; + + if (!(prefix = virCgroupGetBlockDevString(path))) + return -1; + + if (!(lines = virStringSplit(str, "\n", -1))) + goto error; + + ret = VIR_STRDUP(*value, virStringGetFirstWithPrefix(lines, prefix)); + error: + VIR_FREE(str); + VIR_FREE(prefix); + virStringFreeList(lines); + return ret; +} + + +static int virCgroupSetValueU64(virCgroupPtr group, int controller, const char *key, @@ -2259,9 +2289,6 @@ virCgroupSetBlkioDeviceWriteBps(virCgroupPtr group, * @weight: The new device weight (100-1000), * (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. - * * Returns: 0 on success, -1 on error */ int @@ -2289,6 +2316,196 @@ virCgroupSetBlkioDeviceWeight(virCgroupPtr group, return ret; } +/** + * virCgroupGetBlkioDeviceReadIops: + * @group: The cgroup to gather block io setting for + * @path: The path of device + * @riops: Returned device read iops throttle, 0 if there is none + * + * Returns: 0 on success, -1 on error + */ +int +virCgroupGetBlkioDeviceReadIops(virCgroupPtr group, + const char *path, + unsigned int *riops) +{ + char *str = NULL; + int ret = -1; + + if (virCgroupGetValueForBlkDev(group, + VIR_CGROUP_CONTROLLER_BLKIO, + "blkio.throttle.read_iops_device", + path, + &str) < 0) + goto error; + + if (!str) { + *riops = 0; + } else if (virStrToLong_ui(str, NULL, 10, riops) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse '%s' as an integer"), + str); + goto error; + } + + ret = 0; + error: + VIR_FREE(str); + return ret; +} + +/** + * virCgroupGetBlkioDeviceWriteIops: + * @group: The cgroup to gather block io setting for + * @path: The path of device + * @wiops: Returned device write iops throttle, 0 if there is none + * + * Returns: 0 on success, -1 on error + */ +int +virCgroupGetBlkioDeviceWriteIops(virCgroupPtr group, + const char *path, + unsigned int *wiops) +{ + char *str = NULL; + int ret = -1; + + if (virCgroupGetValueForBlkDev(group, + VIR_CGROUP_CONTROLLER_BLKIO, + "blkio.throttle.write_iops_device", + path, + &str) < 0) + goto error; + + if (!str) { + *wiops = 0; + } else if (virStrToLong_ui(str, NULL, 10, wiops) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse '%s' as an integer"), + str); + goto error; + } + + ret = 0; + error: + VIR_FREE(str); + return ret; +} + +/** + * virCgroupGetBlkioDeviceReadBps: + * @group: The cgroup to gather block io setting for + * @path: The path of device + * @rbps: Returned device read bps throttle, 0 if there is none + * + * Returns: 0 on success, -1 on error + */ +int +virCgroupGetBlkioDeviceReadBps(virCgroupPtr group, + const char *path, + unsigned long long *rbps) +{ + char *str = NULL; + int ret = -1; + + if (virCgroupGetValueForBlkDev(group, + VIR_CGROUP_CONTROLLER_BLKIO, + "blkio.throttle.read_bps_device", + path, + &str) < 0) + goto error; + + if (!str) { + *rbps = 0; + } else if (virStrToLong_ull(str, NULL, 10, rbps) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse '%s' as an integer"), + str); + goto error; + } + + ret = 0; + error: + VIR_FREE(str); + return ret; +} + +/** + * virCgroupGetBlkioDeviceWriteBps: + * @group: The cgroup to gather block io setting for + * @path: The path of device + * @wbps: Returned device write bps throttle, 0 if there is none + * + * Returns: 0 on success, -1 on error + */ +int +virCgroupGetBlkioDeviceWriteBps(virCgroupPtr group, + const char *path, + unsigned long long *wbps) +{ + char *str = NULL; + int ret = -1; + + if (virCgroupGetValueForBlkDev(group, + VIR_CGROUP_CONTROLLER_BLKIO, + "blkio.throttle.write_bps_device", + path, + &str) < 0) + goto error; + + if (!str) { + *wbps = 0; + } else if (virStrToLong_ull(str, NULL, 10, wbps) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse '%s' as an integer"), + str); + goto error; + } + + ret = 0; + error: + VIR_FREE(str); + return ret; +} + +/** + * virCgroupGetBlkioDeviceWeight: + * @group: The cgroup to gather block io setting for + * @path: The path of device + * @weight: Returned device weight, 0 if there is none + * + * Returns: 0 on success, -1 on error + */ +int +virCgroupGetBlkioDeviceWeight(virCgroupPtr group, + const char *path, + unsigned int *weight) +{ + char *str = NULL; + int ret = -1; + + if (virCgroupGetValueForBlkDev(group, + VIR_CGROUP_CONTROLLER_BLKIO, + "blkio.weight_device", + path, + &str) < 0) + goto error; + + if (!str) { + *weight = 0; + } else if (virStrToLong_ui(str, NULL, 10, weight) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse '%s' as an integer"), + str); + goto error; + } + + ret = 0; + error: + VIR_FREE(str); + return ret; +} + /** * virCgroupSetMemory: @@ -4204,6 +4421,60 @@ virCgroupSetBlkioDeviceWriteBps(virCgroupPtr group ATTRIBUTE_UNUSED, return -1; } +int +virCgroupGetBlkioDeviceWeight(virCgroupPtr group ATTRIBUTE_UNUSED, + const char *path ATTRIBUTE_UNUSED, + const char *dev_str ATTRIBUTE_UNUSED, + unsigned int weight ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", + _("Control groups not supported on this platform")); + return -1; +} + +int +virCgroupGetBlkioDeviceReadIops(virCgroupPtr group ATTRIBUTE_UNUSED, + const char *path ATTRIBUTE_UNUSED, + const char *dev_str ATTRIBUTE_UNUSED, + unsigned int riops ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", + _("Control groups not supported on this platform")); + return -1; +} + +int +virCgroupGetBlkioDeviceWriteIops(virCgroupPtr group ATTRIBUTE_UNUSED, + const char *path ATTRIBUTE_UNUSED, + const char *dev_str ATTRIBUTE_UNUSED, + unsigned int wiops ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", + _("Control groups not supported on this platform")); + return -1; +} + +int +virCgroupGetBlkioDeviceReadBps(virCgroupPtr group ATTRIBUTE_UNUSED, + const char *path ATTRIBUTE_UNUSED, + const char *dev_str ATTRIBUTE_UNUSED, + unsigned long long rbps ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", + _("Control groups not supported on this platform")); + return -1; +} + +int +virCgroupGetBlkioDeviceWriteBps(virCgroupPtr group ATTRIBUTE_UNUSED, + const char *path ATTRIBUTE_UNUSED, + const char *dev_str ATTRIBUTE_UNUSED, + unsigned long long wbps ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", + _("Control groups not supported on this platform")); + return -1; +} int virCgroupSetMemory(virCgroupPtr group ATTRIBUTE_UNUSED, diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index 675a1851a1c5..63a9e1c05a16 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -169,6 +169,26 @@ int virCgroupSetBlkioDeviceWriteBps(virCgroupPtr group, const char *path, unsigned long long wbps); +int virCgroupGetBlkioDeviceWeight(virCgroupPtr group, + const char *path, + unsigned int *weight); + +int virCgroupGetBlkioDeviceReadIops(virCgroupPtr group, + const char *path, + unsigned int *riops); + +int virCgroupGetBlkioDeviceWriteIops(virCgroupPtr group, + const char *path, + unsigned int *wiops); + +int virCgroupGetBlkioDeviceReadBps(virCgroupPtr group, + const char *path, + unsigned long long *rbps); + +int virCgroupGetBlkioDeviceWriteBps(virCgroupPtr group, + const char *path, + unsigned long long *wbps); + int virCgroupSetMemory(virCgroupPtr group, unsigned long long kb); int virCgroupGetMemoryUsage(virCgroupPtr group, unsigned long *kb); -- 2.5.0

On Tue, Aug 18, 2015 at 01:15:43PM -0400, John Ferlan wrote: Copy-paste error, thanks for catching that. That wouldn't actually do any harm, the function would return 0, but to make it as clear as possible, this is the diff that I applied before pushing: diff --git c/src/util/vircgroup.c w/src/util/vircgroup.c index 9d527fb8b99a..c94512a148b9 100644 --- c/src/util/vircgroup.c +++ w/src/util/vircgroup.c @@ -819,12 +819,15 @@ virCgroupGetValueForBlkDev(virCgroupPtr group, goto error; if (!(prefix = virCgroupGetBlockDevString(path))) - return -1; + goto error; if (!(lines = virStringSplit(str, "\n", -1))) goto error; - ret = VIR_STRDUP(*value, virStringGetFirstWithPrefix(lines, prefix)); + if (VIR_STRDUP(*value, virStringGetFirstWithPrefix(lines, prefix)) < 0) + goto error; + + ret = 0; error: VIR_FREE(str); VIR_FREE(prefix); -- Martin

The problem here is that there are some values that kernel accepts, but does not set them, for example 18446744073709551615 which acts the same way as zero. Let's do the same thing we do with other tuning options and re-read them right after they are set in order to keep our internal structures up-to-date. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1165580 Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/qemu/qemu_cgroup.c | 20 +++++++++++++++----- src/qemu/qemu_driver.c | 25 ++++++++++++++++++++----- 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index e1a28272ee7b..0da6c0238696 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -436,27 +436,37 @@ qemuSetupBlkioCgroup(virDomainObjPtr vm) virBlkioDevicePtr dev = &vm->def->blkio.devices[i]; if (dev->weight && (virCgroupSetBlkioDeviceWeight(priv->cgroup, dev->path, - dev->weight) < 0)) + dev->weight) < 0 || + virCgroupGetBlkioDeviceWeight(priv->cgroup, dev->path, + &dev->weight) < 0)) return -1; if (dev->riops && (virCgroupSetBlkioDeviceReadIops(priv->cgroup, dev->path, - dev->riops) < 0)) + dev->riops) < 0 || + virCgroupGetBlkioDeviceReadIops(priv->cgroup, dev->path, + &dev->riops) < 0)) return -1; if (dev->wiops && (virCgroupSetBlkioDeviceWriteIops(priv->cgroup, dev->path, - dev->wiops) < 0)) + dev->wiops) < 0 || + virCgroupGetBlkioDeviceWriteIops(priv->cgroup, dev->path, + &dev->wiops) < 0)) return -1; if (dev->rbps && (virCgroupSetBlkioDeviceReadBps(priv->cgroup, dev->path, - dev->rbps) < 0)) + dev->rbps) < 0 || + virCgroupGetBlkioDeviceReadBps(priv->cgroup, dev->path, + &dev->rbps) < 0)) return -1; if (dev->wbps && (virCgroupSetBlkioDeviceWriteBps(priv->cgroup, dev->path, - dev->wbps) < 0)) + dev->wbps) < 0 || + virCgroupGetBlkioDeviceWriteBps(priv->cgroup, dev->path, + &dev->wbps) < 0)) return -1; } } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7a984a86ab3a..aa3e05e4a258 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9191,7 +9191,10 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, for (j = 0; j < ndevices; j++) { if (virCgroupSetBlkioDeviceWeight(priv->cgroup, devices[j].path, - devices[j].weight) < 0) { + devices[j].weight) < 0 || + virCgroupGetBlkioDeviceWeight(priv->cgroup, + devices[j].path, + &devices[j].weight) < 0) { ret = -1; break; } @@ -9200,7 +9203,10 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, for (j = 0; j < ndevices; j++) { if (virCgroupSetBlkioDeviceReadIops(priv->cgroup, devices[j].path, - devices[j].riops) < 0) { + devices[j].riops) < 0 || + virCgroupGetBlkioDeviceReadIops(priv->cgroup, + devices[j].path, + &devices[j].riops) < 0) { ret = -1; break; } @@ -9209,7 +9215,10 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, for (j = 0; j < ndevices; j++) { if (virCgroupSetBlkioDeviceWriteIops(priv->cgroup, devices[j].path, - devices[j].wiops) < 0) { + devices[j].wiops) < 0 || + virCgroupGetBlkioDeviceWriteIops(priv->cgroup, + devices[j].path, + &devices[j].wiops) < 0) { ret = -1; break; } @@ -9218,7 +9227,10 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, for (j = 0; j < ndevices; j++) { if (virCgroupSetBlkioDeviceReadBps(priv->cgroup, devices[j].path, - devices[j].rbps) < 0) { + devices[j].rbps) < 0 || + virCgroupGetBlkioDeviceReadBps(priv->cgroup, + devices[j].path, + &devices[j].rbps) < 0) { ret = -1; break; } @@ -9227,7 +9239,10 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, for (j = 0; j < ndevices; j++) { if (virCgroupSetBlkioDeviceWriteBps(priv->cgroup, devices[j].path, - devices[j].wbps) < 0) { + devices[j].wbps) < 0 || + virCgroupGetBlkioDeviceWriteBps(priv->cgroup, + devices[j].path, + &devices[j].wbps) < 0) { ret = -1; break; } -- 2.5.0