[libvirt] [RFCv2 PATCH 4/5] conf: Introduce cputune/memorytune to support memory bandwidth allocation
bing.niu
bing.niu at intel.com
Fri Jul 6 08:41:41 UTC 2018
On 2018年06月30日 06:47, John Ferlan wrote:
>
>
> On 06/15/2018 05:29 AM, bing.niu at intel.com wrote:
>> From: Bing Niu <bing.niu at intel.com>
>>
>> Introduce a new section memorytune to support memory bandwidth allocation.
>> This is consistent with existing cachetune . As the example
>> below:
>>
>> <memorytune vcpus='0'>
>> <node id='0' bandwidth='30'/>
>> </memorytune>
>
> The formatting above needed to be cleaned up.
OK!
>
>>
>> id --- on which node memory bandwidth to be set
>> bandwidth --- the memory bandwidth percent to set.
>>
>> Signed-off-by: Bing Niu <bing.niu at intel.com>
>> ---
>> src/conf/domain_conf.c | 260 +++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 260 insertions(+)
>>
>
> This would seem to be missing docs/schemas/*.rng changes,
> docs/formatdomain.html.in changes, tests/*xml2xmltest.c changes and the
> associated data input/output files, etc.
I will add docs/schemas/*.rng for patch4 and patch5.
Bing
>
> I only scanned what's below - I have too many questions and concerns
> from the previous patches to get too in depth especially since this is
> missing so much stuff. I'm not going to look at patch 5.
>
> John
>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index b3543f3..dbfd69f 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -19076,6 +19076,189 @@ virDomainCachetuneDefParse(virDomainDefPtr def,
>> }
>>
>>
>> +static int
>> +virDomainMemorytuneDefParseMemory(xmlXPathContextPtr ctxt,
>> + xmlNodePtr node,
>> + virResctrlAllocPtr alloc)
>> +{
>> + xmlNodePtr oldnode = ctxt->node;
>> + unsigned int id;
>> + unsigned int bandwidth;
>> + char *tmp = NULL;
>> + int ret = -1;
>> +
>> + ctxt->node = node;
>> +
>> + tmp = virXMLPropString(node, "id");
>> + if (!tmp) {
>> + virReportError(VIR_ERR_XML_ERROR, "%s",
>> + _("Missing memorytune attribute 'id'"));
>> + goto cleanup;
>> + }
>> + if (virStrToLong_uip(tmp, NULL, 10, &id) < 0) {
>> + virReportError(VIR_ERR_XML_ERROR,
>> + _("Invalid memorytune attribute 'id' value '%s'"),
>> + tmp);
>> + goto cleanup;
>> + }
>> + VIR_FREE(tmp);
>> +
>> + tmp = virXMLPropString(node, "bandwidth");
>> + if (!tmp) {
>> + virReportError(VIR_ERR_XML_ERROR, "%s",
>> + _("Missing memorytune attribute 'bandwidth'"));
>> + goto cleanup;
>> + }
>> + if (virStrToLong_uip(tmp, NULL, 10, &bandwidth) < 0) {
>> + virReportError(VIR_ERR_XML_ERROR,
>> + _("Invalid memorytune attribute 'bandwidth' value '%s'"),
>> + tmp);
>> + goto cleanup;
>> + }
>> + VIR_FREE(tmp);
>> + if (virResctrlSetMemoryBandwidth(alloc, id, bandwidth) < 0)
>> + goto cleanup;
>> +
>> + ret = 0;
>> + cleanup:
>> + ctxt->node = oldnode;
>> + VIR_FREE(tmp);
>> + return ret;
>> +}
>> +
>> +
>> +static int
>> +virDomainMemorytuneDefParse(virDomainDefPtr def,
>> + xmlXPathContextPtr ctxt,
>> + xmlNodePtr node,
>> + unsigned int flags)
>> +{
>> + xmlNodePtr oldnode = ctxt->node;
>> + xmlNodePtr *nodes = NULL;
>> + virBitmapPtr vcpus = NULL;
>> + virResctrlAllocPtr alloc = NULL;
>> + virDomainRestuneDefPtr tmp_cachetune = NULL;
>> + char *tmp = NULL;
>> + char *vcpus_str = NULL;
>> + char *alloc_id = NULL;
>> + ssize_t i = 0;
>> + int n;
>> + int ret = -1;
>> + bool new_alloc = false;
>> +
>> + ctxt->node = node;
>> +
>> + if (VIR_ALLOC(tmp_cachetune) < 0)
>> + goto cleanup;
>> +
>> + vcpus_str = virXMLPropString(node, "vcpus");
>> + if (!vcpus_str) {
>> + virReportError(VIR_ERR_XML_ERROR, "%s",
>> + _("Missing memorytune attribute 'vcpus'"));
>> + goto cleanup;
>> + }
>> + if (virBitmapParse(vcpus_str, &vcpus, VIR_DOMAIN_CPUMASK_LEN) < 0) {
>> + virReportError(VIR_ERR_XML_ERROR,
>> + _("Invalid memorytune attribute 'vcpus' value '%s'"),
>> + vcpus_str);
>> + goto cleanup;
>> + }
>> +
>> + /* We need to limit the bitmap to number of vCPUs. If there's nothing left,
>> + * then we can just clean up and return 0 immediately */
>> + virBitmapShrink(vcpus, def->maxvcpus);
>> +
>> + if (virBitmapIsAllClear(vcpus)) {
>> + ret = 0;
>> + goto cleanup;
>> + }
>> +
>> + if ((n = virXPathNodeSet("./node", ctxt, &nodes)) < 0) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> + _("Cannot extract memory nodes under memorytune"));
>> + goto cleanup;
>> + }
>> + for (i = 0; i < def->nrestunes; i++) {
>> + /* vcpus group has been created, directly use the existing one.
>> + * Just updating memory allocation information of that group
>> + * */
>> + if (virBitmapEqual(def->restunes[i]->vcpus, vcpus)) {
>> + alloc = def->restunes[i]->alloc;
>> + break;
>> + }
>> +
>> + /* vcpus can not overlapping */
>> + if (virBitmapOverlaps(def->restunes[i]->vcpus, vcpus)) {
>> + virReportError(VIR_ERR_XML_ERROR, "%s",
>> + _("Overlapping vcpus in memorytunes"));
>> + goto cleanup;
>> + }
>> + }
>> +
>> + if (!alloc) {
>> + alloc = virResctrlAllocNew();
>> + new_alloc = true;
>> + }
>> +
>> + for (i = 0; i < n; i++) {
>> + if (virDomainMemorytuneDefParseMemory(ctxt, nodes[i], alloc) < 0)
>> + goto cleanup;
>> + }
>> + /*
>> + * If this is a new allocation, format ID and append to restune, otherwise
>> + * just update the existing alloc information
>> + * */
>> + if (new_alloc) {
>> + if (virResctrlAllocIsEmpty(alloc)) {
>> + ret = 0;
>> + goto cleanup;
>> + }
>> +
>> +
>> + /* We need to format it back because we need to be consistent in the naming
>> + * even when users specify some "sub-optimal" string there. */
>> + VIR_FREE(vcpus_str);
>> + vcpus_str = virBitmapFormat(vcpus);
>> + if (!vcpus_str)
>> + goto cleanup;
>> +
>> + if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE))
>> + alloc_id = virXMLPropString(node, "id");
>> +
>> + if (!alloc_id) {
>> + /* The number of allocations is limited and the directory structure is flat,
>> + * not hierarchical, so we need to have all same allocations in one
>> + * directory, so it's nice to have it named appropriately. For now it's
>> + * 'vcpus_...' but it's designed in order for it to be changeable in the
>> + * future (it's part of the status XML). */
>> + if (virAsprintf(&alloc_id, "vcpus_%s", vcpus_str) < 0)
>> + goto cleanup;
>> + }
>> +
>> + if (virResctrlAllocSetID(alloc, alloc_id) < 0)
>> + goto cleanup;
>> +
>> + VIR_STEAL_PTR(tmp_cachetune->vcpus, vcpus);
>> + VIR_STEAL_PTR(tmp_cachetune->alloc, alloc);
>> +
>> + if (VIR_APPEND_ELEMENT(def->restunes, def->nrestunes, tmp_cachetune) < 0)
>> + goto cleanup;
>> + }
>> + ret = 0;
>> + cleanup:
>> + ctxt->node = oldnode;
>> + virDomainRestuneDefFree(tmp_cachetune);
>> + if (new_alloc)
>> + virObjectUnref(alloc);
>> + virBitmapFree(vcpus);
>> + VIR_FREE(alloc_id);
>> + VIR_FREE(vcpus_str);
>> + VIR_FREE(nodes);
>> + VIR_FREE(tmp);
>> + return ret;
>> +}
>> +
>> +
>> static virDomainDefPtr
>> virDomainDefParseXML(xmlDocPtr xml,
>> xmlNodePtr root,
>> @@ -19671,6 +19854,18 @@ virDomainDefParseXML(xmlDocPtr xml,
>> }
>> VIR_FREE(nodes);
>>
>> + if ((n = virXPathNodeSet("./cputune/memorytune", ctxt, &nodes)) < 0) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> + _("cannot extract memorytune nodes"));
>> + goto error;
>> + }
>> +
>> + for (i = 0; i < n; i++) {
>> + if (virDomainMemorytuneDefParse(def, ctxt, nodes[i], flags) < 0)
>> + goto error;
>> + }
>> + VIR_FREE(nodes);
>> +
>> if (virCPUDefParseXML(ctxt, "./cpu[1]", VIR_CPU_TYPE_GUEST, &def->cpu) < 0)
>> goto error;
>>
>> @@ -26922,6 +27117,68 @@ virDomainCachetuneDefFormat(virBufferPtr buf,
>>
>>
>> static int
>> +virDomainMemorytuneDefFormatHelper(unsigned int id,
>> + unsigned int bandwidth,
>> + void *opaque)
>> +{
>> + virBufferPtr buf = opaque;
>> +
>> + virBufferAsprintf(buf,
>> + "<node id='%u' bandwidth='%u'/>\n",
>> + id, bandwidth);
>> + return 0;
>> +}
>> +
>> +
>> +static int
>> +virDomainMemorytuneDefFormat(virBufferPtr buf,
>> + virDomainRestuneDefPtr restune,
>> + unsigned int flags)
>> +{
>> + virBuffer childrenBuf = VIR_BUFFER_INITIALIZER;
>> + char *vcpus = NULL;
>> + int ret = -1;
>> +
>> + virBufferSetChildIndent(&childrenBuf, buf);
>> + virResctrlAllocForeachMemory(restune->alloc,
>> + virDomainMemorytuneDefFormatHelper,
>> + &childrenBuf);
>> +
>> +
>> + if (virBufferCheckError(&childrenBuf) < 0)
>> + goto cleanup;
>> +
>> + if (!virBufferUse(&childrenBuf)) {
>> + ret = 0;
>> + goto cleanup;
>> + }
>> +
>> + vcpus = virBitmapFormat(restune->vcpus);
>> + if (!vcpus)
>> + goto cleanup;
>> +
>> + virBufferAsprintf(buf, "<memorytune vcpus='%s'", vcpus);
>> +
>> + if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)) {
>> + const char *alloc_id = virResctrlAllocGetID(restune->alloc);
>> + if (!alloc_id)
>> + goto cleanup;
>> +
>> + virBufferAsprintf(buf, " id='%s'", alloc_id);
>> + }
>> + virBufferAddLit(buf, ">\n");
>> +
>> + virBufferAddBuffer(buf, &childrenBuf);
>> + virBufferAddLit(buf, "</memorytune>\n");
>> +
>> + ret = 0;
>> + cleanup:
>> + virBufferFreeAndReset(&childrenBuf);
>> + VIR_FREE(vcpus);
>> + return ret;
>> +}
>> +
>> +static int
>> virDomainCputuneDefFormat(virBufferPtr buf,
>> virDomainDefPtr def,
>> unsigned int flags)
>> @@ -27026,6 +27283,9 @@ virDomainCputuneDefFormat(virBufferPtr buf,
>> for (i = 0; i < def->nrestunes; i++)
>> virDomainCachetuneDefFormat(&childrenBuf, def->restunes[i], flags);
>>
>> + for (i = 0; i < def->nrestunes; i++)
>> + virDomainMemorytuneDefFormat(&childrenBuf, def->restunes[i], flags);
>> +
>> if (virBufferCheckError(&childrenBuf) < 0)
>> return -1;
>>
>>
More information about the libvir-list
mailing list