Change in vdsm[master]: vm: implement VM.sdIds as read-only property

nsoffer at redhat.com nsoffer at redhat.com
Sun Sep 14 18:42:22 UTC 2014


Nir Soffer has posted comments on this change.

Change subject: vm: implement VM.sdIds as read-only property
......................................................................


Patch Set 5:

(2 comments)

A deeper cleanup is required in vm.py. Otherwise this is fine.

Thanks Fabio!

http://gerrit.ovirt.org/#/c/32796/5/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 1918:                     # A destroy request has been issued, exit early
Line 1919:                     break
Line 1920:                 drive['path'] = self.cif.prepareVolumePath(drive, self.id)
Line 1921:             if drive['device'] == 'disk' and isVdsmImage(drive):
Line 1922:                 domains.append(drive['domainID'])
You left behind a rather useless list of domains. Now that we don't append this list to the self.sdIds, we don't need to maintain the "domains" list.
Line 1923:         else:
Line 1924:             # Now we got all the resources we needed
Line 1925:             self.startDisksStatsCollection()
Line 1926: 


Line 3538:         diskParams = params.get('drive', {})
Line 3539:         diskParams['path'] = self.cif.prepareVolumePath(diskParams)
Line 3540:         vdsmImg = isVdsmImage(diskParams)
Line 3541: 
Line 3542:         if vdsmImg:
After you removed the other use of "vdsmImg" (was in line 3552), we are left with one usage, which makes this temporary pointless, and less readable (vdsmImg vs isVdsmImage(diskParams)). Please remove unneeded temporary.
Line 3543:             self._normalizeVdsmImg(diskParams)
Line 3544:             self._createTransientDisk(diskParams)
Line 3545: 
Line 3546:         self.updateDriveIndex(diskParams)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie445d4689aa562b00229844007ffa05eaecebdcb
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Fabio Pliger <fabio.pliger at gmail.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Fabio Pliger <fabio.pliger at gmail.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: Yeela Kaplan <ykaplan 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