Description of problem: In Bug #1529271 we use balloon drive info to report memory stats as metrics. this information returns also from guest agent if installed. currently we report guest agent info only in getAllVmStats api call. This RFE requests to add guest agent info to sampling virt stats structure which currently we don't fetch while building a sample, we use only libvirt bulk stats.
Taking to investigate feasibility of the change on the Vdsm side.
no doc text needed, users should never see this.
with this fix we get the same fields getAllVmStats returns - now I can send memoryStats always when guess agent is installed, and also I have memUsed value that exists always
since 4.2 memory is reported as balloon stats. I do not see a reason to address guest agent data for <4.2 Is there any other value from guest agent which is relevant?
We have those 2 options to report vms memory values: 1. avoid reporting memory stats for running vms (after vm reboot we will have it - this will cover the issue I describe in Bug #1532681) 2. have GA info using the patch Francesco posted (requires scale test) with option 1 we have mem_buffers and mem_cached fields that are not there and GA reports.
(In reply to Yaniv Bronhaim from comment #5) > We have those 2 options to report vms memory values: > 1. avoid reporting memory stats for running vms (after vm reboot we will > have it - this will cover the issue I describe in Bug #1532681) yes VMs in Cluster Level 4.2 onwards will always have it. > 2. have GA info using the patch Francesco posted (requires scale test) I do not see us investing into that > with option 1 we have mem_buffers and mem_cached fields that are not there > and GA reports. Great, (1) it is then? I do not see much incentive to provide anything for <4.2 since it requires risky code change, I'd rather abandon those preliminary patches. Best way going forward would be to go with collectd-native data retrieval, it should be in a decent shape
(In reply to Michal Skrivanek from comment #6) > (In reply to Yaniv Bronhaim from comment #5) > > We have those 2 options to report vms memory values: > > 1. avoid reporting memory stats for running vms (after vm reboot we will > > have it - this will cover the issue I describe in Bug #1532681) > > yes VMs in Cluster Level 4.2 onwards will always have it. Having to restart the vms to get the memory stats is not acceptable in my opinion. Yaniv do you agree? > > > 2. have GA info using the patch Francesco posted (requires scale test) > > I do not see us investing into that > > > with option 1 we have mem_buffers and mem_cached fields that are not there > > and GA reports. > > Great, (1) it is then? I do not see much incentive to provide anything for > <4.2 since it requires risky code change, I'd rather abandon those > preliminary patches. > Best way going forward would be to go with collectd-native data retrieval, > it should be in a decent shape We do not use the collectd virt plugin at this time. The ne virt plugin is only available in collectd 5.8.0 and it is not yet built d/s for 4.2.
For me it sounds that if only new created vms (or rebooted vms) since 4.2 upgrade will report memory statistics is good enough, based on that that we never reported those values yet. If we still don't use collectd virt plugin, and the correct approach is much less risky they the new proposed one, I also wouldn't take the risk and start scale test for that. I'd verify https://gerrit.ovirt.org/#/c/86309/ with 4.2 new vms to see that the keys and values are reported as expected by the metrics store and leave older vms without this info.
Re-adding the needinfo.
(In reply to Yaniv Bronhaim from comment #8) > For me it sounds that if only new created vms (or rebooted vms) since 4.2 > upgrade will report memory statistics is good enough, based on that that we > never reported those values yet. Understood and agreed. > If we still don't use collectd virt plugin, and the correct approach is much > less risky they the new proposed one, I also wouldn't take the risk and > start scale test for that. We are not yet using it. > I'd verify https://gerrit.ovirt.org/#/c/86309/ with 4.2 new vms to see that > the keys and values are reported as expected by the metrics store and leave > older vms without this info.
Shirly, IIRC you switched to the new collectd method already, correct? Is this code still needed then (values gathered by vdsm)?
Yes. We are now using the collectd virt plugin.
What's the next step here?
I suggest to revert the code to its original state to stop copying data not used by anyone
Can you share verification steps, please?
In order to verify this patch: 1. verify that patched Vdsm is still reporting the metrics *to Engine* correctly, e.g. checking the output of Host.getAllVmStats Now, this is sufficient verification of this change per se. We can maybe add this stretch task: 2. verify that the metrics store is fetching the metrics correctly But this is actually something more related to the metrics store itself. I guess #1 above is enough.
I tried running this command on a host that has vdsm-client and has one running VM: vdsm-client Host getAllVmStats And it returned correct data about the VM to output. Tried it again, but this time with two running VMs. Returns data about both VMs to output. Tried again with no VMs running and in output was no data about VMs. This is the expected behavior. Verified in: vdsm-4.30.3-1.el7ev.x86_64 ovirt-engine-4.3.0-0.5.alpha1.el7.noarch
This bugzilla is included in oVirt 4.3.0 release, published on February 4th 2019. Since the problem described in this bug report should be resolved in oVirt 4.3.0 release, it has been closed with a resolution of CURRENT RELEASE. If the solution does not work for you, please open a new bug report.