Re: [PATCH 7/9] conf: Refactor virDomainCachetuneDefParse

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2018年07月27日 01:48, John Ferlan wrote:


On 07/18/2018 03:57 AM, bing.niu@xxxxxxxxx wrote:
From: Bing Niu <bing.niu@xxxxxxxxx>

Refactoring virDomainCachetuneDefParse, extracting vcpus extracting,
overlapping detecting and new resctrl allocation creating logic.
Those two logic is common among different resource allocation
technologies.

Signed-off-by: Bing Niu <bing.niu@xxxxxxxxx>
---
  src/conf/domain_conf.c | 195 +++++++++++++++++++++++++++++++++----------------
  1 file changed, 131 insertions(+), 64 deletions(-)


You could make 3 patches out of this - one for each reduction of
virDomainCachetuneDefParse...

Will split that.

The virDomainFindResctrlAlloc should have used Restune instead of
ResctrlAlloc, right? Of course that changes based on how the naming
patch shakes out. Actually I think perhaps virDomainRestuneVcpuMatch
would be even better (where Restune ends up being whatever shakes out of
the previous patch naming decision).

Let's use virDomainResctrlVcpuMatch. :)

In this area, I'm wondering what purpose does @new_alloc serve?  If
@alloc was already defined, then you added an error message. So the
reality is - @new_alloc is always true.

As you already note in patch 8. This is used by memory bandwidth to find a resctrl group defined by CAT already.

Secondary to that you've added a new error related to identical vcpus in
the parsing logic. Unfortunately that could make a domain disappear on a
libvirtd restart if for some reason it was already defined in that
manner. If there's a parsing error because two entries had identical
cpus, then there will be an error. Errors would cause a previously
OK/found domain to not be defined. So that type of check (now that the

Sorry, I am not sure if I understand correctly. Are you talking about two domains(or VMs) with the same vcpus setting? If so, above vpu match is not for different domain but for the memory bandwidth allocation and the cache line allocation of one domain.

And above vcpu match function doesn't change any logic of existing virDomainCachetuneDefParseCache. It just extract from virDomainCachetuneDefParseCache and packing into one function. So that memory bandwidth parsing logic in patch 8 can reuse this vcpu matching part. :)
code has been around for more than a release) would need to go in some
sort of Validation API. This is one of those libvirt define and libvirtd
restart quirks.

If my understanding is incorrect. Could you please clarify here or give me some example? :)

Finally, virDomainUpdateRestune should be virDomainRestuneUpdate since
you started with virDomainRestuneParseVcpus and it should go after
virDomainCachetuneDefParseCache


Let's use virDomainResctrlUpdate
Hope this all makes sense!

John

[....]

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux