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

[bing.niu]

On 2018年07月31日 06:09, John Ferlan wrote:
> 
> 
> 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)
[....]
> 
>> +/* 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
>