[libvirt] [PATCHv7 15/18] conf: Add 'id' to virDomainResctrlDef

[Wang, Huaqiang]

On 11/6/2018 3:15 AM, John Ferlan wrote:
> On 10/22/18 4:01 AM, Wang Huaqiang wrote:
>> Adding element 'id' to virDomainResctrlDef tracking resource group
>> id, it reflects the attribute 'id' of of element <cachetune> in XML.
>>
>> virResctrlAlloc.id is a copy from virDomanResctrlDef.id.
>>
>> Signed-off-by: Wang Huaqiang<huaqiang.wang at intel.com>
>> ---
>>   src/conf/domain_conf.c | 20 ++++++++------------
>>   src/conf/domain_conf.h |  1 +
>>   2 files changed, 9 insertions(+), 12 deletions(-)
>>
> Not sure I see the need to duplicate the alloc->id value especially
> since it's "invalid" have "resctrl->alloc == NULL", right?
>
> Can this resctrl->id ever be different?  Not that I can see. I see this
> is a desire to reduce an error case in Format processing, but if
> virResctrlAllocGetID returned NULL there's other problems...
>
> John

This patch should be removed and will be removed.

It is introduced in series v4, in that series the 'resctrl->alloc' is 
allowed
to be NULL representing a default monitor.
Now we changed our design and resctrl->alloc is always assigned with an
virResctrlAlloc data structure, then 'resctrl->id' is not necessary. We 
can keep the
resctrl ID in alloc->id.

Thanks for review.
Huaqiang

>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 01f795f..1aecd8c 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -2980,6 +2980,7 @@ virDomainResctrlDefFree(virDomainResctrlDefPtr resctrl)
>>       virObjectUnref(resctrl->alloc);
>>       virBitmapFree(resctrl->vcpus);
>>       VIR_FREE(resctrl->monitors);
>> +    VIR_FREE(resctrl->id);
>>       VIR_FREE(resctrl);
>>   }
>>   
>> @@ -19145,6 +19146,9 @@ virDomainResctrlNew(xmlNodePtr node,
>>       if (VIR_ALLOC(resctrl) < 0)
>>           goto cleanup;
>>   
>> +    if (VIR_STRDUP(resctrl->id, alloc_id) < 0)
>> +        goto cleanup;
>> +
>>       if (!(resctrl->vcpus = virBitmapNewCopy(*vcpus))) {
>>           virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>>                          _("failed to copy 'vcpus'"));
>> @@ -27323,13 +27327,9 @@ virDomainCachetuneDefFormat(virBufferPtr buf,
>>   
>>       virBufferAsprintf(buf, "<cachetune vcpus='%s'", vcpus);
>>   
>> -    if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)) {
>> -        const char *alloc_id = virResctrlAllocGetID(resctrl->alloc);
>> -        if (!alloc_id)
>> -            goto cleanup;
>> +    if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))
>> +        virBufferAsprintf(buf, " id='%s'", resctrl->id);
>>   
>> -        virBufferAsprintf(buf, " id='%s'", alloc_id);
>> -    }
>>       virBufferAddLit(buf, ">\n");
>>   
>>       virBufferAddBuffer(buf, &childrenBuf);
>> @@ -27386,13 +27386,9 @@ virDomainMemorytuneDefFormat(virBufferPtr buf,
>>   
>>       virBufferAsprintf(buf, "<memorytune vcpus='%s'", vcpus);
>>   
>> -    if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)) {
>> -        const char *alloc_id = virResctrlAllocGetID(resctrl->alloc);
>> -        if (!alloc_id)
>> -            goto cleanup;
>> +    if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))
>> +        virBufferAsprintf(buf, " id='%s'", resctrl->id);
>>   
>> -        virBufferAsprintf(buf, " id='%s'", alloc_id);
>> -    }
>>       virBufferAddLit(buf, ">\n");
>>   
>>       virBufferAddBuffer(buf, &childrenBuf);
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index 60f6464..e190aa2 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -2248,6 +2248,7 @@ typedef struct _virDomainResctrlDef virDomainResctrlDef;
>>   typedef virDomainResctrlDef *virDomainResctrlDefPtr;
>>   
>>   struct _virDomainResctrlDef {
>> +    char *id;
>>       virBitmapPtr vcpus;
>>       virResctrlAllocPtr alloc;
>>   
>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20181112/d845b4cf/attachment-0001.htm>