On 12/13/2017 10:39 AM, Martin Kletzander wrote: static int virRes.... Two blank lines Move this up a few lines to after cbm_mask successfully read... In a cleanup/failure path i_type.cbm_mask is leaked, thus before this if (i_type) VIR_FREE(i_type->cbm_mask); or go with the VIR_STEAL_PTR type logic for a local @cbm_mask to be stored in i_type->cbm_mask (IDC whichever way you prefer) Not really a boolean or pointer comparison... level %u cache size %llu not match expected %llu levek, i_type->size, size Is this really necessary? The caller on failure will call virCapsHostCacheBankFree(bank) which does the free's. Superfluous, but IDC Reviewed-by: John Ferlan <jferlan(a)redhat.com&gt; John

Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/conf/capabilities.c | 53 ++++++++++++++++++++++++------------------------- src/conf/capabilities.h | 2 ++ 2 files changed, 28 insertions(+), 27 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index e93eaed2f0e0..edf9f54f7710 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -245,6 +245,7 @@ virCapabilitiesDispose(void *object) VIR_FREE(caps->host.netprefix); VIR_FREE(caps->host.pagesSize); virCPUDefFree(caps->host.cpu); + virObjectUnref(caps->host.resctrl); } /** @@ -1592,6 +1593,20 @@ virCapsHostCacheBankSorter(const void *a, } +static int +virCapabilitiesInitResctrl(virCapsPtr caps) +{ + if (caps->host.resctrl) + return 0; + + caps->host.resctrl = virResctrlInfoNew(); + if (!caps->host.resctrl) + return -1; + + return virResctrlGetInfo(caps->host.resctrl); +} + + int virCapabilitiesInitCaches(virCapsPtr caps) { @@ -1600,7 +1615,6 @@ virCapabilitiesInitCaches(virCapsPtr caps) ssize_t pos = -1; DIR *dirp = NULL; int ret = -1; - int typeret; char *path = NULL; char *type = NULL; struct dirent *ent = NULL; @@ -1611,6 +1625,9 @@ virCapabilitiesInitCaches(virCapsPtr caps) * lose information. */ const int cache_min_level = 3; + if (virCapabilitiesInitResctrl(caps) < 0) + return -1; + /* offline CPUs don't provide cache info */ if (virFileReadValueBitmap(&cpus, "%s/cpu/online", SYSFS_SYSTEM_PATH) < 0) return -1; @@ -1676,32 +1693,6 @@ virCapabilitiesInitCaches(virCapsPtr caps) SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0) goto cleanup; - typeret = virResctrlGetCacheControlType(bank->level); - if (typeret < 0) - goto cleanup; - - if (typeret == 1) { - if (virResctrlGetCacheInfo(bank->level, - bank->size, - VIR_CACHE_TYPE_BOTH, - &bank->controls, - &bank->ncontrols) < 0) - goto cleanup; - } else if (typeret == 2) { - if (virResctrlGetCacheInfo(bank->level, - bank->size, - VIR_CACHE_TYPE_CODE, - &bank->controls, - &bank->ncontrols) < 0) - goto cleanup; - if (virResctrlGetCacheInfo(bank->level, - bank->size, - VIR_CACHE_TYPE_DATA, - &bank->controls, - &bank->ncontrols) < 0) - goto cleanup; - } - kernel_type = virCacheKernelTypeFromString(type); if (kernel_type < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1717,6 +1708,14 @@ virCapabilitiesInitCaches(virCapsPtr caps) break; } if (i == caps->host.ncaches) { + /* If it is a new cache, then update its resctrl information. */ + if (virResctrlInfoGetCache(caps->host.resctrl, + bank->level, + bank->size, + &bank->ncontrols, + &bank->controls) < 0) + goto cleanup; + if (VIR_APPEND_ELEMENT(caps->host.caches, caps->host.ncaches, bank) < 0) { diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index 27b88cb5edfa..694a3590bf83 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -170,6 +170,8 @@ struct _virCapsHost { size_t nnumaCell_max; virCapsHostNUMACellPtr *numaCell; + virResctrlInfoPtr resctrl; + size_t ncaches; virCapsHostCacheBankPtr *caches; -- 2.15.1

Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/libvirt_private.syms | 2 - src/util/virresctrl.c | 132 ----------------------------------------------- src/util/virresctrl.h | 11 ---- 3 files changed, 145 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 75be612a2f13..1a4f304f5e1f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2548,8 +2548,6 @@ virRandomInt; # util/virresctrl.h virCacheTypeFromString; virCacheTypeToString; -virResctrlGetCacheControlType; -virResctrlGetCacheInfo; virResctrlGetInfo; virResctrlInfoGetCache; virResctrlInfoNew; diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index 6fd1ceb587cf..a681322905be 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -354,135 +354,3 @@ virResctrlInfoGetCache(virResctrlInfoPtr resctrl, VIR_FREE(*controls); goto cleanup; } - - -int -virResctrlGetCacheInfo(unsigned int level, - unsigned long long size, - virCacheType scope, - virResctrlInfoPerCachePtr **controls, - size_t *ncontrols) -{ - int ret = -1; - char *tmp = NULL; - char *path = NULL; - char *cbm_mask = NULL; - char *type_upper = NULL; - unsigned int bits = 0; - unsigned int min_cbm_bits = 0; - virResctrlInfoPerCachePtr control; - - if (VIR_ALLOC(control) < 0) - goto cleanup; - - if (scope != VIR_CACHE_TYPE_BOTH && - virStringToUpper(&type_upper, virCacheTypeToString(scope)) < 0) - goto cleanup; - - if (virFileReadValueUint(&control->max_allocation, - SYSFS_RESCTRL_PATH "/info/L%u%s/num_closids", - level, - type_upper ? type_upper : "") < 0) - goto cleanup; - - if (virFileReadValueString(&cbm_mask, - SYSFS_RESCTRL_PATH - "/info/L%u%s/cbm_mask", - level, - type_upper ? type_upper: "") < 0) - goto cleanup; - - if (virFileReadValueUint(&min_cbm_bits, - SYSFS_RESCTRL_PATH "/info/L%u%s/min_cbm_bits", - level, - type_upper ? type_upper : "") < 0) - goto cleanup; - - virStringTrimOptionalNewline(cbm_mask); - - for (tmp = cbm_mask; *tmp != '\0'; tmp++) { - if (c_isxdigit(*tmp)) - bits += count_one_bits(virHexToBin(*tmp)); - } - - control->granularity = size / bits; - if (min_cbm_bits != 1) - control->min = min_cbm_bits * control->granularity; - - control->scope = scope; - - if (VIR_APPEND_ELEMENT(*controls, *ncontrols, control) < 0) - goto cleanup; - - ret = 0; - - cleanup: - VIR_FREE(path); - VIR_FREE(cbm_mask); - VIR_FREE(type_upper); - VIR_FREE(control); - return ret; -} - - -static inline int -virResctrlGetCacheDir(char **path, - const char *prefix, - unsigned int level, - virCacheType type) -{ - return virAsprintf(path, - SYSFS_RESCTRL_PATH "%s/L%u%s", - prefix ? prefix : "", - level, - virResctrlTypeToString(type)); -} - - -/* - * This function tests whether TYPE of cache control is supported or not. - * - * Returns 0 if not, 1 if yes and negative value on error. - */ -static int -virResctrlGetCacheSupport(unsigned int level, virCacheType type) -{ - int ret = -1; - char *path = NULL; - - if (virResctrlGetCacheDir(&path, "/info", level, type) < 0) - return -1; - - ret = virFileExists(path); - VIR_FREE(path); - return ret; -} - - -/* - * This function tests which TYPE of cache control is supported - * Return values are: - * -1: error - * 0: none - * 1: CAT - * 2: CDP - */ -int -virResctrlGetCacheControlType(unsigned int level) -{ - int rv = -1; - - rv = virResctrlGetCacheSupport(level, VIR_CACHE_TYPE_BOTH); - if (rv < 0) - return -1; - if (rv) - return 1; - - rv = virResctrlGetCacheSupport(level, VIR_CACHE_TYPE_CODE); - if (rv < 0) - return -1; - if (rv) - return 2; - - return 0; -} diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h index 43063903730b..471c02f28dcd 100644 --- a/src/util/virresctrl.h +++ b/src/util/virresctrl.h @@ -68,15 +68,4 @@ virResctrlInfoGetCache(virResctrlInfoPtr resctrl, size_t *ncontrols, virResctrlInfoPerCachePtr **controls); -/* To be removed */ -int -virResctrlGetCacheInfo(unsigned int level, - unsigned long long size, - virCacheType scope, - virResctrlInfoPerCachePtr **controls, - size_t *ncontrols); - -int -virResctrlGetCacheControlType(unsigned int level); - #endif /* __VIR_RESCTRL_H__ */ -- 2.15.1

With this commit we finally have a way to read and manipulate basic resctrl settings. Locking is done only on exposed functions that read/write from/to resctrlfs. Not in functions that are exposed in virresctrlpriv.h as those are only supposed to be used from tests. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/Makefile.am | 2 +- src/libvirt_private.syms | 11 + src/util/virresctrl.c | 1041 ++++++++++++++++++++++++++++++++++++++++++++- src/util/virresctrl.h | 62 +++ src/util/virresctrlpriv.h | 27 ++ 5 files changed, 1141 insertions(+), 2 deletions(-) create mode 100644 src/util/virresctrlpriv.h diff --git a/src/Makefile.am b/src/Makefile.am index 4c022d1e44b9..9b4fd0c1d597 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -167,7 +167,7 @@ UTIL_SOURCES = \ util/virprocess.c util/virprocess.h \ util/virqemu.c util/virqemu.h \ util/virrandom.h util/virrandom.c \ - util/virresctrl.h util/virresctrl.c \ + util/virresctrl.h util/virresctrl.c util/virresctrlpriv.h \ util/virrotatingfile.h util/virrotatingfile.c \ util/virscsi.c util/virscsi.h \ util/virscsihost.c util/virscsihost.h \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1a4f304f5e1f..a8009d913669 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2548,6 +2548,17 @@ virRandomInt; # util/virresctrl.h virCacheTypeFromString; virCacheTypeToString; +virResctrlAllocAddPID; +virResctrlAllocCreate; +virResctrlAllocForeachSize; +virResctrlAllocFormat; +virResctrlAllocGetFree; +virResctrlAllocNew; +virResctrlAllocRemove; +virResctrlAllocSetID; +virResctrlAllocSetSize; +virResctrlAllocUpdateMask; +virResctrlAllocUpdateSize; virResctrlGetInfo; virResctrlInfoGetCache; virResctrlInfoNew; diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index a681322905be..32ab84b6f121 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -18,8 +18,12 @@ #include <config.h> -#include "virresctrl.h" +#include <sys/file.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <fcntl.h> +#include "virresctrlpriv.h" #include "c-ctype.h" #include "count-one-bits.h" #include "viralloc.h" @@ -151,6 +155,156 @@ virResctrlInfoNew(void) } +/* Alloc-related definitions and AllocClass-related functions */ +typedef struct _virResctrlAllocPerType virResctrlAllocPerType; +typedef virResctrlAllocPerType *virResctrlAllocPerTypePtr; +struct _virResctrlAllocPerType { + /* There could be bool saying whether this is set or not, but since everything + * in virResctrlAlloc (and most of libvirt) goes with pointer arrays we would + * have to have one more level of allocation anyway, so this stays faithful to + * the concept */ + unsigned long long **sizes; + size_t nsizes; + + /* Mask for each cache */ + virBitmapPtr *masks; + size_t nmasks; +}; + +typedef struct _virResctrlAllocPerLevel virResctrlAllocPerLevel; +typedef virResctrlAllocPerLevel *virResctrlAllocPerLevelPtr; +struct _virResctrlAllocPerLevel { + virResctrlAllocPerTypePtr *types; /* Indexed with enum virCacheType */ +}; + +struct _virResctrlAlloc { + virObject parent; + + virResctrlAllocPerLevelPtr *levels; + size_t nlevels; + + char *id; /* The identifier (any unique string for now) */ + char *path; +}; + +static virClassPtr virResctrlAllocClass; + +static void +virResctrlAllocDispose(void *obj) +{ + size_t i = 0; + size_t j = 0; + size_t k = 0; + + virResctrlAllocPtr resctrl = obj; + + for (i = 0; i < resctrl->nlevels; i++) { + virResctrlAllocPerLevelPtr level = resctrl->levels[i]; + + if (!level) + continue; + + for (j = 0; j < VIR_CACHE_TYPE_LAST; j++) { + virResctrlAllocPerTypePtr type = level->types[j]; + + if (!type) + continue; + + for (k = 0; k < type->nsizes; k++) + VIR_FREE(type->sizes[k]); + + for (k = 0; k < type->nmasks; k++) + virBitmapFree(type->masks[k]); + + VIR_FREE(type->sizes); + VIR_FREE(type->masks); + VIR_FREE(type); + } + VIR_FREE(level->types); + VIR_FREE(level); + } + + VIR_FREE(resctrl->id); + VIR_FREE(resctrl->path); + VIR_FREE(resctrl->levels); +} + + +static int +virResctrlAllocOnceInit(void) +{ + if (!(virResctrlAllocClass = virClassNew(virClassForObject(), + "virResctrlAlloc", + sizeof(virResctrlAlloc), + virResctrlAllocDispose))) + return -1; + + return 0; +} + + +VIR_ONCE_GLOBAL_INIT(virResctrlAlloc) + + +virResctrlAllocPtr +virResctrlAllocNew(void) +{ + if (virResctrlAllocInitialize() < 0) + return NULL; + + return virObjectNew(virResctrlAllocClass); +} + + +/* Common functions */ +static int +virResctrlLockInternal(int op) +{ + int fd = open(SYSFS_RESCTRL_PATH, O_DIRECTORY | O_CLOEXEC); + + if (fd < 0) { + virReportSystemError(errno, "%s", _("Cannot open resctrlfs")); + return -1; + } + + if (flock(fd, op) < 0) { + virReportSystemError(errno, "%s", _("Cannot lock resctrlfs")); + VIR_FORCE_CLOSE(fd); + return -1; + } + + return fd; +} + + +static inline int +virResctrlLockWrite(void) +{ + 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 resctrlfs")); + + /* Trying to save the already broken */ + if (flock(fd, LOCK_UN) < 0) + virReportSystemError(errno, "%s", _("Cannot unlock resctrlfs")); + return -1; + } + + return 0; +} + + /* Info-related functions */ bool virResctrlInfoIsEmpty(virResctrlInfoPtr resctrl) @@ -354,3 +508,888 @@ virResctrlInfoGetCache(virResctrlInfoPtr resctrl, 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, + virCacheType type) +{ + virResctrlAllocPerLevelPtr a_level = NULL; + + if (resctrl->nlevels <= level && + VIR_EXPAND_N(resctrl->levels, resctrl->nlevels, level - resctrl->nlevels + 1) < 0) + return NULL; + + if (!resctrl->levels[level] && + (VIR_ALLOC(resctrl->levels[level]) < 0 || + VIR_ALLOC_N(resctrl->levels[level]->types, VIR_CACHE_TYPE_LAST) < 0)) + return NULL; + + a_level = resctrl->levels[level]; + + if (!a_level->types[type] && VIR_ALLOC(a_level->types[type]) < 0) + return NULL; + + return a_level->types[type]; +} + + +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); +} + + +int +virResctrlAllocUpdateSize(virResctrlAllocPtr resctrl, + unsigned int level, + virCacheType type, + unsigned int cache, + unsigned long long size) +{ + virResctrlAllocPerTypePtr a_type = virResctrlAllocGetType(resctrl, level, type); + + if (!a_type) + return -1; + + if (a_type->nsizes <= cache && + VIR_EXPAND_N(a_type->sizes, a_type->nsizes, + cache - a_type->nsizes + 1) < 0) + return -1; + + if (!a_type->sizes[cache] && VIR_ALLOC(a_type->sizes[cache]) < 0) + return -1; + + *(a_type->sizes[cache]) = size; + + return 0; +} + + +static bool +virResctrlAllocCheckCollision(virResctrlAllocPtr a, + unsigned int level, + virCacheType type, + unsigned int cache) +{ + virResctrlAllocPerLevelPtr a_level = NULL; + virResctrlAllocPerTypePtr a_type = NULL; + + if (!a) + return false; + + if (a->nlevels <= level) + return false; + + a_level = a->levels[level]; + + if (!a_level) + return false; + + /* All types should be always allocated */ + a_type = a_level->types[VIR_CACHE_TYPE_BOTH]; + + /* If there is an allocation for type 'both', there can be no other + * allocation for the same cache */ + if (a_type && a_type->nsizes > cache && a_type->sizes[cache]) + return true; + + if (type == VIR_CACHE_TYPE_BOTH) { + a_type = a_level->types[VIR_CACHE_TYPE_CODE]; + + if (a_type && a_type->nsizes > cache && a_type->sizes[cache]) + return true; + + a_type = a_level->types[VIR_CACHE_TYPE_DATA]; + + if (a_type && a_type->nsizes > cache && a_type->sizes[cache]) + return true; + } else { + a_type = a_level->types[type]; + + if (a_type && a_type->nsizes > cache && a_type->sizes[cache]) + return true; + } + + return false; +} + + +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); +} + + +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); +} + + +static int +virResctrlAllocParseProcessCache(virResctrlAllocPtr resctrl, + unsigned int level, + virCacheType type, + char *cache) +{ + char *tmp = strchr(cache, '='); + unsigned int cache_id = 0; + virBitmapPtr mask = NULL; + int ret = -1; + + if (!tmp) + return 0; + + *tmp = '\0'; + tmp++; + + if (virStrToLong_uip(cache, NULL, 10, &cache_id) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid cache id '%s'"), cache); + return -1; + } + + mask = virBitmapNewString(tmp); + if (!mask) + return -1; + + if (virResctrlAllocUpdateMask(resctrl, level, type, cache_id, mask) < 0) + goto cleanup; + + ret = 0; + cleanup: + virBitmapFree(mask); + return ret; +} + + +static int +virResctrlAllocParseProcessLine(virResctrlAllocPtr resctrl, + char *line) +{ + char **caches = NULL; + char *tmp = NULL; + unsigned int level = 0; + int type = -1; + size_t ncaches = 0; + size_t i = 0; + int ret = -1; + + /* Skip lines that don't concern caches, e.g. MB: etc. */ + if (line[0] != 'L') + return 0; + + /* And lines that we can't parse too */ + tmp = strchr(line, ':'); + if (!tmp) + return 0; + + *tmp = '\0'; + tmp++; + + if (virStrToLong_uip(line + 1, &line, 10, &level) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot parse resctrl schema level '%s'"), + line + 1); + return -1; + } + + type = virResctrlTypeFromString(line); + if (type < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot parse resctrl schema level '%s'"), + line + 1); + return -1; + } + + caches = virStringSplitCount(tmp, ";", 0, &ncaches); + if (!caches) + return 0; + + for (i = 0; i < ncaches; i++) { + if (virResctrlAllocParseProcessCache(resctrl, level, type, caches[i]) < 0) + goto cleanup; + } + + ret = 0; + cleanup: + virStringListFree(caches); + return ret; +} + + +static int +virResctrlAllocParse(virResctrlAllocPtr alloc, + const char *schemata) +{ + char **lines = NULL; + size_t nlines = 0; + size_t i = 0; + int ret = -1; + + lines = virStringSplitCount(schemata, "\n", 0, &nlines); + for (i = 0; i < nlines; i++) { + if (virResctrlAllocParseProcessLine(alloc, lines[i]) < 0) + goto cleanup; + } + + ret = 0; + cleanup: + virStringListFree(lines); + return ret; +} + + +static void +virResctrlAllocSubtractPerType(virResctrlAllocPerTypePtr a, + virResctrlAllocPerTypePtr b) +{ + size_t i = 0; + + if (!a || !b) + return; + + for (i = 0; i < a->nmasks && i < b->nmasks; ++i) { + if (a->masks[i] && b->masks[i]) + virBitmapSubtract(a->masks[i], b->masks[i]); + } +} + + +static void +virResctrlAllocSubtract(virResctrlAllocPtr a, + virResctrlAllocPtr b) +{ + size_t i = 0; + size_t j = 0; + + if (!b) + return; + + for (i = 0; i < a->nlevels && b->nlevels; ++i) { + if (a->levels[i] && b->levels[i]) { + /* Here we rely on all the system allocations to use the same types. + * We kind of _hope_ it's the case. If this is left here until the + * review and someone finds it, then suggest only removing this last + * sentence. */ + for (j = 0; j < VIR_CACHE_TYPE_LAST; j++) { + virResctrlAllocSubtractPerType(a->levels[i]->types[j], + b->levels[i]->types[j]); + } + } + } +} + + +static virResctrlAllocPtr +virResctrlAllocNewFromInfo(virResctrlInfoPtr info) +{ + size_t i = 0; + size_t j = 0; + size_t k = 0; + virResctrlAllocPtr ret = virResctrlAllocNew(); + virBitmapPtr mask = NULL; + + if (!ret) + return NULL; + + for (i = 0; i < info->nlevels; i++) { + virResctrlInfoPerLevelPtr i_level = info->levels[i]; + + if (!i_level) + continue; + + for (j = 0; j < VIR_CACHE_TYPE_LAST; j++) { + virResctrlInfoPerTypePtr i_type = i_level->types[j]; + + if (!i_type) + continue; + + virBitmapFree(mask); + mask = virBitmapNew(i_type->bits); + if (!mask) + goto error; + virBitmapSetAll(mask); + + for (k = 0; k <= i_type->max_cache_id; k++) { + if (virResctrlAllocUpdateMask(ret, i, j, k, mask) < 0) + goto error; + } + } + } + + cleanup: + virBitmapFree(mask); + return ret; + error: + virObjectUnref(ret); + ret = NULL; + goto cleanup; +} + + +virResctrlAllocPtr +virResctrlAllocGetFree(virResctrlInfoPtr resctrl) +{ + virResctrlAllocPtr ret = NULL; + virResctrlAllocPtr alloc = NULL; + virBitmapPtr mask = NULL; + struct dirent *ent = NULL; + DIR *dirp = NULL; + char *schemata = NULL; + int rv = -1; + + if (virResctrlInfoIsEmpty(resctrl)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Resource control is not supported on this host")); + return NULL; + } + + ret = virResctrlAllocNewFromInfo(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(); + if (!alloc) + goto error; + + if (virResctrlAllocParse(alloc, schemata) < 0) + goto error; + if (virResctrlAllocIsEmpty(alloc)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No schemata for default resctrlfs group")); + goto error; + } + virResctrlAllocSubtract(ret, alloc); + + if (virDirOpen(&dirp, SYSFS_RESCTRL_PATH) < 0) + goto error; + + while ((rv = virDirRead(dirp, &ent, SYSFS_RESCTRL_PATH)) > 0) { + if (ent->d_type != DT_DIR) + continue; + + if (STREQ(ent->d_name, "info")) + continue; + + VIR_FREE(schemata); + if (virFileReadValueString(&schemata, + SYSFS_RESCTRL_PATH + "/%s/schemata", + ent->d_name) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not read schemata file for group %s"), + ent->d_name); + goto error; + } + + virObjectUnref(alloc); + alloc = virResctrlAllocNew(); + if (!alloc) + goto error; + + if (virResctrlAllocParse(alloc, schemata) < 0) + goto error; + + virResctrlAllocSubtract(ret, alloc); + + VIR_FREE(schemata); + } + if (rv < 0) + goto error; + + cleanup: + virObjectUnref(alloc); + VIR_DIR_CLOSE(dirp); + VIR_FREE(schemata); + virBitmapFree(mask); + return ret; + + error: + virObjectUnref(ret); + ret = NULL; + goto cleanup; +} + + +static int +virResctrlAllocMasksAssign(virResctrlInfoPtr r_info, + virResctrlAllocPtr alloc) +{ + int ret = -1; + unsigned int level = 0; + virResctrlAllocPtr alloc_free = NULL; + + if (!r_info) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Resource control is not supported on this host")); + return -1; + } + + alloc_free = virResctrlAllocGetFree(r_info); + if (!alloc_free) + return -1; + + for (level = 0; level < alloc->nlevels; level++) { + virResctrlAllocPerLevelPtr a_level = alloc->levels[level]; + virResctrlAllocPerLevelPtr f_level = NULL; + unsigned int type = 0; + + if (!a_level) + continue; + + if (level < alloc_free->nlevels) + f_level = alloc_free->levels[level]; + + if (!f_level) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Cache level %d does not support tuning"), + level); + goto cleanup; + } + + for (type = 0; type < VIR_CACHE_TYPE_LAST; type++) { + virResctrlAllocPerTypePtr a_type = a_level->types[type]; + virResctrlAllocPerTypePtr f_type = f_level->types[type]; + unsigned int cache = 0; + + if (!a_type) + continue; + + if (!f_type) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Cache level %d does not support tuning for " + "scope type '%s'"), + level, virCacheTypeToString(type)); + goto cleanup; + } + + for (cache = 0; cache < a_type->nsizes; cache++) { + unsigned long long *size = a_type->sizes[cache]; + virBitmapPtr a_mask = NULL; + virBitmapPtr f_mask = NULL; + virResctrlInfoPerLevelPtr i_level = r_info->levels[level]; + virResctrlInfoPerTypePtr i_type = i_level->types[type]; + unsigned long long granularity; + unsigned long long need_bits; + size_t i = 0; + ssize_t pos = -1; + ssize_t last_bits = 0; + ssize_t last_pos = -1; + + if (!size) + continue; + + if (cache >= f_type->nmasks) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Cache with id %u does not exists for level %d"), + cache, level); + goto cleanup; + } + + f_mask = f_type->masks[cache]; + if (!f_mask) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Cache level %d id %u does not support tuning for " + "scope type '%s'"), + level, cache, virCacheTypeToString(type)); + goto cleanup; + } + + granularity = i_type->size / i_type->bits; + need_bits = *size / granularity; + + if (*size % granularity) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Cache allocation of size %llu is not " + "divisible by granularity %llu"), + *size, granularity); + goto cleanup; + } + + if (need_bits < i_type->min_cbm_bits) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Cache allocation of size %llu is smaller " + "than the minimum allowed allocation %llu"), + *size, granularity * i_type->min_cbm_bits); + goto cleanup; + } + + while ((pos = virBitmapNextSetBit(f_mask, pos)) >= 0) { + ssize_t pos_clear = virBitmapNextClearBit(f_mask, pos); + ssize_t bits; + + if (pos_clear < 0) + pos_clear = virBitmapSize(f_mask); + + bits = pos_clear - pos; + + /* Not enough bits, move on and skip all of them */ + if (bits < need_bits) { + pos = pos_clear; + continue; + } + + /* This fits perfectly */ + if (bits == need_bits) { + last_pos = pos; + break; + } + + /* Remember the smaller region if we already found on before */ + if (last_pos < 0 || (last_bits && bits < last_bits)) { + last_bits = bits; + last_pos = pos; + } + + pos = pos_clear; + } + + if (last_pos < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Not enough room for allocation of " + "%llu bytes for level %u cache %u " + "scope type '%s'"), + *size, level, cache, + virCacheTypeToString(type)); + goto cleanup; + } + + a_mask = virBitmapNew(i_type->bits); + for (i = last_pos; i < last_pos + need_bits; i++) { + ignore_value(virBitmapSetBit(a_mask, i)); + ignore_value(virBitmapClearBit(f_mask, i)); + } + + if (a_type->nmasks <= cache) { + if (VIR_EXPAND_N(a_type->masks, a_type->nmasks, + cache - a_type->nmasks + 1) < 0) { + virBitmapFree(a_mask); + goto cleanup; + } + } + a_type->masks[cache] = a_mask; + } + } + } + + ret = 0; + cleanup: + virObjectUnref(alloc_free); + return ret; +} + + +/* This checks if the directory for the alloc exists. If not it tries to create + * it and apply appropriate alloc settings. */ +int +virResctrlAllocCreate(virResctrlInfoPtr r_info, + virResctrlAllocPtr alloc, + const char *drivername, + const char *machinename) +{ + char *schemata_path = NULL; + char *alloc_str = NULL; + int ret = -1; + int lockfd = -1; + + if (!alloc) + return 0; + + if (!r_info) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Resource control is not supported on this host")); + return -1; + } + + if (!alloc->path && + virAsprintf(&alloc->path, "%s/%s-%s-%s", + SYSFS_RESCTRL_PATH, drivername, machinename, alloc->id) < 0) + return -1; + + /* Check if this allocation was already created */ + if (virFileIsDir(alloc->path)) + return 0; + + if (virFileExists(alloc->path)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Path '%s' for resctrl allocation exists and is not a " + "directory"), alloc->path); + goto cleanup; + } + + lockfd = virResctrlLockWrite(); + if (lockfd < 0) + goto cleanup; + + if (virResctrlAllocMasksAssign(r_info, alloc) < 0) + goto cleanup; + + alloc_str = virResctrlAllocFormat(alloc); + if (!alloc_str) + goto cleanup; + + if (virAsprintf(&schemata_path, "%s/schemata", alloc->path) < 0) + goto cleanup; + + if (virFileMakePath(alloc->path) < 0) { + virReportSystemError(errno, + _("Cannot create resctrl directory '%s'"), + alloc->path); + goto cleanup; + } + + if (virFileWriteStr(schemata_path, alloc_str, 0) < 0) { + rmdir(alloc->path); + virReportSystemError(errno, + _("Cannot write into schemata file '%s'"), + schemata_path); + goto cleanup; + } + + ret = 0; + cleanup: + virResctrlUnlock(lockfd); + VIR_FREE(alloc_str); + VIR_FREE(schemata_path); + return ret; +} + + +int +virResctrlAllocAddPID(virResctrlAllocPtr alloc, + pid_t pid) +{ + char *tasks = NULL; + char *pidstr = NULL; + int ret = 0; + + if (!alloc->path) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot add pid to non-existing resctrl allocation")); + return -1; + } + + if (virAsprintf(&tasks, "%s/tasks", alloc->path) < 0) + return -1; + + if (virAsprintf(&pidstr, "%lld", (long long int) pid) < 0) + goto cleanup; + + if (virFileWriteStr(tasks, pidstr, 0) < 0) { + virReportSystemError(errno, + _("Cannot write pid in tasks file '%s'"), + tasks); + goto cleanup; + } + + ret = 0; + cleanup: + VIR_FREE(tasks); + VIR_FREE(pidstr); + return ret; +} + + +int +virResctrlAllocRemove(virResctrlAllocPtr alloc) +{ + int ret = 0; + + if (!alloc->path) + return 0; + + VIR_DEBUG("Removing resctrl allocation %s", alloc->path); + if (rmdir(alloc->path) != 0 && errno != ENOENT) { + ret = -errno; + VIR_ERROR(_("Unable to remove %s (%d)"), alloc->path, errno); + } + + return ret; +} diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h index 471c02f28dcd..0fbd9dd3acfb 100644 --- a/src/util/virresctrl.h +++ b/src/util/virresctrl.h @@ -68,4 +68,66 @@ virResctrlInfoGetCache(virResctrlInfoPtr resctrl, size_t *ncontrols, virResctrlInfoPerCachePtr **controls); +/* Alloc-related things */ +typedef struct _virResctrlAlloc virResctrlAlloc; +typedef virResctrlAlloc *virResctrlAllocPtr; + +typedef int virResctrlAllocForeachSizeCallback(unsigned int level, + virCacheType type, + unsigned int cache, + unsigned long long size, + void *opaque); + +virResctrlAllocPtr +virResctrlAllocNew(void); + +bool +virResctrlAllocIsEmpty(virResctrlAllocPtr resctrl); + +int +virResctrlAllocUpdateSize(virResctrlAllocPtr resctrl, + unsigned int level, + virCacheType type, + unsigned int cache, + unsigned long long size); + +int +virResctrlAllocUpdateMask(virResctrlAllocPtr resctrl, + unsigned int level, + virCacheType type, + unsigned int cache, + virBitmapPtr mask); + +int +virResctrlAllocSetSize(virResctrlAllocPtr resctrl, + unsigned int level, + virCacheType type, + unsigned int cache, + unsigned long long size); + +int +virResctrlAllocForeachSize(virResctrlAllocPtr resctrl, + virResctrlAllocForeachSizeCallback cb, + void *opaque); + +int +virResctrlAllocSetID(virResctrlAllocPtr alloc, + const char *id); + +char * +virResctrlAllocFormat(virResctrlAllocPtr alloc); + +int +virResctrlAllocCreate(virResctrlInfoPtr r_info, + virResctrlAllocPtr alloc, + const char *drivername, + const char *machinename); + +int +virResctrlAllocAddPID(virResctrlAllocPtr alloc, + pid_t pid); + +int +virResctrlAllocRemove(virResctrlAllocPtr alloc); + #endif /* __VIR_RESCTRL_H__ */ diff --git a/src/util/virresctrlpriv.h b/src/util/virresctrlpriv.h new file mode 100644 index 000000000000..932ff7329c8c --- /dev/null +++ b/src/util/virresctrlpriv.h @@ -0,0 +1,27 @@ +/* + * virresctrlpriv.h: + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + */ + +#ifndef __VIR_RESCTRL_PRIV_H__ +# define __VIR_RESCTRL_PRIV_H__ + +# include "virresctrl.h" + +virResctrlAllocPtr +virResctrlAllocGetFree(virResctrlInfoPtr resctrl); + +#endif /* __VIR_RESCTRL_PRIV_H__ */ -- 2.15.1

On Wed, Jan 03, 2018 at 04:05:38PM -0500, John Ferlan wrote: It's an array that stores allocation size per CPU cache, since there can be multiple CPUs in the host system. The mask represents how the cache allocation is written into the schemata file and it is based on the cache allocation size. Let's say that you have L3 cache with size 15 MiB where granularity of cache allocation is 768 KiB but minimal allocation is 1536 KiB. This means that you have 20bit mask to configure the cache allocation. So for example, if you would like to configure 3072 KiB of L3 cache for a guest, the size would be stored in sizes. When the guest is started, the following code tries to find a free allocation, that can be used by the guest and if the allocation is possible it will store the allocation mask for that specified CPU. This would be the guest XML part: <cachetune vcpus='0-3'> <cache id='0' level='3' type='both' size='3' unit='MiB'/> </cachetune> And the resulting mask can be for example: 0x00F00 Which would lead into this schemata file: L3:0=00F00 Nice catch, however I would rather refactor the virResctrlAllocGetType function to always allocate it correctly or cleanup after itself it the allocation fails. For example instead of this code: if (!resctrl->levels[level] && (VIR_ALLOC(resctrl->levels[level]) < 0 || VIR_ALLOC_N(resctrl->levels[level]->types, VIR_CACHE_TYPE_LAST) < 0)) return NULL; we can have something like this: if (!resctrl->levels[level]) { virResctrlAllocPerTypePtr *types = NULL; if (VIR_ALLOC_N(types, VIR_CACHE_TYPE_LAST) < 0) return NULL; if (VIR_ALLOC(resctrl->levels[level]) < 0) { VIR_FREE(types); return NULL; } resctrl->levels[level]->types = types; } Well, we kind of need to write to /sys/fs/resctrl to configure the cache allocation in the host for the guest that is about to start. This shouldn't ever happen and if it does, something is seriously wrong since the mask is stored in /sys/fs/resctrl and it should already ensure that everything is correctly formatted. I have to agree that following this code is extremely hard, perhaps splitting it into several functions would help to review this madness :). As I've wrote, we need to write to /sys/fs/resctrl in order to apply the cache allocation for a guest. This is not saving the "guest data" anywhere, that's already done by having it configured in the guest XML. I was able to find some documentation: <https://github.com/intel/intel-cmt-cat/wiki/resctrl> Pavel

More info in the documentation, this is basically the XML parsing/formatting support, schemas, tests and documentation for the new cputune/cachetune element that will get used by following patches. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- docs/formatdomain.html.in | 54 +++++ docs/schemas/domaincommon.rng | 32 +++ src/conf/domain_conf.c | 251 +++++++++++++++++++++ src/conf/domain_conf.h | 13 ++ tests/genericxml2xmlindata/cachetune-cdp.xml | 36 +++ .../cachetune-colliding-allocs.xml | 30 +++ .../cachetune-colliding-tunes.xml | 32 +++ .../cachetune-colliding-types.xml | 30 +++ tests/genericxml2xmlindata/cachetune-small.xml | 29 +++ tests/genericxml2xmlindata/cachetune.xml | 33 +++ tests/genericxml2xmltest.c | 10 + 11 files changed, 550 insertions(+) create mode 100644 tests/genericxml2xmlindata/cachetune-cdp.xml create mode 100644 tests/genericxml2xmlindata/cachetune-colliding-allocs.xml create mode 100644 tests/genericxml2xmlindata/cachetune-colliding-tunes.xml create mode 100644 tests/genericxml2xmlindata/cachetune-colliding-types.xml create mode 100644 tests/genericxml2xmlindata/cachetune-small.xml create mode 100644 tests/genericxml2xmlindata/cachetune.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 01db83e60820..623860cc0e95 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -689,6 +689,10 @@ &lt;iothread_quota&gt;-1&lt;/iothread_quota&gt; &lt;vcpusched vcpus='0-4,^3' scheduler='fifo' priority='1'/&gt; &lt;iothreadsched iothreads='2' scheduler='batch'/&gt; + &lt;cachetune vcpus='0-3'&gt; + &lt;cache id='0' level='3' type='both' size='3' unit='MiB'/&gt; + &lt;cache id='1' level='3' type='both' size='3' unit='MiB'/&gt; + &lt;/cachetune&gt; &lt;/cputune&gt; ... &lt;/domain&gt; @@ -834,6 +838,56 @@ <span class="since">Since 1.2.13</span> </dd> + <dt><code>cachetune</code><span class="since">Since 3.10.0</span></dt> + <dd> + Optional <code>cachetune</code> element can control allocations for CPU + caches using the resctrlfs on the host. Whether or not is this supported + can be gathered from capabilities where some limitations like minimum + size and required granularity are reported as well. The required + attribute <code>vcpus</code> specifies to which vCPUs this allocation + applies. A vCPU can only be member of one <code>cachetune</code> element + allocations. Supported subelements are: + <dl> + <dt><code>cache</code></dt> + <dd> + This element controls the allocation of CPU cache and has the + following attributes: + <dl> + <dt><code>level</code></dt> + <dd> + Host cache level from which to allocate. + </dd> + <dt><code>id</code></dt> + <dd> + Host cache id from which to allocate. + </dd> + <dt><code>type</code></dt> + <dd> + Type of allocation. Can be <code>code</code> for code + (instructions), <code>data</code> for data or <code>both</code> + for both code and data (unified). Currently the allocation can + be done only with the same type as the host supports, meaning + you cannot request <code>both</code> for host with CDP + (code/data prioritization) enabled. + </dd> + <dt><code>size</code></dt> + <dd> + The size of the region to allocate. The value by default is in + bytes, but the <code>unit</code> attribute can be used to scale + the value. + </dd> + <dt><code>unit</code> (optional)</dt> + <dd> + If specified it is the unit such as KiB, MiB, GiB, or TiB + (described in the <code>memory</code> element + for <a href="#elementsMemoryAllocation">Memory Allocation</a>) + in which <code>size</code> is specified, defaults to bytes. + </dd> + </dl> + </dd> + </dl> + + </dd> </dl> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index f22c932f6c09..252f58f4379c 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -900,6 +900,38 @@ <ref name="schedparam"/> </element> </zeroOrMore> + <zeroOrMore> + <element name="cachetune"> + <attribute name="vcpus"> + <ref name='cpuset'/> + </attribute> + <oneOrMore> + <element name="cache"> + <attribute name="id"> + <ref name='unsignedInt'/> + </attribute> + <attribute name="level"> + <ref name='unsignedInt'/> + </attribute> + <attribute name="type"> + <choice> + <value>both</value> + <value>code</value> + <value>data</value> + </choice> + </attribute> + <attribute name="size"> + <ref name='unsignedLong'/> + </attribute> + <optional> + <attribute name='unit'> + <ref name='unit'/> + </attribute> + </optional> + </element> + </oneOrMore> + </element> + </zeroOrMore> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 66e21c4bdbee..ec8d760c7971 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2883,6 +2883,17 @@ virDomainLoaderDefFree(virDomainLoaderDefPtr loader) VIR_FREE(loader); } +static void +virDomainCachetuneDefFree(virDomainCachetuneDefPtr cachetune) +{ + if (!cachetune) + return; + + virObjectUnref(cachetune->alloc); + virBitmapFree(cachetune->vcpus); + VIR_FREE(cachetune); +} + void virDomainDefFree(virDomainDefPtr def) { size_t i; @@ -3055,6 +3066,10 @@ void virDomainDefFree(virDomainDefPtr def) virDomainShmemDefFree(def->shmems[i]); VIR_FREE(def->shmems); + for (i = 0; i < def->ncachetunes; i++) + virDomainCachetuneDefFree(def->cachetunes[i]); + VIR_FREE(def->cachetunes); + VIR_FREE(def->keywrap); if (def->namespaceData && def->ns.free) @@ -18202,6 +18217,165 @@ virDomainDefParseBootOptions(virDomainDefPtr def, } +static int +virDomainCachetuneDefParseCache(xmlXPathContextPtr ctxt, + xmlNodePtr node, + virResctrlAllocPtr alloc) +{ + xmlNodePtr oldnode = ctxt->node; + unsigned int level; + unsigned int cache; + int type; + unsigned long long size; + char *tmp = NULL; + int ret = -1; + + ctxt->node = node; + + tmp = virXMLPropString(node, "id"); + if (!tmp) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing cachetune attribute 'id'")); + goto cleanup; + } + if (virStrToLong_uip(tmp, NULL, 10, &cache) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid cachetune attribute 'id' value '%s'"), + tmp); + goto cleanup; + } + VIR_FREE(tmp); + + tmp = virXMLPropString(node, "level"); + if (!tmp) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing cachetune attribute 'level'")); + goto cleanup; + } + if (virStrToLong_uip(tmp, NULL, 10, &level) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid cachetune attribute 'level' value '%s'"), + tmp); + goto cleanup; + } + VIR_FREE(tmp); + + tmp = virXMLPropString(node, "type"); + if (!tmp) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing cachetune attribute 'type'")); + goto cleanup; + } + type = virCacheTypeFromString(tmp); + if (type < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid cachetune attribute 'type' value '%s'"), + tmp); + goto cleanup; + } + VIR_FREE(tmp); + + if (virDomainParseScaledValue("./@size", "./@unit", + ctxt, &size, 1024, + ULLONG_MAX, true) < 0) + goto cleanup; + + if (virResctrlAllocSetSize(alloc, level, type, cache, size) < 0) + goto cleanup; + + ret = 0; + cleanup: + ctxt->node = oldnode; + VIR_FREE(tmp); + return ret; +} + + +static int +virDomainCachetuneDefParse(virDomainDefPtr def, + xmlXPathContextPtr ctxt, + xmlNodePtr node) +{ + xmlNodePtr oldnode = ctxt->node; + xmlNodePtr *nodes = NULL; + virBitmapPtr vcpus = NULL; + virResctrlAllocPtr alloc = virResctrlAllocNew(); + virDomainCachetuneDefPtr tmp_cachetune = NULL; + char *tmp = NULL; + char *vcpus_str = NULL; + ssize_t i = 0; + int n; + int ret = -1; + + ctxt->node = node; + + if (!alloc) + goto cleanup; + + if (VIR_ALLOC(tmp_cachetune) < 0) + goto cleanup; + + vcpus_str = virXMLPropString(node, "vcpus"); + if (!vcpus_str) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing cachetune attribute 'vcpus'")); + goto cleanup; + } + if (virBitmapParse(vcpus_str, &vcpus, VIR_DOMAIN_CPUMASK_LEN) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid cachetune attribute 'vcpus' value '%s'"), + vcpus_str); + goto cleanup; + } + + if ((n = virXPathNodeSet("./cache", ctxt, &nodes)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot extract cache nodes under cachetune")); + goto cleanup; + } + + for (i = 0; i < n; i++) { + if (virDomainCachetuneDefParseCache(ctxt, nodes[i], alloc) < 0) + goto cleanup; + } + + if (virResctrlAllocIsEmpty(alloc)) { + ret = 0; + goto cleanup; + } + + virBitmapShrink(vcpus, def->maxvcpus); + + for (i = 0; i < def->ncachetunes; i++) { + if (virBitmapOverlaps(def->cachetunes[i]->vcpus, vcpus)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Overlapping vcpus in cachetunes")); + goto cleanup; + } + } + + if (virResctrlAllocSetID(alloc, vcpus_str) < 0) + goto cleanup; + + VIR_STEAL_PTR(tmp_cachetune->vcpus, vcpus); + VIR_STEAL_PTR(tmp_cachetune->alloc, alloc); + + if (VIR_APPEND_ELEMENT(def->cachetunes, def->ncachetunes, tmp_cachetune) < 0) + goto cleanup; + + ret = 0; + cleanup: + ctxt->node = oldnode; + virDomainCachetuneDefFree(tmp_cachetune); + virObjectUnref(alloc); + virBitmapFree(vcpus); + VIR_FREE(vcpus_str); + VIR_FREE(nodes); + VIR_FREE(tmp); + return ret; +} + + static virDomainDefPtr virDomainDefParseXML(xmlDocPtr xml, xmlNodePtr root, @@ -18754,6 +18928,18 @@ virDomainDefParseXML(xmlDocPtr xml, } VIR_FREE(nodes); + if ((n = virXPathNodeSet("./cputune/cachetune", ctxt, &nodes)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot extract cachetune nodes")); + goto error; + } + + for (i = 0; i < n; i++) { + if (virDomainCachetuneDefParse(def, ctxt, nodes[i]) < 0) + goto error; + } + VIR_FREE(nodes); + if (virCPUDefParseXML(ctxt, "./cpu[1]", VIR_CPU_TYPE_GUEST, &def->cpu) < 0) goto error; @@ -25666,6 +25852,68 @@ virDomainSchedulerFormat(virBufferPtr buf, } +struct virCachetuneHelperData { + virBufferPtr buf; + size_t vcpu_id; +}; + +static int +virDomainCachetuneDefFormatHelper(unsigned int level, + virCacheType type, + unsigned int cache, + unsigned long long size, + void *opaque) +{ + const char *unit; + virBufferPtr buf = opaque; + unsigned long long short_size = virFormatIntPretty(size, &unit); + + virBufferAsprintf(buf, + "<cache id='%u' level='%u' type='%s' " + "size='%llu' unit='%s'/>\n", + cache, level, virCacheTypeToString(type), + short_size, unit); + + return 0; +} + + +static int +virDomainCachetuneDefFormat(virBufferPtr buf, + virDomainCachetuneDefPtr cachetune) +{ + virBuffer childrenBuf = VIR_BUFFER_INITIALIZER; + char *vcpus = NULL; + int ret = -1; + + virBufferSetChildIndent(&childrenBuf, buf); + virResctrlAllocForeachSize(cachetune->alloc, + virDomainCachetuneDefFormatHelper, + &childrenBuf); + + + if (virBufferCheckError(&childrenBuf) < 0) + goto cleanup; + + if (!virBufferUse(&childrenBuf)) { + ret = 0; + goto cleanup; + } + + vcpus = virBitmapFormat(cachetune->vcpus); + + virBufferAsprintf(buf, "<cachetune vcpus='%s'>\n", vcpus); + virBufferAddBuffer(buf, &childrenBuf); + virBufferAddLit(buf, "</cachetune>\n"); + + ret = 0; + cleanup: + virBufferFreeAndReset(&childrenBuf); + VIR_FREE(vcpus); + return ret; +} + + static int virDomainCputuneDefFormat(virBufferPtr buf, virDomainDefPtr def) @@ -25767,6 +26015,9 @@ virDomainCputuneDefFormat(virBufferPtr buf, def->iothreadids[i]->iothread_id); } + for (i = 0; i < def->ncachetunes; i++) + virDomainCachetuneDefFormat(&childrenBuf, def->cachetunes[i]); + if (virBufferCheckError(&childrenBuf) < 0) return -1; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 59f250ac96ce..f5f6aed82e54 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -56,6 +56,7 @@ # include "virperf.h" # include "virtypedparam.h" # include "virsavecookie.h" +# include "virresctrl.h" /* forward declarations of all device types, required by * virDomainDeviceDef @@ -2194,6 +2195,15 @@ struct _virDomainCputune { }; +typedef struct _virDomainCachetuneDef virDomainCachetuneDef; +typedef virDomainCachetuneDef *virDomainCachetuneDefPtr; + +struct _virDomainCachetuneDef { + virBitmapPtr vcpus; + virResctrlAllocPtr alloc; +}; + + typedef struct _virDomainVcpuDef virDomainVcpuDef; typedef virDomainVcpuDef *virDomainVcpuDefPtr; @@ -2322,6 +2332,9 @@ struct _virDomainDef { virDomainCputune cputune; + virDomainCachetuneDefPtr *cachetunes; + size_t ncachetunes; + virDomainNumaPtr numa; virDomainResourceDefPtr resource; virDomainIdMapDef idmap; diff --git a/tests/genericxml2xmlindata/cachetune-cdp.xml b/tests/genericxml2xmlindata/cachetune-cdp.xml new file mode 100644 index 000000000000..1331aad06e54 --- /dev/null +++ b/tests/genericxml2xmlindata/cachetune-cdp.xml @@ -0,0 +1,36 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>2</vcpu> + <cputune> + <cachetune vcpus='0-1'> + <cache id='0' level='3' type='code' size='7680' unit='KiB'/> + <cache id='1' level='3' type='data' size='3840' unit='KiB'/> + </cachetune> + <cachetune vcpus='2'> + <cache id='1' level='3' type='code' size='6' unit='MiB'/> + </cachetune> + <cachetune vcpus='3'> + <cache id='1' level='3' type='data' size='6912' unit='KiB'/> + </cachetune> + </cputune> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/genericxml2xmlindata/cachetune-colliding-allocs.xml b/tests/genericxml2xmlindata/cachetune-colliding-allocs.xml new file mode 100644 index 000000000000..994f8fcf2a4e --- /dev/null +++ b/tests/genericxml2xmlindata/cachetune-colliding-allocs.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>2</vcpu> + <cputune> + <cachetune vcpus='0'> + <cache id='0' level='3' type='code' size='12' unit='KiB'/> + <cache id='0' level='3' type='code' size='18' unit='KiB'/> + </cachetune> + </cputune> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/genericxml2xmlindata/cachetune-colliding-tunes.xml b/tests/genericxml2xmlindata/cachetune-colliding-tunes.xml new file mode 100644 index 000000000000..3d9db85b12d4 --- /dev/null +++ b/tests/genericxml2xmlindata/cachetune-colliding-tunes.xml @@ -0,0 +1,32 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>2</vcpu> + <cputune> + <cachetune vcpus='0'> + <cache id='0' level='3' type='code' size='12' unit='KiB'/> + </cachetune> + <cachetune vcpus='0'> + <cache id='0' level='3' type='data' size='18' unit='KiB'/> + </cachetune> + </cputune> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/genericxml2xmlindata/cachetune-colliding-types.xml b/tests/genericxml2xmlindata/cachetune-colliding-types.xml new file mode 100644 index 000000000000..1a4e62cd17d3 --- /dev/null +++ b/tests/genericxml2xmlindata/cachetune-colliding-types.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>2</vcpu> + <cputune> + <cachetune vcpus='0-1'> + <cache id='0' level='3' type='both' size='12' unit='KiB'/> + <cache id='0' level='3' type='code' size='6' unit='KiB'/> + </cachetune> + </cputune> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/genericxml2xmlindata/cachetune-small.xml b/tests/genericxml2xmlindata/cachetune-small.xml new file mode 100644 index 000000000000..4b25490b3abb --- /dev/null +++ b/tests/genericxml2xmlindata/cachetune-small.xml @@ -0,0 +1,29 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>2</vcpu> + <cputune> + <cachetune vcpus='0-1'> + <cache id='0' level='3' type='both' size='768' unit='KiB'/> + </cachetune> + </cputune> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/genericxml2xmlindata/cachetune.xml b/tests/genericxml2xmlindata/cachetune.xml new file mode 100644 index 000000000000..0051410aec32 --- /dev/null +++ b/tests/genericxml2xmlindata/cachetune.xml @@ -0,0 +1,33 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>2</vcpu> + <cputune> + <cachetune vcpus='0-1'> + <cache id='0' level='3' type='both' size='3' unit='MiB'/> + <cache id='1' level='3' type='both' size='3' unit='MiB'/> + </cachetune> + <cachetune vcpus='3'> + <cache id='0' level='3' type='both' size='3' unit='MiB'/> + </cachetune> + </cputune> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c index deaf5bd9b823..496e9260fa86 100644 --- a/tests/genericxml2xmltest.c +++ b/tests/genericxml2xmltest.c @@ -130,6 +130,16 @@ mymain(void) DO_TEST_FULL("chardev-reconnect-invalid-mode", 0, false, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); + DO_TEST("cachetune"); + DO_TEST("cachetune-small"); + DO_TEST("cachetune-cdp"); + DO_TEST_FULL("cachetune-colliding-allocs", false, true, + TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); + DO_TEST_FULL("cachetune-colliding-tunes", false, true, + TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); + DO_TEST_FULL("cachetune-colliding-types", false, true, + TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); + virObjectUnref(caps); virObjectUnref(xmlopt); -- 2.15.1

On Wed, Jan 03, 2018 at 06:50:22PM -0500, John Ferlan wrote: If you search for Resource Control File System you will find lot of unrelevant stuff. For resctrlfs (or rather resctrl), however, you will get the precise info you are looking for. It's also called resctrl in the kernel. You mount it by doing `mount -t resctrl resctrl [-o cdp] /sys/fs/resctrl`, so I think it's clearer this way (or without the fs at the end). We don't. We don't mention any other sysfs path in the docs. Why would we? It's defined elsewhere, not in libvirt. And since it's the only place to be mounted for now (according to the docs) we can have it hardcoded. Just like the other paths. I don't see the point in that. Especially since it's called `cache` in the code. This will look awkward when other functions around have one only, but I've already given up this fight. 1) Because that way we can default to something else in the future. We want to specify everything possible, otherwise we'll get stuck with something we can't change. 2) All other code does that, it's consistent. 3) It's more readable for users even without reading the docs. 4) It would be more code with no useful output. Sure, this will be shrunk and forgotten. I have to send another version anyway. Not only because virCachetuneHelperData is not used at all, but other things need adjustments.

This test initializes capabilities from vircaps2xmldata (since it exists there already) and then requests list of free bitmaps (all unallocated space) from virresctrl.c Desirable outputs are saved in virresctrldata. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- tests/Makefile.am | 8 +- tests/virresctrldata/resctrl-cdp.schemata | 2 + .../virresctrldata/resctrl-skx-twocaches.schemata | 1 + tests/virresctrldata/resctrl-skx.schemata | 1 + tests/virresctrldata/resctrl.schemata | 1 + tests/virresctrltest.c | 102 +++++++++++++++++++++ 6 files changed, 114 insertions(+), 1 deletion(-) create mode 100644 tests/virresctrldata/resctrl-cdp.schemata create mode 100644 tests/virresctrldata/resctrl-skx-twocaches.schemata create mode 100644 tests/virresctrldata/resctrl-skx.schemata create mode 100644 tests/virresctrldata/resctrl.schemata create mode 100644 tests/virresctrltest.c diff --git a/tests/Makefile.am b/tests/Makefile.am index 3441dab6f6bb..d045073726f0 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -232,6 +232,7 @@ if WITH_LINUX test_programs += fchosttest test_programs += scsihosttest test_programs += vircaps2xmltest +test_programs += virresctrltest test_libraries += virusbmock.la \ virnetdevbandwidthmock.la \ virnumamock.la \ @@ -1165,8 +1166,13 @@ virnumamock_la_SOURCES = \ virnumamock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) virnumamock_la_LIBADD = $(MOCKLIBS_LIBS) +virresctrltest_SOURCES = \ + virresctrltest.c testutils.h testutils.c virfilewrapper.h virfilewrapper.c +virresctrltest_LDADD = $(LDADDS) + else ! WITH_LINUX -EXTRA_DIST += vircaps2xmltest.c virnumamock.c virfilewrapper.c virfilewrapper.h +EXTRA_DIST += vircaps2xmltest.c virnumamock.c virfilewrapper.c \ + virfilewrapper.h virresctrltest.c endif ! WITH_LINUX if WITH_NSS diff --git a/tests/virresctrldata/resctrl-cdp.schemata b/tests/virresctrldata/resctrl-cdp.schemata new file mode 100644 index 000000000000..2897e2afa603 --- /dev/null +++ b/tests/virresctrldata/resctrl-cdp.schemata @@ -0,0 +1,2 @@ +L3CODE:0=00ffc;1=0ff00 +L3DATA:0=00000;1=03fff diff --git a/tests/virresctrldata/resctrl-skx-twocaches.schemata b/tests/virresctrldata/resctrl-skx-twocaches.schemata new file mode 100644 index 000000000000..86b3801a04c2 --- /dev/null +++ b/tests/virresctrldata/resctrl-skx-twocaches.schemata @@ -0,0 +1 @@ +L3:0=001;1=400 diff --git a/tests/virresctrldata/resctrl-skx.schemata b/tests/virresctrldata/resctrl-skx.schemata new file mode 100644 index 000000000000..5e8b0d636277 --- /dev/null +++ b/tests/virresctrldata/resctrl-skx.schemata @@ -0,0 +1 @@ +L3:0=70f diff --git a/tests/virresctrldata/resctrl.schemata b/tests/virresctrldata/resctrl.schemata new file mode 100644 index 000000000000..fa980e58c9dd --- /dev/null +++ b/tests/virresctrldata/resctrl.schemata @@ -0,0 +1 @@ +L3:0=000ff;1=000f0 diff --git a/tests/virresctrltest.c b/tests/virresctrltest.c new file mode 100644 index 000000000000..80d0e82e7cbf --- /dev/null +++ b/tests/virresctrltest.c @@ -0,0 +1,102 @@ +#include <config.h> +#include <stdlib.h> + +#include "testutils.h" +#include "virfilewrapper.h" +#include "virresctrlpriv.h" + + +#define VIR_FROM_THIS VIR_FROM_NONE + +struct virResctrlData { + const char *filename; + bool fail; +}; + + +static int +test_virResctrlGetFree(const void *opaque) +{ + struct virResctrlData *data = (struct virResctrlData *) opaque; + char *system_dir = NULL; + char *resctrl_dir = NULL; + int ret = -1; + virResctrlAllocPtr alloc = NULL; + char *schemata_str = NULL; + char *schemata_file; + virCapsPtr caps = NULL; + + if (virAsprintf(&system_dir, "%s/vircaps2xmldata/linux-%s/system", + abs_srcdir, data->filename) < 0) + goto cleanup; + + if (virAsprintf(&resctrl_dir, "%s/vircaps2xmldata/linux-%s/resctrl", + abs_srcdir, data->filename) < 0) + goto cleanup; + + if (virAsprintf(&schemata_file, "%s/virresctrldata/%s.schemata", + abs_srcdir, data->filename) < 0) + goto cleanup; + + virFileWrapperAddPrefix("/sys/devices/system", system_dir); + virFileWrapperAddPrefix("/sys/fs/resctrl", resctrl_dir); + + caps = virCapabilitiesNew(VIR_ARCH_X86_64, false, false); + if (!caps || virCapabilitiesInitCaches(caps) < 0) { + fprintf(stderr, "Could not initialize capabilities"); + goto cleanup; + } + + alloc = virResctrlAllocGetFree(caps->host.resctrl); + + virFileWrapperClearPrefixes(); + + if (!alloc) { + if (data->fail) + ret = 0; + goto cleanup; + } else if (data->fail) { + VIR_TEST_DEBUG("Error expected but there wasn't any.\n"); + ret = -1; + goto cleanup; + } + + schemata_str = virResctrlAllocFormat(alloc); + + if (virTestCompareToFile(schemata_str, schemata_file) < 0) + goto cleanup; + + ret = 0; + cleanup: + virObjectUnref(caps); + virObjectUnref(alloc); + VIR_FREE(system_dir); + VIR_FREE(resctrl_dir); + VIR_FREE(schemata_str); + VIR_FREE(schemata_file); + return ret; +} + + +static int +mymain(void) +{ + struct virResctrlData data = {0}; + int ret = 0; + +#define DO_TEST_FREE(_filename) \ + do { \ + data = (struct virResctrlData) { .filename = _filename }; \ + if (virTestRun("Free: " _filename, test_virResctrlGetFree, &data) < 0) \ + ret = -1; \ + } while (0) + + DO_TEST_FREE("resctrl"); + DO_TEST_FREE("resctrl-cdp"); + DO_TEST_FREE("resctrl-skx"); + DO_TEST_FREE("resctrl-skx-twocaches"); + + return ret; +} + +VIR_TEST_MAIN(mymain) -- 2.15.1

Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/qemu/qemu_process.c | 61 +++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 56 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a0f430f89f06..8abd9a5a8da3 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2507,6 +2507,33 @@ qemuProcessSetupEmulator(virDomainObjPtr vm) } +static int +qemuProcessResctrlCreate(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + int ret = -1; + size_t i = 0; + virCapsPtr caps = virQEMUDriverGetCapabilities(driver, false); + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (!caps) + return -1; + + for (i = 0; i < vm->def->ncachetunes; i++) { + if (virResctrlAllocCreate(caps->host.resctrl, + vm->def->cachetunes[i]->alloc, + "qemu", + priv->machineName) < 0) + goto cleanup; + } + + ret = 0; + cleanup: + virObjectUnref(caps); + return ret; +} + + static int qemuProcessInitPasswords(virConnectPtr conn, virQEMUDriverPtr driver, @@ -5013,12 +5040,26 @@ qemuProcessSetupVcpu(virDomainObjPtr vm, { pid_t vcpupid = qemuDomainGetVcpuPid(vm, vcpuid); virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(vm->def, vcpuid); + size_t i = 0; - return qemuProcessSetupPid(vm, vcpupid, VIR_CGROUP_THREAD_VCPU, - vcpuid, vcpu->cpumask, - vm->def->cputune.period, - vm->def->cputune.quota, - &vcpu->sched); + if (qemuProcessSetupPid(vm, vcpupid, VIR_CGROUP_THREAD_VCPU, + vcpuid, vcpu->cpumask, + vm->def->cputune.period, + vm->def->cputune.quota, + &vcpu->sched) < 0) + return -1; + + for (i = 0; i < vm->def->ncachetunes; i++) { + virDomainCachetuneDefPtr ct = vm->def->cachetunes[i]; + + if (virBitmapIsBitSet(ct->vcpus, vcpuid)) { + if (virResctrlAllocAddPID(ct->alloc, vcpupid) < 0) + return -1; + break; + } + } + + return 0; } @@ -5891,6 +5932,10 @@ qemuProcessLaunch(virConnectPtr conn, if (qemuProcessSetupEmulator(vm) < 0) goto cleanup; + VIR_DEBUG("Setting up resctrlfs"); + if (qemuProcessResctrlCreate(driver, vm) < 0) + goto cleanup; + VIR_DEBUG("Setting domain security labels"); if (qemuSecuritySetAllLabel(driver, vm, @@ -6539,6 +6584,12 @@ void qemuProcessStop(virQEMUDriverPtr driver, vm->def->name); } + /* Remove resctrl allocation after cgroups are cleaned up which makes it + * kind of safer (although removing the allocation should work even with + * pids in tasks file */ + for (i = 0; i < vm->def->ncachetunes; i++) + virResctrlAllocRemove(vm->def->cachetunes[i]->alloc); + qemuProcessRemoveDomainStatus(driver, vm); /* Remove VNC and Spice ports from port reservation bitmap, but only if -- 2.15.1

On 12/13/2017 10:39 AM, Martin Kletzander wrote: Just for completeness... Obviously this will need final tweaks... So far so good though... John