On 2018年07月31日 06:09, John Ferlan wrote:


On 07/29/2018 11:12 PM, bing....@intel.com wrote:
From: Bing Niu <bing....@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....@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)
[....]

+/* 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.


Yes. I miss this and should return -1 here.
+    }
+
+    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.


Yes my bad, sorry.
I saw virStringSplitCount will return a pointer array even 0 delim found. And this pointer array need to do a virStringListFree.

Should change like:
  mbs = virStringSplitCount(tmp, ";", 0, &nmbs);
  if (mbs == 0)
      return 0;
Bing
+
+    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


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to