Re: [libvirt] [RFCv2 PATCH 3/5] conf: rename cachetune to restune

On 2018年07月06日 06:40, John Ferlan wrote: Hopefully not, and Intel do backward compatible well.:) So overall programing module should be steady. Yup! vCPU actually is a thread of QEMU from host view. Yes, that is a real "challenge". However, kernel modules usually give a consistent and unified interface. That will help a lot. We should be able to support new features by extending the existing logic. You are definitely right! That is how we implement in virDomainMemorytuneDefParse() of [RFCv2 PATCH 4/5] like below. + 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; + } Indeed. It's hard to keep space for feature extension without enough information. Introducing a parent section <resourcetune> for the single child <cachetune> without <memorytune> also confuses people. OK. I will pack vcpus logic into a helper function in next version. Great! So we all agree to make <memtune> a separated session. Beside more than 1 vCPU and 1 node, there is also the other possibility: put the host supporting threads (QEMU I/O thread and monitor thread) to the other group, So that memory bandwidth can be distributed among vCPUs and host supporting threads. I will put the previous review link in the cover letter for easy reference in future. hmmm..., just learn the <auth> part. That cross reference between disk/auth and source/auth does make code complex and not so straightforward. So better we keep cputune/cachetune and introduce cputune/memtune. Great~ :) Thanks for jumping in. I wait too long a time. :( Your comments are absolutely helpful. The current cachetunes data is stored in a pointer based fashion. The advantage of that is NULL pointer means no cachetuen information for particular level/type cache. And vCPU is stored in virDomainCachetuneDef of domain_conf. And overlapping testing happens in domain_conf part.The reason vCPU not part of virresctrl is properly because domain_conf need this information to formate domain's XML to store. As your suggested, I changed this virDomainCachetuneDef --> virDomainRestuneDef in this patch. Since it will hold information both cachetune and memtune. And yes, we can revisit virresctrl to extract common part of cachetune and memtune. Thanks for let me know this. I think we align together basing on above discuss. I will reply your comment in the PATH1 PATH2 and PATH4, and revise. Especially docs/schemas/*.rng, you pointed out. At the same time, we can see whether Pavel will give comment also. ;) Bing [.....]