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

Re: [libvirt] [PATCHv2 00/11] util/resctrl cleanups and refactors



On 7/31/19 8:37 AM, Michal Privoznik wrote:
> On 6/11/19 5:31 AM, Wang Huaqiang wrote:
>> Patches submitted for purpose of refactoring existing 'resctrl' related
>> source code, including some code cleanups as well as some fixes. This is
>> also a preparation for memory bandwidth monitor codes.
>>
>> Plan to support Resctrl Control Monitors, which is a feature introduced
>> by kernel 'resctrl' sub-model. Submit some cleanup and refactoring
>> patches
>> for upcoming memory bandwidth resource monitoring (MBM) monitors.
>>
>> Related MBM RFC is
>> https://www.redhat.com/archives/libvir-list/2019-April/msg01409.html.
>> This RFC is not actively discussed since libvirt already implemented
>> similar
>> resctrl cache monitoring (CMT), and lots details have been discussed
>> and implemented during the work of CMT.
>>
>> The cleanups and refactoring includes:
>> v2 changes:
>> 1. Addressed comments of v1.
>> 2. Introduce a new algorithm for verifying new monitor vcpus and existing
>> monitors and allocations.
>> 3. Fixes for creating default-allocation-monitor in 'resctrl' file
>> system.(patch 0001).
>>
>> v1 changes:
>> 1. Removing some reluctant lines and white spaces that is existing
>> in current code and not meet the libvirt coding style.
>> 2. Replace 'virResctrlAllocIsEmpty' with @n==0 for indicating no
>> resctrl allocation in configuration file.
>> 3. Private API changes, removed 'virResctrlMonitorGetCacheOccupancy'
>> and exported a new API named 'virResctrlMonitorGetStats' with similar
>> functionality, but with capability to be used for retrieving MBM
>> statistical information.
>> 4. Refactoring 'virResctrlMonitorFreeStats' for more reusing in code.
>> 5. Extend data structure 'virResctrlMonitorStats' with the capability
>> to carry multiple statistical information from monitor.
>>
>> Wang Huaqiang (11):
>>    util,conf: Handle default monitor group of an allocation properly
>>    conf: code cleanup, remove empty line and one space
>>    conf: code cleanup for return error code directly
>>    conf: some code cleanup
>>    conf: refactor 'virDomainResctrlVcpuMatch' and some code cleanup

I think this patch introduced an issue. resctrl returned from VcpuMatch
should not be free'd, but that can happen in virDomainMemorytuneDefParse

Use the attached XML and do:

./tools/virsh --connect test:///default define bad.xml

>>    conf: Append 'resctrl' object according to number of monitor group
>>      directly
>>    util: Refactor and rename 'virResctrlMonitorFreeStats'
>>    util: Refactor 'virResctrlMonitorStats'
>>    util: Extend virresctl API to retrieve multiple monitor statistics
>>    util: Remove unused virResctrlMonitorGetCacheOccupancy
>>    conf: Refactor and rename the function to validate a new resctrl
>>      monitor
>>
>>   src/conf/domain_conf.c   | 145
>> ++++++++++++++++++++++++-----------------------
>>   src/libvirt_private.syms |   5 +-
>>   src/qemu/qemu_driver.c   |  41 ++++++++++----
>>   src/qemu/qemu_process.c  |   3 +-
>>   src/util/virresctrl.c    |  75 ++++++++++--------------
>>   src/util/virresctrl.h    |  23 +++++---
>>   6 files changed, 156 insertions(+), 136 deletions(-)
>>
> 
> Reviewed-by: Michal Privoznik <mprivozn redhat com>
> 
> I'll push these once the freeze is over.
> 




- Cole
<domain type="test">
  <name>fedora</name>
  <vcpu>9</vcpu>
  <os>
    <type>hvm</type>
  </os>
  <cputune>
    <vcpupin vcpu="0" cpuset="0-3"/>
    <cachetune vcpus="0-3">
      <cache level="3" id="0" type="both" size="3" unit="MiB"/>
    </cachetune>
    <memorytune vcpus="0-3">
      <node id="0" bandwidth="60"/>
    </memorytune>
  </cputune>
</domain>

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