Re: [libvirt] [PATCH v2 RESEND 07/17] util: Add MBA schemata parse and format methods
On 2018年07月31日 06:09, John Ferlan wrote:
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)
[....]
> +/* 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