Francesco Romani has posted comments on this change.

Change subject: virt: faster getAllVmStats
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.ovirt.org/#/c/35140/2/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 1881:         Please note that some values are provided by client (engine)
Line 1882:         but can change as result as interaction with libvirt 
(display*)
Line 1883:         """
Line 1884:         stats = {
Line 1885:             'vmId': self.conf['vmId'],
> Is this becasue we were adding the id externally in API.getAllVMStats?
Sorry, I somehow missed the followup-to this comment (just added a too terse 
reply).

Yes, you got the reason. If we look at 

api.VM.getStats(), we see the function adds the vmId field:

      
    def _getStats(self, runHooks=True):
        v = self._cif.vmContainer.get(self._UUID)
        if not v:
            return errCode['noVM']

        if runHooks:
            hooks.before_get_vm_stats()
        stats = v.getStats().copy()
        stats['vmId'] = self._UUID
        if runHooks:
            stats = hooks.after_get_vm_stats([stats])[0]
        return {'status': doneCode, 'statsList': [stats]}

Now I need to move this addition in vdsm.virt.Vm.getStats().
This change should be safe, I can't think of any case on which this could be 
harmful.
Line 1886:             'pid': self.conf['pid'],
Line 1887:             'vmType': self.conf['vmType'],
Line 1888:             'kvmEnable': self._kvmEnable,
Line 1889:             'acpiEnable': self.conf.get('acpiEnable', 'true')}


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If1ed3f49c236086c6184973762d9909d7f79bf7e
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to