On 5/28/19 10:32 AM, Huaqiang,Wang wrote:
On 2019年05月27日 23:26, Michal Privoznik wrote:
On 5/23/19 11:34 AM, Wang Huaqiang wrote:
Export virResctrlMonitorGetStats and make virResctrlMonitorGetCacheOccupancy obsoleted.
Signed-off-by: Wang Huaqiang <huaqiang.wang@intel.com> --- src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 33 +++++++++++++++++++++++++-------- src/util/virresctrl.c | 46 +++++++++++++++++++++++++++------------------- src/util/virresctrl.h | 6 ++++++ 4 files changed, 59 insertions(+), 27 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9099757..2e3d48c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2771,6 +2771,7 @@ virResctrlMonitorDeterminePath; virResctrlMonitorFreeStats; virResctrlMonitorGetCacheOccupancy; virResctrlMonitorGetID; +virResctrlMonitorGetStats; virResctrlMonitorNew; virResctrlMonitorRemove; virResctrlMonitorSetAlloc; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4ea346c..bc19171 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19987,6 +19987,7 @@ qemuDomainFreeResctrlMonData(virQEMUResctrlMonDataPtr resdata) /** * qemuDomainGetResctrlMonData: + * @driver: Pointer to qemu driver * @dom: Pointer for the domain that the resctrl monitors reside in * @resdata: Pointer of virQEMUResctrlMonDataPtr pointer for receiving the * virQEMUResctrlMonDataPtr array. Caller is responsible for @@ -20005,16 +20006,29 @@ qemuDomainFreeResctrlMonData(virQEMUResctrlMonDataPtr resdata) * Returns -1 on failure, or 0 on success. */ static int -qemuDomainGetResctrlMonData(virDomainObjPtr dom, +qemuDomainGetResctrlMonData(virQEMUDriverPtr driver, + virDomainObjPtr dom, virQEMUResctrlMonDataPtr **resdata, size_t *nresdata, virResctrlMonitorType tag) { virDomainResctrlDefPtr resctrl = NULL; virQEMUResctrlMonDataPtr res = NULL; + char **features = NULL; size_t i = 0; size_t j = 0; + if (tag == VIR_RESCTRL_MONITOR_TYPE_CACHE) { + features = driver->caps->host.cache.monitor->features;
No, we have virQEMUDriverGetCapabilities() which ensures that driver->caps object doesn't go away while accessing this.
I can't refer 'driver->caps->host.cache.monitor->features' directly because this function (qemuDomainGetResctrlMonData) will be used for 'memory bandwidth' monitors also. at that time that the piece of code looks like: + if (tag == VIR_RESCTRL_MONITOR_TYPE_CACHE) { + if (driver->caps->host.cache.monitor) + features = driver->caps->host.cache.monitor->features; + if (tag == VIR_RESCTRL_MONITOR_TYPE_MEMBW) { + if(driver->caps->host.membw) + features = driver->caps->host.membw.monitor->features;
What I meant is that you can obtain virCaps object and then access it instead: virCapsPtr caps = virQEMUDriverGetCapabilities(driver, false); if (tag == VIR_RES..) features = caps->host.cache.monitor->features; It's not safe to access driver->caps directly because another thread might refresh those meanwhile. What may happen for instance is the following: Thread1: fetches driver pointer adds ->caps displacement fetches value from that address (which is a long way of saying 'fetch driver->caps') Thread2: Executes virQEMUDriverGetCapabilities(), which boils down to: virObjectUnref(driver->caps); driver->caps = caps; Thread1: resumes execution and tries to access ->host member. This is where Thread1 would be accessing invalid memory because the memory it's trying to access was freed meanwhile by Thread2. Michal