[libvirt] [PATCH 0/7] Last CAT fixups
Few finishing touches for the Cache Allocation Technology. This series copies the default group's mask to newly created groups, fixes a possible wrong memory access, cleans up after domains even if daemon was restarted, and then reorders the functions in virresctrl.c so that it's nice to find the non-Linux stubs and Linux-only functions. And the last thing is just one variable rename that was forgotten before. Martin Kletzander (8): util: Add helpers for getting resctrl group allocs util: Use default group's mask for unspecified resctrl allocations util: Don't overwrite mask in virResctrlAllocFindUnused qemu: Restore machinename even without cgroups util: Extract path formatting into virResctrlAllocDeterminePath qemu: Restore resctrl alloc data after restart util: Reorder functions in virresctrl to minimize stubs util: Rename resctrl to alloc in virResctrlAllocUpdateMask to be consistent src/libvirt_linux.syms | 3 + src/libvirt_private.syms | 2 +- src/qemu/qemu_cgroup.c | 4 - src/qemu/qemu_process.c | 11 + src/util/virresctrl.c | 1620 +++++++++++++++++++++++----------------------- src/util/virresctrl.h | 4 + 6 files changed, 843 insertions(+), 801 deletions(-) -- 2.16.1
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/util/virresctrl.c | 79 +++++++++++++++++++++++++++++++++------------------ 1 file changed, 52 insertions(+), 27 deletions(-) diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index 42876c6e2b9b..fa7c28e9b45e 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -1041,6 +1041,53 @@ virResctrlAllocParse(virResctrlInfoPtr resctrl, } +static int +virResctrlAllocGetGroup(virResctrlInfoPtr resctrl, + const char *groupname, + virResctrlAllocPtr *alloc) +{ + char *schemata = NULL; + int rv = virFileReadValueString(&schemata, + SYSFS_RESCTRL_PATH + "/%s/schemata", + groupname); + + if (rv < 0) + return rv; + + *alloc = virResctrlAllocNew(); + if (!*alloc) + goto error; + + if (virResctrlAllocParse(resctrl, *alloc, schemata) < 0) + goto error; + + VIR_FREE(schemata); + return 0; + + error: + VIR_FREE(schemata); + virObjectUnref(*alloc); + *alloc = NULL; + return -1; +} + + +static virResctrlAllocPtr +virResctrlAllocGetDefault(virResctrlInfoPtr resctrl) +{ + virResctrlAllocPtr ret = NULL; + + if (virResctrlAllocGetGroup(resctrl, ".", &ret) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not read schemata file for the default group")); + return NULL; + } + + return ret; +} + + static void virResctrlAllocSubtractPerType(virResctrlAllocPerTypePtr dst, virResctrlAllocPerTypePtr src) @@ -1141,7 +1188,6 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl) virResctrlAllocPtr alloc = NULL; struct dirent *ent = NULL; DIR *dirp = NULL; - char *schemata = NULL; int rv = -1; if (virResctrlInfoIsEmpty(resctrl)) { @@ -1154,22 +1200,12 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl) if (!ret) return NULL; - if (virFileReadValueString(&schemata, - SYSFS_RESCTRL_PATH - "/schemata") < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not read schemata file for the default group")); - goto error; - } - - alloc = virResctrlAllocNew(); + alloc = virResctrlAllocGetDefault(resctrl); if (!alloc) goto error; - if (virResctrlAllocParse(resctrl, alloc, schemata) < 0) - goto error; - virResctrlAllocSubtract(ret, alloc); + virObjectUnref(alloc); if (virDirOpen(&dirp, SYSFS_RESCTRL_PATH) < 0) goto error; @@ -1178,11 +1214,7 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl) if (STREQ(ent->d_name, "info")) continue; - VIR_FREE(schemata); - rv = virFileReadValueString(&schemata, - SYSFS_RESCTRL_PATH - "/%s/schemata", - ent->d_name); + rv = virResctrlAllocGetGroup(resctrl, ent->d_name, &alloc); if (rv == -2) continue; @@ -1193,15 +1225,9 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl) goto error; } - virObjectUnref(alloc); - alloc = virResctrlAllocNew(); - if (!alloc) - goto error; - - if (virResctrlAllocParse(resctrl, alloc, schemata) < 0) - goto error; - virResctrlAllocSubtract(ret, alloc); + virObjectUnref(alloc); + alloc = NULL; } if (rv < 0) goto error; @@ -1209,7 +1235,6 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl) cleanup: virObjectUnref(alloc); VIR_DIR_CLOSE(dirp); - VIR_FREE(schemata); return ret; error: -- 2.16.1
On 01/31/2018 10:41 AM, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/util/virresctrl.c | 79 +++++++++++++++++++++++++++++++++------------------ 1 file changed, 52 insertions(+), 27 deletions(-)
diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index 42876c6e2b9b..fa7c28e9b45e 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -1041,6 +1041,53 @@ virResctrlAllocParse(virResctrlInfoPtr resctrl, }
+static int +virResctrlAllocGetGroup(virResctrlInfoPtr resctrl, + const char *groupname, + virResctrlAllocPtr *alloc) +{ + char *schemata = NULL; + int rv = virFileReadValueString(&schemata, + SYSFS_RESCTRL_PATH + "/%s/schemata", + groupname); + + if (rv < 0) + return rv;
I guess we need to set *alloc = NULL before returning. Because with this patch the function may or may not do that when returning an error. But it's just cosmetics.
+ + *alloc = virResctrlAllocNew(); + if (!*alloc) + goto error; + + if (virResctrlAllocParse(resctrl, *alloc, schemata) < 0) + goto error; + + VIR_FREE(schemata); + return 0; + + error: + VIR_FREE(schemata); + virObjectUnref(*alloc); + *alloc = NULL; + return -1; +} + + +static virResctrlAllocPtr +virResctrlAllocGetDefault(virResctrlInfoPtr resctrl) +{ + virResctrlAllocPtr ret = NULL; + + if (virResctrlAllocGetGroup(resctrl, ".", &ret) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not read schemata file for the default group")); + return NULL;
No need to report an error. virResctrlAllocGetGroup() calls functions which report error on failure. But this is more user friendly error message than 'unable to open /sys/blah: Bad address'. Your call.
+ } + + return ret; +} + + static void virResctrlAllocSubtractPerType(virResctrlAllocPerTypePtr dst, virResctrlAllocPerTypePtr src) @@ -1141,7 +1188,6 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl) virResctrlAllocPtr alloc = NULL; struct dirent *ent = NULL; DIR *dirp = NULL; - char *schemata = NULL; int rv = -1;
if (virResctrlInfoIsEmpty(resctrl)) { @@ -1154,22 +1200,12 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl) if (!ret) return NULL;
- if (virFileReadValueString(&schemata, - SYSFS_RESCTRL_PATH - "/schemata") < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not read schemata file for the default group")); - goto error; - } - - alloc = virResctrlAllocNew(); + alloc = virResctrlAllocGetDefault(resctrl); if (!alloc) goto error;
- if (virResctrlAllocParse(resctrl, alloc, schemata) < 0) - goto error; - virResctrlAllocSubtract(ret, alloc); + virObjectUnref(alloc);
if (virDirOpen(&dirp, SYSFS_RESCTRL_PATH) < 0) goto error; @@ -1178,11 +1214,7 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl) if (STREQ(ent->d_name, "info")) continue;
- VIR_FREE(schemata); - rv = virFileReadValueString(&schemata, - SYSFS_RESCTRL_PATH - "/%s/schemata", - ent->d_name); + rv = virResctrlAllocGetGroup(resctrl, ent->d_name, &alloc); if (rv == -2) continue;
Unfortunately, not visible in the context but the same 'error message overwrite' problem.
@@ -1193,15 +1225,9 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl) goto error; }
- virObjectUnref(alloc); - alloc = virResctrlAllocNew(); - if (!alloc) - goto error; - - if (virResctrlAllocParse(resctrl, alloc, schemata) < 0) - goto error; - virResctrlAllocSubtract(ret, alloc); + virObjectUnref(alloc); + alloc = NULL; } if (rv < 0) goto error;
Michal
Introduce virResctrlAllocCopyMasks() and use that to initially copy the default group schemata to the allocation before reserving any parts of the cache. The reason for this is that when new group is created the schemata will have unknown data in it. If there was previously group with the same CLoS ID, it will have the previous valies, if not it will have all bits set. And we need to set all unspecified (in the XML) allocations to the same one as the default group. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1289368 Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/util/virresctrl.c | 64 ++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 48 insertions(+), 16 deletions(-) diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index fa7c28e9b45e..2bdca5455783 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -1296,23 +1296,8 @@ virResctrlAllocFindUnused(virResctrlAllocPerTypePtr a_type, ssize_t last_bits = 0; ssize_t last_pos = -1; - /* If there is no reservation requested we need to set all bits. That's due - * to weird interface of the resctrl sysfs. It's also the reason why we - * cannot reserve the whole cache in one allocation. */ - if (!size) { - a_mask = virBitmapNew(i_type->bits); - if (!a_mask) - return -1; - - virBitmapSetAll(a_mask); - - if (virResctrlAllocSetMask(a_type, cache, a_mask) < 0) { - virBitmapFree(a_mask); - return -1; - } - + if (!size) return 0; - } if (cache >= f_type->nmasks) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -1415,6 +1400,44 @@ virResctrlAllocFindUnused(virResctrlAllocPerTypePtr a_type, } +static int +virResctrlAllocCopyMasks(virResctrlAllocPtr dst, + virResctrlAllocPtr src) +{ + unsigned int level = 0; + + for (level = 0; level < src->nlevels; level++) { + virResctrlAllocPerLevelPtr s_level = src->levels[level]; + unsigned int type = 0; + + if (!s_level) + continue; + + for (type = 0; type < VIR_CACHE_TYPE_LAST; type++) { + virResctrlAllocPerTypePtr s_type = s_level->types[type]; + virResctrlAllocPerTypePtr d_type = NULL; + unsigned int cache = 0; + + if (!s_type) + continue; + + d_type = virResctrlAllocGetType(dst, level, type); + if (!d_type) + return -1; + + for (cache = 0; cache < s_type->nmasks; cache++) { + virBitmapPtr mask = s_type->masks[cache]; + + if (mask && virResctrlAllocUpdateMask(dst, level, type, cache, mask) < 0) + return -1; + } + } + } + + return 0; +} + + /* * This function is called when creating an allocation in the system. What it * does is that it gets all the unused bits using virResctrlAllocGetUnused() and @@ -1428,11 +1451,19 @@ virResctrlAllocMasksAssign(virResctrlInfoPtr resctrl, int ret = -1; unsigned int level = 0; virResctrlAllocPtr alloc_free = NULL; + virResctrlAllocPtr alloc_default = NULL; alloc_free = virResctrlAllocGetUnused(resctrl); if (!alloc_free) return -1; + alloc_default = virResctrlAllocGetDefault(resctrl); + if (!alloc_default) + return -1; + + if (virResctrlAllocCopyMasks(alloc, alloc_default) < 0) + return -1; + for (level = 0; level < alloc->nlevels; level++) { virResctrlAllocPerLevelPtr a_level = alloc->levels[level]; virResctrlAllocPerLevelPtr f_level = NULL; @@ -1480,6 +1511,7 @@ virResctrlAllocMasksAssign(virResctrlInfoPtr resctrl, ret = 0; cleanup: virObjectUnref(alloc_free); + virObjectUnref(alloc_default); return ret; } -- 2.16.1
Due to confusing naming the pointer to the mask got copied which must not happen, so use UpdateMask instead of SetMask. That also means we can get completely rid of SetMask. Also don't clear the free bits since it is not used again (leftover from previous versions). Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/util/virresctrl.c | 41 ++++++++++++++--------------------------- 1 file changed, 14 insertions(+), 27 deletions(-) diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index 2bdca5455783..14a3c459ffd3 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -1255,22 +1255,6 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl ATTRIBUTE_UNUSED) #endif /* ! __linux__ */ -static int -virResctrlAllocSetMask(virResctrlAllocPerTypePtr a_type, - unsigned int cache, - virBitmapPtr mask) -{ - if (a_type->nmasks <= cache && - VIR_EXPAND_N(a_type->masks, a_type->nmasks, - cache - a_type->nmasks + 1) < 0) - return -1; - - virBitmapFree(a_type->masks[cache]); - a_type->masks[cache] = mask; - - return 0; -} - /* * Given the information about requested allocation type `a_type`, the host @@ -1278,16 +1262,19 @@ virResctrlAllocSetMask(virResctrlAllocPerTypePtr a_type, * this function tries to find the smallest free space in which the allocation * for cache id `cache` would fit. We're looking for the smallest place in * order to minimize fragmentation and maximize the possibility of succeeding. + * + * Per-cache allocation for the @level, @type and @cache must already be + * allocated for @alloc (does not have to exist though). */ static int -virResctrlAllocFindUnused(virResctrlAllocPerTypePtr a_type, +virResctrlAllocFindUnused(virResctrlAllocPtr alloc, virResctrlInfoPerTypePtr i_type, virResctrlAllocPerTypePtr f_type, unsigned int level, unsigned int type, unsigned int cache) { - unsigned long long *size = a_type->sizes[cache]; + unsigned long long *size = alloc->levels[level]->types[type]->sizes[cache]; virBitmapPtr a_mask = NULL; virBitmapPtr f_mask = NULL; unsigned long long need_bits; @@ -1295,6 +1282,7 @@ virResctrlAllocFindUnused(virResctrlAllocPerTypePtr a_type, ssize_t pos = -1; ssize_t last_bits = 0; ssize_t last_pos = -1; + int ret = -1; if (!size) return 0; @@ -1386,17 +1374,16 @@ virResctrlAllocFindUnused(virResctrlAllocPerTypePtr a_type, if (!a_mask) return -1; - for (i = last_pos; i < last_pos + need_bits; i++) { + for (i = last_pos; i < last_pos + need_bits; i++) ignore_value(virBitmapSetBit(a_mask, i)); - ignore_value(virBitmapClearBit(f_mask, i)); - } - if (virResctrlAllocSetMask(a_type, cache, a_mask) < 0) { - virBitmapFree(a_mask); - return -1; - } + if (virResctrlAllocUpdateMask(alloc, level, type, cache, a_mask) < 0) + goto cleanup; - return 0; + ret = 0; + cleanup: + virBitmapFree(a_mask); + return ret; } @@ -1502,7 +1489,7 @@ virResctrlAllocMasksAssign(virResctrlInfoPtr resctrl, virResctrlInfoPerLevelPtr i_level = resctrl->levels[level]; virResctrlInfoPerTypePtr i_type = i_level->types[type]; - if (virResctrlAllocFindUnused(a_type, i_type, f_type, level, type, cache) < 0) + if (virResctrlAllocFindUnused(alloc, i_type, f_type, level, type, cache) < 0) goto cleanup; } } -- 2.16.1
On Wed, Jan 31, 2018 at 10:41:41AM +0100, Martin Kletzander wrote:
Due to confusing naming the pointer to the mask got copied which must not happen, so use UpdateMask instead of SetMask. That also means we can get completely rid of SetMask.
Also don't clear the free bits since it is not used again (leftover from previous versions).
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/util/virresctrl.c | 41 ++++++++++++++--------------------------- 1 file changed, 14 insertions(+), 27 deletions(-)
After the last rebase I'm still hitting an issue on non-Linux, I'l send a v2, sorry for the noise.
The virresctrl will use this as well and we need to have that info after restart to properly clean up /sys/fs/resctrl. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_cgroup.c | 4 ---- src/qemu/qemu_process.c | 4 ++++ 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index fbf79a6d8f33..b604edb31c0d 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -986,10 +986,6 @@ qemuConnectCgroup(virDomainObjPtr vm) if (!virCgroupAvailable()) goto done; - priv->machineName = qemuDomainGetMachineName(vm); - if (!priv->machineName) - goto cleanup; - virCgroupFree(&priv->cgroup); if (virCgroupNewDetectMachine(vm->def->name, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 0577f4c35d08..239798fa5d7c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7198,6 +7198,10 @@ qemuProcessReconnect(void *opaque) if (qemuHostdevUpdateActiveDomainDevices(driver, obj->def) < 0) goto error; + priv->machineName = qemuDomainGetMachineName(obj); + if (!priv->machineName) + goto error; + if (qemuConnectCgroup(obj) < 0) goto error; -- 2.16.1
We can use this from more places later, so just a future code de-duplication. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virresctrl.c | 29 ++++++++++++++++++++--------- src/util/virresctrl.h | 4 ++++ 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 792fb60568bd..9339c2c3259d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2560,6 +2560,7 @@ virCacheTypeFromString; virCacheTypeToString; virResctrlAllocAddPID; virResctrlAllocCreate; +virResctrlAllocDeterminePath; virResctrlAllocForeachSize; virResctrlAllocFormat; virResctrlAllocGetID; diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index 14a3c459ffd3..fefca32a6710 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -1503,6 +1503,25 @@ virResctrlAllocMasksAssign(virResctrlInfoPtr resctrl, } +int +virResctrlAllocDeterminePath(virResctrlAllocPtr alloc, + const char *machinename) +{ + if (!alloc->id) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Resctrl Allocation ID must be set before creation")); + return -1; + } + + if (!alloc->path && + virAsprintf(&alloc->path, "%s/%s-%s", + SYSFS_RESCTRL_PATH, machinename, alloc->id) < 0) + return -1; + + return 0; +} + + /* This checks if the directory for the alloc exists. If not it tries to create * it and apply appropriate alloc settings. */ int @@ -1524,15 +1543,7 @@ virResctrlAllocCreate(virResctrlInfoPtr resctrl, return -1; } - if (!alloc->id) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Resctrl Allocation ID must be set before creation")); - return -1; - } - - if (!alloc->path && - virAsprintf(&alloc->path, "%s/%s-%s", - SYSFS_RESCTRL_PATH, machinename, alloc->id) < 0) + if (virResctrlAllocDeterminePath(alloc, machinename) < 0) return -1; if (virFileExists(alloc->path)) { diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h index 63a954feaad1..5368ba211924 100644 --- a/src/util/virresctrl.h +++ b/src/util/virresctrl.h @@ -102,6 +102,10 @@ virResctrlAllocGetID(virResctrlAllocPtr alloc); char * virResctrlAllocFormat(virResctrlAllocPtr alloc); +int +virResctrlAllocDeterminePath(virResctrlAllocPtr alloc, + const char *machinename); + int virResctrlAllocCreate(virResctrlInfoPtr r_info, virResctrlAllocPtr alloc, -- 2.16.1
During reconnect we need to reconstruct the paths of all cachetunes so that they get cleaned up when the domain is stopped. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_process.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 239798fa5d7c..bcd4ac8ad699 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -77,6 +77,7 @@ #include "configmake.h" #include "nwfilter_conf.h" #include "netdev_bandwidth_conf.h" +#include "virresctrl.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -7332,6 +7333,12 @@ qemuProcessReconnect(void *opaque) if (qemuConnectAgent(driver, obj) < 0) goto error; + for (i = 0; i < obj->def->ncachetunes; i++) { + if (virResctrlAllocDeterminePath(obj->def->cachetunes[i]->alloc, + priv->machineName) < 0) + goto error; + } + /* update domain state XML with possibly updated state in virDomainObj */ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, obj, driver->caps) < 0) goto error; -- 2.16.1
If we reorder the functions properly, we only need to have few stub functions for non-Linux platforms and it is also nicer to look at and read if they are in one place with only one preprocessor condition. The order after this patch is: - Class allocation and Static functions (used by everything, static dependencies) - Linux-only functions - Non-Linux stubs - Exported functions (as they rely on all previous functions) Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/libvirt_linux.syms | 3 + src/libvirt_private.syms | 1 - src/util/virresctrl.c | 939 +++++++++++++++++++++++------------------------ 3 files changed, 459 insertions(+), 484 deletions(-) diff --git a/src/libvirt_linux.syms b/src/libvirt_linux.syms index 5fa2c790efc1..27425e4bbebb 100644 --- a/src/libvirt_linux.syms +++ b/src/libvirt_linux.syms @@ -9,6 +9,9 @@ virHostCPUGetSiblingsList; virHostCPUGetSocket; virHostCPUGetStatsLinux; +# util/virresctrl.h +virResctrlAllocGetUnused; + # Let emacs know we want case-insensitive sorting # Local Variables: # sort-fold-case: t diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9339c2c3259d..21ec8d8c8c34 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2564,7 +2564,6 @@ virResctrlAllocDeterminePath; virResctrlAllocForeachSize; virResctrlAllocFormat; virResctrlAllocGetID; -virResctrlAllocGetUnused; virResctrlAllocNew; virResctrlAllocRemove; virResctrlAllocSetID; diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index fefca32a6710..cab7570f7b8e 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -294,346 +294,6 @@ virResctrlAllocNew(void) } -/* Common functions */ -#ifdef __linux__ -static int -virResctrlLockInternal(int op) -{ - int fd = open(SYSFS_RESCTRL_PATH, O_DIRECTORY | O_CLOEXEC); - - if (fd < 0) { - virReportSystemError(errno, "%s", _("Cannot open resctrl")); - return -1; - } - - if (flock(fd, op) < 0) { - virReportSystemError(errno, "%s", _("Cannot lock resctrl")); - VIR_FORCE_CLOSE(fd); - return -1; - } - - return fd; -} - - -static inline int -virResctrlLockWrite(void) -{ - return virResctrlLockInternal(LOCK_EX); -} - -#else - -static inline int -virResctrlLockWrite(void) -{ - virReportSystemError(ENOSYS, "%s", - _("resctrl not supported on this platform")); - return -1; -} - -#endif - - - - -static int -virResctrlUnlock(int fd) -{ - if (fd == -1) - return 0; - -#ifdef __linux__ - /* The lock gets unlocked by closing the fd, which we need to do anyway in - * order to clean up properly */ - if (VIR_CLOSE(fd) < 0) { - virReportSystemError(errno, "%s", _("Cannot close resctrl")); - - /* Trying to save the already broken */ - if (flock(fd, LOCK_UN) < 0) - virReportSystemError(errno, "%s", _("Cannot unlock resctrl")); - return -1; - } -#endif /* ! __linux__ */ - - return 0; -} - - -/* Info-related functions */ -static bool -virResctrlInfoIsEmpty(virResctrlInfoPtr resctrl) -{ - size_t i = 0; - size_t j = 0; - - if (!resctrl) - return true; - - for (i = 0; i < resctrl->nlevels; i++) { - virResctrlInfoPerLevelPtr i_level = resctrl->levels[i]; - - if (!i_level) - continue; - - for (j = 0; j < VIR_CACHE_TYPE_LAST; j++) { - if (i_level->types[j]) - return false; - } - } - - return true; -} - - -#ifdef __linux__ - -int -virResctrlGetInfo(virResctrlInfoPtr resctrl) -{ - DIR *dirp = NULL; - char *endptr = NULL; - char *tmp_str = NULL; - int ret = -1; - int rv = -1; - int type = 0; - struct dirent *ent = NULL; - unsigned int level = 0; - virResctrlInfoPerLevelPtr i_level = NULL; - virResctrlInfoPerTypePtr i_type = NULL; - - rv = virDirOpenIfExists(&dirp, SYSFS_RESCTRL_PATH "/info"); - if (rv <= 0) { - ret = rv; - goto cleanup; - } - - while ((rv = virDirRead(dirp, &ent, SYSFS_RESCTRL_PATH "/info")) > 0) { - VIR_DEBUG("Parsing info type '%s'", ent->d_name); - if (ent->d_name[0] != 'L') - continue; - - if (virStrToLong_uip(ent->d_name + 1, &endptr, 10, &level) < 0) { - VIR_DEBUG("Cannot parse resctrl cache info level '%s'", ent->d_name + 1); - continue; - } - - type = virResctrlTypeFromString(endptr); - if (type < 0) { - VIR_DEBUG("Cannot parse resctrl cache info type '%s'", endptr); - continue; - } - - if (VIR_ALLOC(i_type) < 0) - goto cleanup; - - i_type->control.scope = type; - - rv = virFileReadValueUint(&i_type->control.max_allocation, - SYSFS_RESCTRL_PATH "/info/%s/num_closids", - ent->d_name); - if (rv == -2) { - /* The file doesn't exist, so it's unusable for us, - * but we can scan further */ - VIR_WARN("The path '" SYSFS_RESCTRL_PATH "/info/%s/num_closids' " - "does not exist", - ent->d_name); - } else if (rv < 0) { - /* Other failures are fatal, so just quit */ - goto cleanup; - } - - rv = virFileReadValueString(&i_type->cbm_mask, - SYSFS_RESCTRL_PATH - "/info/%s/cbm_mask", - ent->d_name); - if (rv == -2) { - /* If the previous file exists, so should this one. Hence -2 is - * fatal in this case as well (errors out in next condition) - the - * kernel interface might've changed too much or something else is - * wrong. */ - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot get cbm_mask from resctrl cache info")); - } - if (rv < 0) - goto cleanup; - - virStringTrimOptionalNewline(i_type->cbm_mask); - - rv = virFileReadValueUint(&i_type->min_cbm_bits, - SYSFS_RESCTRL_PATH "/info/%s/min_cbm_bits", - ent->d_name); - if (rv == -2) - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot get min_cbm_bits from resctrl cache info")); - if (rv < 0) - goto cleanup; - - if (resctrl->nlevels <= level && - VIR_EXPAND_N(resctrl->levels, resctrl->nlevels, - level - resctrl->nlevels + 1) < 0) - goto cleanup; - - if (!resctrl->levels[level]) { - virResctrlInfoPerTypePtr *types = NULL; - - if (VIR_ALLOC_N(types, VIR_CACHE_TYPE_LAST) < 0) - goto cleanup; - - if (VIR_ALLOC(resctrl->levels[level]) < 0) { - VIR_FREE(types); - goto cleanup; - } - resctrl->levels[level]->types = types; - } - - i_level = resctrl->levels[level]; - - if (i_level->types[type]) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Duplicate cache type in resctrl for level %u"), - level); - goto cleanup; - } - - for (tmp_str = i_type->cbm_mask; *tmp_str != '\0'; tmp_str++) { - if (!c_isxdigit(*tmp_str)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse cbm_mask from resctrl cache info")); - goto cleanup; - } - - i_type->bits += count_one_bits(virHexToBin(*tmp_str)); - } - - VIR_STEAL_PTR(i_level->types[type], i_type); - } - - ret = 0; - cleanup: - VIR_DIR_CLOSE(dirp); - if (i_type) - VIR_FREE(i_type->cbm_mask); - VIR_FREE(i_type); - return ret; -} - -#else /* ! __linux__ */ - -int -virResctrlGetInfo(virResctrlInfoPtr resctrl ATTRIBUTE_UNUSED) -{ - virReportSystemError(ENOSYS, "%s", - _("Cache tune not supported on this platform")); - return -1; -} - -#endif /* ! __linux__ */ - - -int -virResctrlInfoGetCache(virResctrlInfoPtr resctrl, - unsigned int level, - unsigned long long size, - size_t *ncontrols, - virResctrlInfoPerCachePtr **controls) -{ - virResctrlInfoPerLevelPtr i_level = NULL; - virResctrlInfoPerTypePtr i_type = NULL; - size_t i = 0; - int ret = -1; - - if (virResctrlInfoIsEmpty(resctrl)) - return 0; - - if (level >= resctrl->nlevels) - return 0; - - i_level = resctrl->levels[level]; - if (!i_level) - return 0; - - for (i = 0; i < VIR_CACHE_TYPE_LAST; i++) { - i_type = i_level->types[i]; - if (!i_type) - continue; - - /* Let's take the opportunity to update our internal information about - * the cache size */ - if (!i_type->size) { - i_type->size = size; - i_type->control.granularity = size / i_type->bits; - if (i_type->min_cbm_bits != 1) - i_type->control.min = i_type->min_cbm_bits * i_type->control.granularity; - } else { - if (i_type->size != size) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("level %u cache size %llu does not match " - "expected size %llu"), - level, i_type->size, size); - goto error; - } - i_type->max_cache_id++; - } - - if (VIR_EXPAND_N(*controls, *ncontrols, 1) < 0) - goto error; - if (VIR_ALLOC((*controls)[*ncontrols - 1]) < 0) - goto error; - - memcpy((*controls)[*ncontrols - 1], &i_type->control, sizeof(i_type->control)); - } - - ret = 0; - cleanup: - return ret; - error: - while (*ncontrols) - VIR_FREE((*controls)[--*ncontrols]); - VIR_FREE(*controls); - goto cleanup; -} - - -/* Alloc-related functions */ -bool -virResctrlAllocIsEmpty(virResctrlAllocPtr resctrl) -{ - size_t i = 0; - size_t j = 0; - size_t k = 0; - - if (!resctrl) - return true; - - for (i = 0; i < resctrl->nlevels; i++) { - virResctrlAllocPerLevelPtr a_level = resctrl->levels[i]; - - if (!a_level) - continue; - - for (j = 0; j < VIR_CACHE_TYPE_LAST; j++) { - virResctrlAllocPerTypePtr a_type = a_level->types[j]; - - if (!a_type) - continue; - - for (k = 0; k < a_type->nsizes; k++) { - if (a_type->sizes[k]) - return false; - } - - for (k = 0; k < a_type->nmasks; k++) { - if (a_type->masks[k]) - return false; - } - } - } - - return true; -} - - static virResctrlAllocPerTypePtr virResctrlAllocGetType(virResctrlAllocPtr resctrl, unsigned int level, @@ -667,38 +327,6 @@ virResctrlAllocGetType(virResctrlAllocPtr resctrl, } -#ifdef __linux__ - -static int -virResctrlAllocUpdateMask(virResctrlAllocPtr resctrl, - unsigned int level, - virCacheType type, - unsigned int cache, - virBitmapPtr mask) -{ - virResctrlAllocPerTypePtr a_type = virResctrlAllocGetType(resctrl, level, type); - - if (!a_type) - return -1; - - if (a_type->nmasks <= cache && - VIR_EXPAND_N(a_type->masks, a_type->nmasks, - cache - a_type->nmasks + 1) < 0) - return -1; - - if (!a_type->masks[cache]) { - a_type->masks[cache] = virBitmapNew(virBitmapSize(mask)); - - if (!a_type->masks[cache]) - return -1; - } - - return virBitmapCopy(a_type->masks[cache], mask); -} - -#endif - - static int virResctrlAllocUpdateSize(virResctrlAllocPtr resctrl, unsigned int level, @@ -725,6 +353,31 @@ virResctrlAllocUpdateSize(virResctrlAllocPtr resctrl, } +static bool +virResctrlInfoIsEmpty(virResctrlInfoPtr resctrl) +{ + size_t i = 0; + size_t j = 0; + + if (!resctrl) + return true; + + for (i = 0; i < resctrl->nlevels; i++) { + virResctrlInfoPerLevelPtr i_level = resctrl->levels[i]; + + if (!i_level) + continue; + + for (j = 0; j < VIR_CACHE_TYPE_LAST; j++) { + if (i_level->types[j]) + return false; + } + } + + return true; +} + + /* * Check if there is an allocation for this level/type/cache already. Called * before updating the structure. VIR_CACHE_TYPE_BOTH collides with any type, @@ -775,150 +428,223 @@ virResctrlAllocCheckCollision(virResctrlAllocPtr alloc, } else { a_type = a_level->types[type]; - if (a_type && a_type->nsizes > cache && a_type->sizes[cache]) - return true; + if (a_type && a_type->nsizes > cache && a_type->sizes[cache]) + return true; + } + + return false; +} + + +#ifdef __linux__ + +static int +virResctrlLockInternal(int op) +{ + int fd = open(SYSFS_RESCTRL_PATH, O_DIRECTORY | O_CLOEXEC); + + if (fd < 0) { + virReportSystemError(errno, "%s", _("Cannot open resctrl")); + return -1; + } + + if (flock(fd, op) < 0) { + virReportSystemError(errno, "%s", _("Cannot lock resctrl")); + VIR_FORCE_CLOSE(fd); + return -1; } - return false; + return fd; } -int -virResctrlAllocSetSize(virResctrlAllocPtr resctrl, - unsigned int level, - virCacheType type, - unsigned int cache, - unsigned long long size) +static inline int +virResctrlLockWrite(void) { - if (virResctrlAllocCheckCollision(resctrl, level, type, cache)) { - virReportError(VIR_ERR_XML_ERROR, - _("Colliding cache allocations for cache " - "level '%u' id '%u', type '%s'"), - level, cache, virCacheTypeToString(type)); + return virResctrlLockInternal(LOCK_EX); +} + + +static int +virResctrlUnlock(int fd) +{ + if (fd == -1) + return 0; + + /* The lock gets unlocked by closing the fd, which we need to do anyway in + * order to clean up properly */ + if (VIR_CLOSE(fd) < 0) { + virReportSystemError(errno, "%s", _("Cannot close resctrl")); + + /* Trying to save the already broken */ + if (flock(fd, LOCK_UN) < 0) + virReportSystemError(errno, "%s", _("Cannot unlock resctrl")); return -1; } - return virResctrlAllocUpdateSize(resctrl, level, type, cache, size); + return 0; } int -virResctrlAllocForeachSize(virResctrlAllocPtr resctrl, - virResctrlAllocForeachSizeCallback cb, - void *opaque) +virResctrlGetInfo(virResctrlInfoPtr resctrl) { - int ret = 0; + DIR *dirp = NULL; + char *endptr = NULL; + char *tmp_str = NULL; + int ret = -1; + int rv = -1; + int type = 0; + struct dirent *ent = NULL; unsigned int level = 0; - unsigned int type = 0; - unsigned int cache = 0; - - if (!resctrl) - return 0; + virResctrlInfoPerLevelPtr i_level = NULL; + virResctrlInfoPerTypePtr i_type = NULL; - for (level = 0; level < resctrl->nlevels; level++) { - virResctrlAllocPerLevelPtr a_level = resctrl->levels[level]; + rv = virDirOpenIfExists(&dirp, SYSFS_RESCTRL_PATH "/info"); + if (rv <= 0) { + ret = rv; + goto cleanup; + } - if (!a_level) + while ((rv = virDirRead(dirp, &ent, SYSFS_RESCTRL_PATH "/info")) > 0) { + VIR_DEBUG("Parsing info type '%s'", ent->d_name); + if (ent->d_name[0] != 'L') continue; - for (type = 0; type < VIR_CACHE_TYPE_LAST; type++) { - virResctrlAllocPerTypePtr a_type = a_level->types[type]; + if (virStrToLong_uip(ent->d_name + 1, &endptr, 10, &level) < 0) { + VIR_DEBUG("Cannot parse resctrl cache info level '%s'", ent->d_name + 1); + continue; + } - if (!a_type) - continue; + type = virResctrlTypeFromString(endptr); + if (type < 0) { + VIR_DEBUG("Cannot parse resctrl cache info type '%s'", endptr); + continue; + } - for (cache = 0; cache < a_type->nsizes; cache++) { - unsigned long long *size = a_type->sizes[cache]; + if (VIR_ALLOC(i_type) < 0) + goto cleanup; - if (!size) - continue; + i_type->control.scope = type; - ret = cb(level, type, cache, *size, opaque); - if (ret < 0) - return ret; - } + rv = virFileReadValueUint(&i_type->control.max_allocation, + SYSFS_RESCTRL_PATH "/info/%s/num_closids", + ent->d_name); + if (rv == -2) { + /* The file doesn't exist, so it's unusable for us, + * but we can scan further */ + VIR_WARN("The path '" SYSFS_RESCTRL_PATH "/info/%s/num_closids' " + "does not exist", + ent->d_name); + } else if (rv < 0) { + /* Other failures are fatal, so just quit */ + goto cleanup; } - } - return 0; -} + rv = virFileReadValueString(&i_type->cbm_mask, + SYSFS_RESCTRL_PATH + "/info/%s/cbm_mask", + ent->d_name); + if (rv == -2) { + /* If the previous file exists, so should this one. Hence -2 is + * fatal in this case as well (errors out in next condition) - the + * kernel interface might've changed too much or something else is + * wrong. */ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot get cbm_mask from resctrl cache info")); + } + if (rv < 0) + goto cleanup; + virStringTrimOptionalNewline(i_type->cbm_mask); -int -virResctrlAllocSetID(virResctrlAllocPtr alloc, - const char *id) -{ - if (!id) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Resctrl allocation 'id' cannot be NULL")); - return -1; - } + rv = virFileReadValueUint(&i_type->min_cbm_bits, + SYSFS_RESCTRL_PATH "/info/%s/min_cbm_bits", + ent->d_name); + if (rv == -2) + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot get min_cbm_bits from resctrl cache info")); + if (rv < 0) + goto cleanup; - return VIR_STRDUP(alloc->id, id); -} + if (resctrl->nlevels <= level && + VIR_EXPAND_N(resctrl->levels, resctrl->nlevels, + level - resctrl->nlevels + 1) < 0) + goto cleanup; + if (!resctrl->levels[level]) { + virResctrlInfoPerTypePtr *types = NULL; -const char * -virResctrlAllocGetID(virResctrlAllocPtr alloc) -{ - return alloc->id; -} + if (VIR_ALLOC_N(types, VIR_CACHE_TYPE_LAST) < 0) + goto cleanup; + if (VIR_ALLOC(resctrl->levels[level]) < 0) { + VIR_FREE(types); + goto cleanup; + } + resctrl->levels[level]->types = types; + } -char * -virResctrlAllocFormat(virResctrlAllocPtr resctrl) -{ - virBuffer buf = VIR_BUFFER_INITIALIZER; - unsigned int level = 0; - unsigned int type = 0; - unsigned int cache = 0; + i_level = resctrl->levels[level]; - if (!resctrl) - return NULL; + if (i_level->types[type]) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Duplicate cache type in resctrl for level %u"), + level); + goto cleanup; + } - for (level = 0; level < resctrl->nlevels; level++) { - virResctrlAllocPerLevelPtr a_level = resctrl->levels[level]; + for (tmp_str = i_type->cbm_mask; *tmp_str != '\0'; tmp_str++) { + if (!c_isxdigit(*tmp_str)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse cbm_mask from resctrl cache info")); + goto cleanup; + } - if (!a_level) - continue; + i_type->bits += count_one_bits(virHexToBin(*tmp_str)); + } - for (type = 0; type < VIR_CACHE_TYPE_LAST; type++) { - virResctrlAllocPerTypePtr a_type = a_level->types[type]; + VIR_STEAL_PTR(i_level->types[type], i_type); + } - if (!a_type) - continue; + ret = 0; + cleanup: + VIR_DIR_CLOSE(dirp); + if (i_type) + VIR_FREE(i_type->cbm_mask); + VIR_FREE(i_type); + return ret; +} - virBufferAsprintf(&buf, "L%u%s:", level, virResctrlTypeToString(type)); - for (cache = 0; cache < a_type->nmasks; cache++) { - virBitmapPtr mask = a_type->masks[cache]; - char *mask_str = NULL; +static int +virResctrlAllocUpdateMask(virResctrlAllocPtr resctrl, + unsigned int level, + virCacheType type, + unsigned int cache, + virBitmapPtr mask) +{ + virResctrlAllocPerTypePtr a_type = virResctrlAllocGetType(resctrl, level, type); - if (!mask) - continue; + if (!a_type) + return -1; - mask_str = virBitmapToString(mask, false, true); - if (!mask_str) { - virBufferFreeAndReset(&buf); - return NULL; - } + if (a_type->nmasks <= cache && + VIR_EXPAND_N(a_type->masks, a_type->nmasks, + cache - a_type->nmasks + 1) < 0) + return -1; - virBufferAsprintf(&buf, "%u=%s;", cache, mask_str); - VIR_FREE(mask_str); - } + if (!a_type->masks[cache]) { + a_type->masks[cache] = virBitmapNew(virBitmapSize(mask)); - virBufferTrim(&buf, ";", 1); - virBufferAddChar(&buf, '\n'); - } + if (!a_type->masks[cache]) + return -1; } - virBufferCheckError(&buf); - return virBufferContentAndReset(&buf); + return virBitmapCopy(a_type->masks[cache], mask); } -#ifdef __linux__ - static int virResctrlAllocParseProcessCache(virResctrlInfoPtr resctrl, virResctrlAllocPtr alloc, @@ -1171,6 +897,7 @@ virResctrlAllocNewFromInfo(virResctrlInfoPtr info) goto cleanup; } + /* * This function creates an allocation that represents all unused parts of all * caches in the system. It uses virResctrlInfo for creating a new full @@ -1238,23 +965,11 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl) return ret; error: - virObjectUnref(ret); - ret = NULL; - goto cleanup; -} - -#else /* ! __linux__ */ - -virResctrlAllocPtr -virResctrlAllocGetUnused(virResctrlInfoPtr resctrl ATTRIBUTE_UNUSED) -{ - virReportSystemError(ENOSYS, "%s", - _("Cache tune not supported on this platform")); - return NULL; + virObjectUnref(ret); + ret = NULL; + goto cleanup; } -#endif /* ! __linux__ */ - /* * Given the information about requested allocation type `a_type`, the host @@ -1591,6 +1306,264 @@ virResctrlAllocCreate(virResctrlInfoPtr resctrl, return ret; } +#else /* ! __linux__ */ + +int +virResctrlGetInfo(virResctrlInfoPtr resctrl ATTRIBUTE_UNUSED) +{ + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Resource control is not supported on this host")); + return -1; +} + +int +virResctrlAllocCreate(virResctrlInfoPtr resctrl ATTRIBUTE_UNUSED, + virResctrlAllocPtr alloc ATTRIBUTE_UNUSED, + const char *machinename ATTRIBUTE_UNUSED) +{ + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Resource control is not supported on this host")); + return -1; +} + +#endif /* ! __linux__ */ + + +int +virResctrlInfoGetCache(virResctrlInfoPtr resctrl, + unsigned int level, + unsigned long long size, + size_t *ncontrols, + virResctrlInfoPerCachePtr **controls) +{ + virResctrlInfoPerLevelPtr i_level = NULL; + virResctrlInfoPerTypePtr i_type = NULL; + size_t i = 0; + int ret = -1; + + if (virResctrlInfoIsEmpty(resctrl)) + return 0; + + if (level >= resctrl->nlevels) + return 0; + + i_level = resctrl->levels[level]; + if (!i_level) + return 0; + + for (i = 0; i < VIR_CACHE_TYPE_LAST; i++) { + i_type = i_level->types[i]; + if (!i_type) + continue; + + /* Let's take the opportunity to update our internal information about + * the cache size */ + if (!i_type->size) { + i_type->size = size; + i_type->control.granularity = size / i_type->bits; + if (i_type->min_cbm_bits != 1) + i_type->control.min = i_type->min_cbm_bits * i_type->control.granularity; + } else { + if (i_type->size != size) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("level %u cache size %llu does not match " + "expected size %llu"), + level, i_type->size, size); + goto error; + } + i_type->max_cache_id++; + } + + if (VIR_EXPAND_N(*controls, *ncontrols, 1) < 0) + goto error; + if (VIR_ALLOC((*controls)[*ncontrols - 1]) < 0) + goto error; + + memcpy((*controls)[*ncontrols - 1], &i_type->control, sizeof(i_type->control)); + } + + ret = 0; + cleanup: + return ret; + error: + while (*ncontrols) + VIR_FREE((*controls)[--*ncontrols]); + VIR_FREE(*controls); + goto cleanup; +} + + +bool +virResctrlAllocIsEmpty(virResctrlAllocPtr resctrl) +{ + size_t i = 0; + size_t j = 0; + size_t k = 0; + + if (!resctrl) + return true; + + for (i = 0; i < resctrl->nlevels; i++) { + virResctrlAllocPerLevelPtr a_level = resctrl->levels[i]; + + if (!a_level) + continue; + + for (j = 0; j < VIR_CACHE_TYPE_LAST; j++) { + virResctrlAllocPerTypePtr a_type = a_level->types[j]; + + if (!a_type) + continue; + + for (k = 0; k < a_type->nsizes; k++) { + if (a_type->sizes[k]) + return false; + } + + for (k = 0; k < a_type->nmasks; k++) { + if (a_type->masks[k]) + return false; + } + } + } + + return true; +} + + +int +virResctrlAllocSetSize(virResctrlAllocPtr resctrl, + unsigned int level, + virCacheType type, + unsigned int cache, + unsigned long long size) +{ + if (virResctrlAllocCheckCollision(resctrl, level, type, cache)) { + virReportError(VIR_ERR_XML_ERROR, + _("Colliding cache allocations for cache " + "level '%u' id '%u', type '%s'"), + level, cache, virCacheTypeToString(type)); + return -1; + } + + return virResctrlAllocUpdateSize(resctrl, level, type, cache, size); +} + + +int +virResctrlAllocForeachSize(virResctrlAllocPtr resctrl, + virResctrlAllocForeachSizeCallback cb, + void *opaque) +{ + int ret = 0; + unsigned int level = 0; + unsigned int type = 0; + unsigned int cache = 0; + + if (!resctrl) + return 0; + + for (level = 0; level < resctrl->nlevels; level++) { + virResctrlAllocPerLevelPtr a_level = resctrl->levels[level]; + + if (!a_level) + continue; + + for (type = 0; type < VIR_CACHE_TYPE_LAST; type++) { + virResctrlAllocPerTypePtr a_type = a_level->types[type]; + + if (!a_type) + continue; + + for (cache = 0; cache < a_type->nsizes; cache++) { + unsigned long long *size = a_type->sizes[cache]; + + if (!size) + continue; + + ret = cb(level, type, cache, *size, opaque); + if (ret < 0) + return ret; + } + } + } + + return 0; +} + + +int +virResctrlAllocSetID(virResctrlAllocPtr alloc, + const char *id) +{ + if (!id) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Resctrl allocation 'id' cannot be NULL")); + return -1; + } + + return VIR_STRDUP(alloc->id, id); +} + + +const char * +virResctrlAllocGetID(virResctrlAllocPtr alloc) +{ + return alloc->id; +} + + +char * +virResctrlAllocFormat(virResctrlAllocPtr resctrl) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + unsigned int level = 0; + unsigned int type = 0; + unsigned int cache = 0; + + if (!resctrl) + return NULL; + + for (level = 0; level < resctrl->nlevels; level++) { + virResctrlAllocPerLevelPtr a_level = resctrl->levels[level]; + + if (!a_level) + continue; + + for (type = 0; type < VIR_CACHE_TYPE_LAST; type++) { + virResctrlAllocPerTypePtr a_type = a_level->types[type]; + + if (!a_type) + continue; + + virBufferAsprintf(&buf, "L%u%s:", level, virResctrlTypeToString(type)); + + for (cache = 0; cache < a_type->nmasks; cache++) { + virBitmapPtr mask = a_type->masks[cache]; + char *mask_str = NULL; + + if (!mask) + continue; + + mask_str = virBitmapToString(mask, false, true); + if (!mask_str) { + virBufferFreeAndReset(&buf); + return NULL; + } + + virBufferAsprintf(&buf, "%u=%s;", cache, mask_str); + VIR_FREE(mask_str); + } + + virBufferTrim(&buf, ";", 1); + virBufferAddChar(&buf, '\n'); + } + } + + virBufferCheckError(&buf); + return virBufferContentAndReset(&buf); +} + int virResctrlAllocAddPID(virResctrlAllocPtr alloc, -- 2.16.1
On Wed, Jan 31, 2018 at 10:41:45AM +0100, Martin Kletzander wrote:
If we reorder the functions properly, we only need to have few stub functions for non-Linux platforms and it is also nicer to look at and read if they are in one place with only one preprocessor condition. The order after this patch is:
- Class allocation and Static functions (used by everything, static dependencies)
- Linux-only functions
- Non-Linux stubs
- Exported functions (as they rely on all previous functions)
One of the functions (latest addition) needs to be moved due to non-Linux, I'll move it, don't look at this patch, I'll send a v2, check the rest in the meatime ;)
This was just forgotten when doing more renames in the past when working in the reviews, I believe. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/util/virresctrl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index cab7570f7b8e..2948f2b9569c 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -618,13 +618,13 @@ virResctrlGetInfo(virResctrlInfoPtr resctrl) static int -virResctrlAllocUpdateMask(virResctrlAllocPtr resctrl, +virResctrlAllocUpdateMask(virResctrlAllocPtr alloc, unsigned int level, virCacheType type, unsigned int cache, virBitmapPtr mask) { - virResctrlAllocPerTypePtr a_type = virResctrlAllocGetType(resctrl, level, type); + virResctrlAllocPerTypePtr a_type = virResctrlAllocGetType(alloc, level, type); if (!a_type) return -1; -- 2.16.1
participants (2)
-
Martin Kletzander -
Michal Privoznik