[libvirt] [PATCH v2 RESEND 07/17] util: Add MBA schemata parse and format methods

[John Ferlan]

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