Change in vdsm[master]: Live Merge: Restore watermark tracking

fromani at redhat.com fromani at redhat.com
Wed Feb 4 13:35:56 UTC 2015


Francesco Romani has posted comments on this change.

Change subject: Live Merge: Restore watermark tracking
......................................................................


Patch Set 4:

(4 comments)

sorry for going out of sync. Seen lot of action between versions 3 and 4, waiting to see how version 5 becomes before to add more comments.

http://gerrit.ovirt.org/#/c/36924/4/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 723:             'flushLatency': str(compute_latency('flush'))}
Line 724: 
Line 725: 
Line 726: def _getAllVMsBulkStats(conn, vmList, statsTypes=0, statsFlags=0):
Line 727:     domList = [x._dom._dom for x in vmList]
> "x" would be more clear as "vm".
+1
Line 728:     if statsTypes == 0:
Line 729:         statsTypes = (libvirt.VIR_DOMAIN_STATS_STATE |
Line 730:                       libvirt.VIR_DOMAIN_STATS_CPU_TOTAL |
Line 731:                       libvirt.VIR_DOMAIN_STATS_BALLOON |


Line 730:                       libvirt.VIR_DOMAIN_STATS_CPU_TOTAL |
Line 731:                       libvirt.VIR_DOMAIN_STATS_BALLOON |
Line 732:                       libvirt.VIR_DOMAIN_STATS_VCPU |
Line 733:                       libvirt.VIR_DOMAIN_STATS_INTERFACE |
Line 734:                       libvirt.VIR_DOMAIN_STATS_BLOCK)
Are you interested in this specific subset here and in the time being or you just want them all?
Because if you want all it is safe to just use statsTypes == 0:

Using 0 for @stats returns all stats groups supported by the given hypervisor.

both in
http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainListGetStats
and
http://libvirt.org/html/libvirt-libvirt-domain.html#virConnectGetAllDomainStats
Line 735:     statsList = conn.domainListGetStats(domList, statsTypes, statsFlags)
Line 736:     return dict([(x[0].UUIDString(), x[1]) for x in statsList])
Line 737: 
Line 738: 


Line 732:                       libvirt.VIR_DOMAIN_STATS_VCPU |
Line 733:                       libvirt.VIR_DOMAIN_STATS_INTERFACE |
Line 734:                       libvirt.VIR_DOMAIN_STATS_BLOCK)
Line 735:     statsList = conn.domainListGetStats(domList, statsTypes, statsFlags)
Line 736:     return dict([(x[0].UUIDString(), x[1]) for x in statsList])
> The x variable is not very helpful, specially when you don't know what are 
+1 for the genexp
Line 737: 
Line 738: 
Line 739: class TimeoutError(libvirt.libvirtError):
Line 740:     pass


Line 1436:         volAllocMap = {}
Line 1437:         bulkStats = self._getBulkStats(libvirt.VIR_DOMAIN_STATS_BLOCK,
Line 1438:                                        _LIBVIRT_BACKING_CHAIN_STATS_FLAG)
Line 1439:         for i in xrange(bulkStats['block.count']):
Line 1440:             name = bulkStats['block.%i.name' % i]
> What is this expected key is missing?
Yes, this is possible, especially when dealing with backing chains (and not with simple block devices).
Line 1441:             try:
Line 1442:                 drive = self._findDriveByName(name)
Line 1443:             except LookupError:
Line 1444:                 self.log.error("Unable to find drive '%s'", name)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I632f31e7795ec5d8c6f52a480116b14470c3163f
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani 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