Re: [libvirt] [PATCH 7/9] conf: Refactor virDomainCachetuneDefParse
On 2018年07月27日 01:48, John Ferlan wrote:
On 07/18/2018 03:57 AM, bing.niu(a)intel.com wrote:
> From: Bing Niu <bing.niu(a)intel.com>
>
> 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(a)intel.com>
> ---
> 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
[....]