1:09 a.m.
Eduardo has uploaded a new change for review.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Fix getStorageDomainInfo() logic.
Making repoStats pool independent.
Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Signed-off-by: Eduardo <ewarszaw(a)redhat.com>
---
M vdsm/storage/hsm.py
1 file changed, 18 insertions(+), 9 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/71/14671/1
diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index c90ea07..9a44795 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -2664,17 +2664,26 @@
# BC) so it's not that horrible. In any case please
# remove this when we can stop supporting this API.
info.update({'lver': -1, 'spm_id': -1, 'master_ver': 0})
- if dom.getDomainRole() == sd.MASTER_DOMAIN:
- # make sure it's THE master
+ if info['role'] == sd.MASTER_DOMAIN:
try:
- pool = self.getPool(dom.getPools()[0])
- if pool.masterDomain.sdUUID == sdUUID:
- poolInfo = pool.getInfo()
- for key in ['lver', 'spm_id', 'master_ver']:
- info[key] = poolInfo['info'][key]
+ pool = self.getPool(info['pool'][0])
+ except IndexError:
+ self.log.error("Domain %s is marked as master but is not "
+ "attached to any pool", sdUUID, exc_info=True)
except se.StoragePoolUnknown:
- # Its pool is not connected
- pass
+ self.log.error("Domain %s, marked as master, is attached to "
+ "pool %s but this host is connected to pool
%s",
+ sdUUID, pool.spUUID)
+ else:
+ # make sure it's THE master of this pool
+ if pool.masterDomain.sdUUID != sdUUID:
+ self.log.error("Domain %s is marked as master but actual "
+ "master is %s",
+ sdUUID, pool.masterDomain.sdUUID)
+ else:
+ poolInfo = pool.getInfo()
+ for key in ('lver', 'spm_id', 'master_ver'):
+ info[key] = poolInfo['info'][key]
return dict(info=info)
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
10:27 p.m.
Eduardo has posted comments on this change.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Patch Set 6: Verified+1
Verified with Change-Id: I983d49b0a42cc06428ec75b7795d23abaa6ab84c
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Daniel Paikov <paikov(a)gmail.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
10:10 p.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Patch Set 6: Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3847/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4732/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4656/ : SUCCESS
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Daniel Paikov <paikov(a)gmail.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
8:28 a.m.
Ayal Baron has posted comments on this change.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Daniel Paikov <paikov(a)gmail.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
8:10 a.m.
Ayal Baron has posted comments on this change.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Daniel Paikov <paikov(a)gmail.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
7:29 a.m.
Saggi Mizrahi has posted comments on this change.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Patch Set 5: Code-Review+1
I give +1 because Edu has assured me he will push the correct fix later.
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Daniel Paikov <paikov(a)gmail.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
1:52 a.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Patch Set 5:
Build Successful
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4552/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3655/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4474/ : SUCCESS
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Daniel Paikov <paikov(a)gmail.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
1:44 a.m.
Eduardo has posted comments on this change.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Daniel Paikov <paikov(a)gmail.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-HasComments: No
5:55 a.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Patch Set 4: Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4388/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3491/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4307/ : SUCCESS
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Daniel Paikov <paikov(a)gmail.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
2:08 a.m.
Oved Ourfali has posted comments on this change.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Daniel Paikov <paikov(a)gmail.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
4:05 a.m.
Ayal Baron has posted comments on this change.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Patch Set 3:
(3 comments)
....................................................
File vdsm/storage/hsm.py
Line 2740: if info['role'] == sd.MASTER_DOMAIN:
Line 2741: try:
Line 2742: # Verify that the host is connected to the same pool which
Line 2743: # the SD is attached to.
Line 2744: pool = self.getPool(info['pool'][0])
write the code and you'll see it makes it a lot uglier and does not make it more
comprehensible (i.e. comment would still be needed)
Line 2745: except IndexError:
Line 2746: self.log.error("Domain %s is marked as master but is not
"
Line 2747: "attached to any pool", sdUUID,
exc_info=True)
Line 2748: except se.StoragePoolUnknown:
Line 2743: # the SD is attached to.
Line 2744: pool = self.getPool(info['pool'][0])
Line 2745: except IndexError:
Line 2746: self.log.error("Domain %s is marked as master but is not
"
Line 2747: "attached to any pool", sdUUID,
exc_info=True)
exc_info is not adding any relevant data here and is just trashing the log
Line 2748: except se.StoragePoolUnknown:
Line 2749: self.log.error("Domain %s, marked as master, is attached
to "
Line 2750: "pool %s but this host is connected to pool
%s",
Line 2751: sdUUID, info['pool'],
self.pools[0].spUUID,
Line 2752: exc_info=True)
Line 2753: else:
Line 2754: # make sure it's THE master of this pool
Line 2755: if pool.masterDomain.sdUUID != sdUUID:
Line 2756: self.log.error("Domain %s is marked as master but
actual "
fake data? seriously?
Line 2757: "master is %s",
Line 2758: sdUUID, pool.masterDomain.sdUUID)
Line 2759: else:
Line 2760: poolInfo = pool.getInfo()
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Daniel Paikov <paikov(a)gmail.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
9:51 a.m.
Saggi Mizrahi has posted comments on this change.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Patch Set 3: Code-Review-1
(3 comments)
There are no tests for any of the corner cases being handled.
Please add them before committing this code.
....................................................
File vdsm/storage/hsm.py
Line 2740: if info['role'] == sd.MASTER_DOMAIN:
Line 2741: try:
Line 2742: # Verify that the host is connected to the same pool which
Line 2743: # the SD is attached to.
Line 2744: pool = self.getPool(info['pool'][0])
Making sure that len(info['pool'] >= 1) and looking for info['pool'][0]
in the pool list would have made the comment unnecessary. It would have made the code
exception free.
You are actually using the side effects to check for things instead of writing the checks
out in code.
Line 2745: except IndexError:
Line 2746: self.log.error("Domain %s is marked as master but is not
"
Line 2747: "attached to any pool", sdUUID,
exc_info=True)
Line 2748: except se.StoragePoolUnknown:
Line 2752: exc_info=True)
Line 2753: else:
Line 2754: # make sure it's THE master of this pool
Line 2755: if pool.masterDomain.sdUUID != sdUUID:
Line 2756: self.log.error("Domain %s is marked as master but
actual "
IIRC it's also being checked *and fixed* elswhere
Also, if we are already checking. Isn't it better to fake it and tell return that the
domain is a data domain so that we don't confuse the user?
Line 2757: "master is %s",
Line 2758: sdUUID, pool.masterDomain.sdUUID)
Line 2759: else:
Line 2760: poolInfo = pool.getInfo()
Line 2755: if pool.masterDomain.sdUUID != sdUUID:
Line 2756: self.log.error("Domain %s is marked as master but
actual "
Line 2757: "master is %s",
Line 2758: sdUUID, pool.masterDomain.sdUUID)
Line 2759: else:
I would put the positive (and shorter) case as the first clause of the conditional and the
longer one at the end
Line 2760: poolInfo = pool.getInfo()
Line 2761: for key in ('lver', 'spm_id',
'master_ver'):
Line 2762: info[key] = poolInfo['info'][key]
Line 2763:
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Daniel Paikov <paikov(a)gmail.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
2:02 a.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Patch Set 3:
Build Successful
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4272/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3375/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4191/ : SUCCESS
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Daniel Paikov <paikov(a)gmail.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
10:03 a.m.
Ayal Baron has posted comments on this change.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Patch Set 2: I would prefer that you didn't submit this
(3 inline comments)
....................................................
Commit Message
Line 3: AuthorDate: 2013-05-12 04:58:11 +0300
Line 4: Commit: Eduardo Warszawski <ewarszaw(a)redhat.com>
Line 5: CommitDate: 2013-05-16 10:46:09 +0300
Line 6:
Line 7: Fix getStorageDomainInfo() logic.
Fix how? what are you doing? why?
Line 8:
Line 9: Making repoStats pool independent.
Line 10:
Line 11: Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Line 5: CommitDate: 2013-05-16 10:46:09 +0300
Line 6:
Line 7: Fix getStorageDomainInfo() logic.
Line 8:
Line 9: Making repoStats pool independent.
This patch is not making anything pool independent. Please explain the motivation for it
properly.
Line 10:
Line 11: Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
....................................................
File vdsm/storage/hsm.py
Line 2665: # remove this when we can stop supporting this API.
Line 2666: info.update({'lver': -1, 'spm_id': -1,
'master_ver': 0})
Line 2667: if info['role'] == sd.MASTER_DOMAIN:
Line 2668: try:
Line 2669: pool = self.getPool(info['pool'][0])
To have a clue of what's going on here you need to split the code like so:
try:
poolUUID = info['pool'][0]
except IndexError:
self.log.error("Domain %s is marked as master but is not "
...
try:
pool = self.getPool(poolUUID)
except se.StoragePoolUnknown:
...
Line 2670: except IndexError:
Line 2671: self.log.error("Domain %s is marked as master but is not
"
Line 2672: "attached to any pool", sdUUID,
exc_info=True)
Line 2673: except se.StoragePoolUnknown:
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Daniel Paikov <paikov(a)gmail.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
7:45 a.m.
Ayal Baron has posted comments on this change.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Patch Set 3: Code-Review-1
(3 comments)
....................................................
Commit Message
Line 6:
Line 7: Fix getStorageDomainInfo() logic.
Line 8:
Line 9: Domains with 'role' == MASTER_DOMAIN should be attached to a
Line 10: pool at any time.
s/at any time/all the time/ ?
Line 11: Detect 'stale' master domains.
Line 12: Not swallowing errors anymore.
Line 13: Catching specific errors.
Line 14:
Line 8:
Line 9: Domains with 'role' == MASTER_DOMAIN should be attached to a
Line 10: pool at any time.
Line 11: Detect 'stale' master domains.
Line 12: Not swallowing errors anymore.
s/swallowing/hiding/
Line 13: Catching specific errors.
Line 14:
Line 15: Required for making repoStats pool independent.
Line 16:
....................................................
File vdsm/storage/hsm.py
Line 2752: exc_info=True)
Line 2753: else:
Line 2754: # make sure it's THE master of this pool
Line 2755: if pool.masterDomain.sdUUID != sdUUID:
Line 2756: self.log.error("Domain %s is marked as master but
actual "
this is not an error, *at most* it's a warning.
Line 2757: "master is %s",
Line 2758: sdUUID, pool.masterDomain.sdUUID)
Line 2759: else:
Line 2760: poolInfo = pool.getInfo()
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Daniel Paikov <paikov(a)gmail.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
4:30 p.m.
Dan Kenigsberg has submitted this change and it was merged.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Fix getStorageDomainInfo() logic.
Domains with 'role' == MASTER_DOMAIN should always be attached
to a pool.
Detect 'stale' master domains.
Not hiding errors anymore.
Catching specific errors.
BC: If the domain is a Master but it is not connected to the actual
(host) pool, master_ver, host_id and lver are not returned.
Required for making repoStats pool independent.
Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Signed-off-by: Eduardo <ewarszaw(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/14671
Reviewed-by: Ayal Baron <abaron(a)redhat.com>
---
M vdsm/storage/hsm.py
1 file changed, 21 insertions(+), 9 deletions(-)
Approvals:
Ayal Baron: Looks good to me, approved
Eduardo: Verified
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Daniel Paikov <paikov(a)gmail.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
9:29 a.m.
Yeela Kaplan has posted comments on this change.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Patch Set 2: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Daniel Paikov <paikov(a)gmail.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
11:53 p.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Patch Set 2:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2286/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1447/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2350/ : SUCCESS
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Daniel Paikov <paikov(a)gmail.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
11:51 p.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Patch Set 2:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1447/ (3/3)
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Daniel Paikov <paikov(a)gmail.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
11:50 p.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Patch Set 2:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2350/ (2/3)
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Daniel Paikov <paikov(a)gmail.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
11:48 p.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Patch Set 2:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2286/ (1/3)
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Daniel Paikov <paikov(a)gmail.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
1:20 a.m.
Yeela Kaplan has posted comments on this change.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(1 inline comment)
Just need to make one minor change, other than that it's great.
....................................................
File vdsm/storage/hsm.py
Line 2672: "attached to any pool", sdUUID,
exc_info=True)
Line 2673: except se.StoragePoolUnknown:
Line 2674: self.log.error("Domain %s, marked as master, is attached
to "
Line 2675: "pool %s but this host is connected to pool
%s",
Line 2676: sdUUID, pool.spUUID)
Remove redundant %s (3 instead of 2...)
"Domain %s, marked as master, is attached to pool %s but host is not connected to
pool"
Line 2677: else:
Line 2678: # make sure it's THE master of this pool
Line 2679: if pool.masterDomain.sdUUID != sdUUID:
Line 2680: self.log.error("Domain %s is marked as master but
actual "
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Daniel Paikov <paikov(a)gmail.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
1:14 a.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Patch Set 1:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2229/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1390/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2293/ : SUCCESS
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
1:10 a.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Patch Set 1:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2229/ (3/3)
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
1:10 a.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Patch Set 1:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2293/ (2/3)
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
1:10 a.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Patch Set 1:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1390/ (1/3)
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
1:52 a.m.
Ayal Baron has posted comments on this change.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Daniel Paikov <paikov(a)gmail.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
3:32 a.m.
Saggi Mizrahi has posted comments on this change.
Change subject: Fix getStorageDomainInfo() logic.
......................................................................
Patch Set 3:
(2 comments)
....................................................
File vdsm/storage/hsm.py
Line 2740: if info['role'] == sd.MASTER_DOMAIN:
Line 2741: try:
Line 2742: # Verify that the host is connected to the same pool which
Line 2743: # the SD is attached to.
Line 2744: pool = self.getPool(info['pool'][0])
https://gist.github.com/ficoos/6657930
Looks good to me. Since logs are pretty good comments in themselves when I keep them close
to the conditional the relationship is apparent.
Also, no we have a sanity check method that can be used wherever.
Line 2745: except IndexError:
Line 2746: self.log.error("Domain %s is marked as master but is not
"
Line 2747: "attached to any pool", sdUUID,
exc_info=True)
Line 2748: except se.StoragePoolUnknown:
Line 2752: exc_info=True)
Line 2753: else:
Line 2754: # make sure it's THE master of this pool
Line 2755: if pool.masterDomain.sdUUID != sdUUID:
Line 2756: self.log.error("Domain %s is marked as master but
actual "
Yea, why should the user be aware of it?
We never guarantee we only report things from the disk.
We report the role as we believe it to be.
There are two ways this can happen:
1. We had a reconstruct master and this is the old MD. This means *it is* a data domain we
just never got the time to put that on the disk
2. We have an error - in this case we need to lock the domain\pool since we have no idea
about the consistency of the pool. We don't know if locking works or if all the nodes
in the cluster are locking the same things.
So either crash, or fake it. It's your choice.
Line 2757: "master is %s",
Line 2758: sdUUID, pool.masterDomain.sdUUID)
Line 2759: else:
Line 2760: poolInfo = pool.getInfo()
--
To view, visit http://gerrit.ovirt.org/14671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Daniel Paikov <paikov(a)gmail.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
3862
days inactive
4010
days old
vdsm-patches@lists.fedorahosted.org
27 comments
7 participants
participants (7)
-
abaron＠redhat.com -
Dan Kenigsberg -
ewarszaw＠redhat.com -
Jenkins CI RO -
Oved Ourfali -
smizrahi＠redhat.com -
ykaplan＠redhat.com