Re: [libvirt] [PATCH v2 RESEND 07/17] util: Add MBA schemata parse and format methods
On 07/29/2018 11:12 PM, bing.niu(a)intel.com wrote:
From: Bing Niu <bing.niu(a)intel.com>
Introduce virResctrlAllocMemoryBandwidthFormat and
virResctrlAllocParseMemoryBandwidthLine which will format
and parse an entry in the schemata file for MBA.
Signed-off-by: Bing Niu <bing.niu(a)intel.com>
---
src/util/virresctrl.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 141 insertions(+)
diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index 8a25798..1cbf9b3 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -986,6 +986,139 @@ virResctrlAllocGetID(virResctrlAllocPtr alloc)
}
+/* Format the Memory Bandwidth Allocation line that will be found in
+ * the schemata files. The line should be start with "MB:" and be
+ * followed by "id=value" pairs separated by a colon such as:
semi-colon
+ *
+ * MB:0=100;1=100
+ *
+ * which indicates node id 0 has 100 percent bandwith and node id 1
+ * has 100 percent bandwidth
s/$/. A trailing semi-colon is not formatted./
+ */
[...]
> +/* Parse a schemata formatted MB: entry. Format details are described in
> + * virResctrlAllocMemoryBandwidthFormat.
+ */
> +static int
> +virResctrlAllocParseMemoryBandwidthLine(virResctrlInfoPtr resctrl,
> + virResctrlAllocPtr alloc,
> + char *line)
> +{
> + char **mbs = NULL;
> + char *tmp = NULL;
> + size_t nmbs = 0;
> + size_t i;
> + int ret = -1;
> +
> + /* For no reason there can be spaces */
> + virSkipSpaces((const char **) &line);
> +
> + if (STRNEQLEN(line, "MB", 2))
> + return 0;
> +
> + if (!resctrl || !resctrl->membw_info ||
> + !resctrl->membw_info->min_bandwidth ||
> + !resctrl->membw_info->bandwidth_granularity) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Missing or inconsistent resctrl info for "
> + "memory bandwidth allocation"));
I assume a return -1 is in order here.
Missed this earlier, but the Coverity checker found it.
+ }
+
+ if (!alloc->mem_bw) {
+ if (VIR_ALLOC(alloc->mem_bw) < 0)
+ return -1;
+ }
+
+ tmp = strchr(line, ':');
+ if (!tmp)
+ return 0;
+ tmp++;
+
+ mbs = virStringSplitCount(tmp, ";", 0, &nmbs);
+ if (nmbs == 0)
+ return 0;
As strange as this is - the above 2 aren't necessary given the next for
loop. Keeping them cause Coverity to whine that virStringSplitCount can
return allocated memory which is then leaked. It's a false positive, but
avoidable.
+
+ for (i = 0; i < nmbs; i++) {
+ if (virResctrlAllocParseProcessMemoryBandwidth(resctrl, alloc, mbs[i]) < 0)
+ goto cleanup;
+ }
+ ret = 0;
+ cleanup:
+ virStringListFree(mbs);
+ return ret;
+}
+
+
[...]
I'll adjust in my branch before pushing.
John