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

alitke at redhat.com alitke at redhat.com
Wed Jan 21 21:37:25 UTC 2015


Adam Litke has posted comments on this change.

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


Patch Set 1:

(5 comments)

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

Line 1520:             raise LookupError("Unable to find VolumeID for path '%s'", path)
Line 1521: 
Line 1522:         volAllocMap = {}
Line 1523:         statsFlags = self._libvirtBackingChainStatsFlag()
Line 1524:         conn = libvirtconnection.get()
> unless you need a specific different connection (then please explain why), 
Done
Line 1525:         blkStats = conn.domainListGetStats([self._dom._dom],
Line 1526:                                            libvirt.VIR_DOMAIN_STATS_BLOCK,
Line 1527:                                            statsFlags)[0][1]
Line 1528:         for i in xrange(0, blkStats['block.count']):


Line 1523:         statsFlags = self._libvirtBackingChainStatsFlag()
Line 1524:         conn = libvirtconnection.get()
Line 1525:         blkStats = conn.domainListGetStats([self._dom._dom],
Line 1526:                                            libvirt.VIR_DOMAIN_STATS_BLOCK,
Line 1527:                                            statsFlags)[0][1]
> I prefer to avoid the double indexing.
Done
Line 1528:         for i in xrange(0, blkStats['block.count']):
Line 1529:             name = blkStats['block.%i.name' % i]
Line 1530:             try:
Line 1531:                 drive = self._findDriveByName(name)


Line 1523:         statsFlags = self._libvirtBackingChainStatsFlag()
Line 1524:         conn = libvirtconnection.get()
Line 1525:         blkStats = conn.domainListGetStats([self._dom._dom],
Line 1526:                                            libvirt.VIR_DOMAIN_STATS_BLOCK,
Line 1527:                                            statsFlags)[0][1]
> about the bulk stats, we need a common shared API/pattern to use them. We'r
I've headed down this direction in the next series.  I hope you like the changes.
Line 1528:         for i in xrange(0, blkStats['block.count']):
Line 1529:             name = blkStats['block.%i.name' % i]
Line 1530:             try:
Line 1531:                 drive = self._findDriveByName(name)


Line 1524:         conn = libvirtconnection.get()
Line 1525:         blkStats = conn.domainListGetStats([self._dom._dom],
Line 1526:                                            libvirt.VIR_DOMAIN_STATS_BLOCK,
Line 1527:                                            statsFlags)[0][1]
Line 1528:         for i in xrange(0, blkStats['block.count']):
> why the explicit start? Isn't xrange(blkStats['block.count']) enough?
Just a personal preference but am happy to change it if it's important enough to mention here.
Line 1529:             name = blkStats['block.%i.name' % i]
Line 1530:             try:
Line 1531:                 drive = self._findDriveByName(name)
Line 1532:             except LookupError:


Line 4858:         # block statistics for all volumes in the chain when using a new flag.
Line 4859:         try:
Line 4860:             return libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING
Line 4861:         except AttributeError:
Line 4862:             return 0
> This could be detected once per VDSM run, no need to check it for every ope
Done
Line 4863: 
Line 4864:     def merge(self, driveSpec, baseVolUUID, topVolUUID, bandwidth, jobUUID):
Line 4865:         if not caps.getLiveMergeSupport():
Line 4866:             self.log.error("Live merge is not supported on this host")


-- 
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: 1
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