Change in vdsm[master]: Collect current QoS settings for IO devices and report throu...

michal.skrivanek at redhat.com michal.skrivanek at redhat.com
Fri Jun 27 11:06:11 UTC 2014


Michal Skrivanek has posted comments on this change.

Change subject: Collect current QoS settings for IO devices and report through RunningVmStats
......................................................................


Patch Set 6:

(4 comments)

not sure the design is actually correct if these values are not really dynamic. Most harm has already been done with previous patches though, so I can live with some proposal and plan how to change it...

http://gerrit.ovirt.org/#/c/28712/6/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 331:             infos['vcpuLimit'] = nodeList[0].childNodes[0].data
Line 332: 
Line 333:         return infos
Line 334: 
Line 335:     def _sampleIoTune(self):
> MOM and dynamic updates. When you set the blkio values, libvirt proceses th
If you set something and you just want to check once that it was applied(or applied partially or adjusted) you should only read it once. This doesn't belong to stats thread.
If it can change over the life of VM there should be libvirt event for that (file a bug and add TODO)
If it can then change only on actions we do (migration, live storage mig, etc) the vdsm code should retrieve the data only on that specific actions

And yes, cpu tune is wrong as well.
Line 336:         info = []
Line 337: 
Line 338:         try:
Line 339:             lastXMLDesc = self._vm._dom.XMLDesc(0)


Line 338:         try:
Line 339:             lastXMLDesc = self._vm._dom.XMLDesc(0)
Line 340:         except libvirt.libvirtError:
Line 341:             self._log.exception("Error while collecting ioTune for %s",
Line 342:                                 self._vm.name)
> You did not answer my question about the usefulness of the traceback. Does 
+1
as errors in stats thread do occur quite often around the lifecycle events…they always alarm people and we should keep them at minimum
Line 343:             return None
Line 344: 
Line 345:         devxml = _domParseStr(lastXMLDesc).childNodes[0] \
Line 346:             .getElementsByTagName('devices')[0]


Line 346:             .getElementsByTagName('devices')[0]
Line 347: 
Line 348:         diskxml = devxml.getElementsByTagName('disk')
Line 349: 
Line 350:         for disk in diskxml:
> No way to match the xml record with the proper _devices entry without parsi
isn't it what alias is good for and used elsewhere?
Line 351:             alias, path, name = self._vm._getDriveIdentification(disk)
Line 352:             io_tune_dom = disk.getElementsByTagName('iotune')
Line 353:             io_tune_values = {}
Line 354: 


Line 607:         self._getDiskStats(stats)
Line 608:         self._getDiskLatency(stats)
Line 609:         self._getBalloonStats(stats)
Line 610:         self._getVmJobs(stats)
Line 611:         self._getIoTuneStats(stats)
> And btw are you two aware of the fact that this call only merges two dictio
yes, it will make me happy to add new things at the end and over the course of versions be able to quickly see what is there and what not.

Of course the NUMA ugliness below should be fixed as well (separately) :)
Line 612: 
Line 613:         vmNumaNodeRuntimeMap = numaUtils.getVmNumaNodeRuntimeInfo(self._vm)
Line 614:         if vmNumaNodeRuntimeMap:
Line 615:             stats['vNodeRuntimeInfo'] = vmNumaNodeRuntimeMap


-- 
To view, visit http://gerrit.ovirt.org/28712
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie4fab132b60ce6ae2285115034f3d1ffcdca9e9d
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Sivák <msivak at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Martin Sivák <msivak at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list