Change in vdsm[master]: getStorageDomainInfo: SP keys when disconnected.
abaron at redhat.com
abaron at redhat.com
Tue Nov 26 21:46:46 UTC 2013
Ayal Baron has posted comments on this change.
Change subject: getStorageDomainInfo: SP keys when disconnected.
......................................................................
Patch Set 2: Code-Review-1
(6 comments)
1. No clear motivation for this patch.
2. It is breaking encapsulation
....................................................
Commit Message
Line 12: directly the pool MD for any MSD.
Line 13: This changes the previous behaviour.
Line 14:
Line 15: TODO: pool related information should not be returned as part of
Line 16: this API response.
+1
Line 17:
Line 18: Change-Id: I8b7323d4ccaaaec0f39d5590a06879faa7fc999e
....................................................
File vdsm/storage/hsm.py
Line 2735: # This, of-course breaks backward compatibility.
Line 2736: # These keys are not likely to change (also because of
Line 2737: # BC) so it's not that horrible. In any case please
Line 2738: # remove this when we can stop supporting this API.
Line 2739: if info['role'] != sd.MASTER_DOMAIN:
remove this if
Line 2740: info.update({'lver': -1, 'spm_id': -1, 'master_ver': 0})
Line 2741: else:
Line 2742: pmd = sp.StoragePool._getPoolMD(dom)
Line 2743: info['lver'] = pmd[sp.PMDK_LVER]
Line 2737: # BC) so it's not that horrible. In any case please
Line 2738: # remove this when we can stop supporting this API.
Line 2739: if info['role'] != sd.MASTER_DOMAIN:
Line 2740: info.update({'lver': -1, 'spm_id': -1, 'master_ver': 0})
Line 2741: else:
change else to: if info['role'] == sd.MASTER_DOMAIN:
Line 2742: pmd = sp.StoragePool._getPoolMD(dom)
Line 2743: info['lver'] = pmd[sp.PMDK_LVER]
Line 2744: info['master_ver'] = pmd[sp.PMDK_MASTER_VER]
Line 2745: info['spm_id'] = -1
Line 2738: # remove this when we can stop supporting this API.
Line 2739: if info['role'] != sd.MASTER_DOMAIN:
Line 2740: info.update({'lver': -1, 'spm_id': -1, 'master_ver': 0})
Line 2741: else:
Line 2742: pmd = sp.StoragePool._getPoolMD(dom)
instead of accessing internals, you should be adding a method that returns the info you need or something
Line 2743: info['lver'] = pmd[sp.PMDK_LVER]
Line 2744: info['master_ver'] = pmd[sp.PMDK_MASTER_VER]
Line 2745: info['spm_id'] = -1
Line 2746: try:
Line 2740: info.update({'lver': -1, 'spm_id': -1, 'master_ver': 0})
Line 2741: else:
Line 2742: pmd = sp.StoragePool._getPoolMD(dom)
Line 2743: info['lver'] = pmd[sp.PMDK_LVER]
Line 2744: info['master_ver'] = pmd[sp.PMDK_MASTER_VER]
I see no reason to return this info if getPool fails?
i.e. move both lines down to join the spm_id line
Line 2745: info['spm_id'] = -1
Line 2746: try:
Line 2747: # Verify that the host is connected to the same pool which
Line 2748: # the SD is attached to.
Line 2741: else:
Line 2742: pmd = sp.StoragePool._getPoolMD(dom)
Line 2743: info['lver'] = pmd[sp.PMDK_LVER]
Line 2744: info['master_ver'] = pmd[sp.PMDK_MASTER_VER]
Line 2745: info['spm_id'] = -1
remove this line (spm_id = -1) as with above changes it is redundant
Line 2746: try:
Line 2747: # Verify that the host is connected to the same pool which
Line 2748: # the SD is attached to.
Line 2749: pool = self.getPool(info['pool'][0])
--
To view, visit http://gerrit.ovirt.org/19555
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b7323d4ccaaaec0f39d5590a06879faa7fc999e
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Elad Ben Aharon <eladba1990 at gmail.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Gadi Ickowicz <gickowic at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list