Change in vdsm[master]: sp: move reconnection info check to StoragePool
Sergey Gotliv has posted comments on this change. Change subject: sp: move reconnection info check to StoragePool .. Patch Set 8: This is the bug I mentioned in my comment BZ#1026697 -- To view, visit http://gerrit.ovirt.org/21424 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia57afc04db02b6c15633d09349a55b3ff5ae7fda Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: move reconnection info check to StoragePool
Federico Simoncelli has posted comments on this change. Change subject: sp: move reconnection info check to StoragePool .. Patch Set 8: (3 comments) File vdsm/storage/hsm.py Line 1028: pass # pool not connected yet Line 1029: else: Line 1030: with rmanager.acquireResource(STORAGE, spUUID, rm.LockType.shared): Line 1031: self.getPool(spUUID).update(hostID, scsiKey, msdUUID, Line 1032: masterVersion) Yeah, actually in my latest code (sadly not pushed yet) update disappeared completely and I am just using refresh. (Sorry for wasting your time on describing this). Line 1033: return True Line 1034: Line 1035: with rmanager.acquireResource(STORAGE, spUUID, rm.LockType.exclusive): Line 1036: try: Line 1040: else: Line 1041: pool.update(hostID, scsiKey, msdUUID, masterVersion) Line 1042: return True Line 1043: Line 1044: pool = sp.StoragePool(spUUID, self.domainMonitor, self.taskMng) because we use connect at line 1052 Line 1045: Line 1046: # Must register domain state change callbacks *before* connecting Line 1047: # the pool, which starts domain monitor threads. Otherwise we will Line 1048: # miss the first event from the monitor thread. File vdsm/storage/sp.py Line 677: f.writelines(pers) Line 678: Line 679: @unsecured Line 680: def getPoolParams(self, hostID, scsiKey, msdUUID, masterVersion): Line 681: if all((hostID, scsiKey, msdUUID, masterVersion)): noted, please allow another couple of cycles with this behavior and then I'll take care of it. Line 682: return hostID, scsiKey, msdUUID, masterVersion Line 683: Line 684: self.log.info( Line 685: Using saved connection parameters: hostID=%s, scsiKey=%s, -- To view, visit http://gerrit.ovirt.org/21424 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia57afc04db02b6c15633d09349a55b3ff5ae7fda Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: move reconnection info check to StoragePool
Sergey Gotliv has posted comments on this change. Change subject: sp: move reconnection info check to StoragePool .. Patch Set 8: (1 comment) File vdsm/storage/hsm.py Line 1028: pass # pool not connected yet Line 1029: else: Line 1030: with rmanager.acquireResource(STORAGE, spUUID, rm.LockType.shared): Line 1031: self.getPool(spUUID).update(hostID, scsiKey, msdUUID, Line 1032: masterVersion) I enjoyed reviewing it so this is not the waste of the time :-). I will find a bug later so you can refer you new patch to it. Line 1033: return True Line 1034: Line 1035: with rmanager.acquireResource(STORAGE, spUUID, rm.LockType.exclusive): Line 1036: try: -- To view, visit http://gerrit.ovirt.org/21424 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia57afc04db02b6c15633d09349a55b3ff5ae7fda Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: move reconnection info check to StoragePool
Ayal Baron has posted comments on this change. Change subject: sp: move reconnection info check to StoragePool .. Patch Set 8: (2 comments) File vdsm/storage/hsm.py Line 1040: else: Line 1041: pool.update(hostID, scsiKey, msdUUID, masterVersion) Line 1042: return True Line 1043: Line 1044: pool = sp.StoragePool(spUUID, self.domainMonitor, self.taskMng) why is it ok to not add pool.update here? Line 1045: Line 1046: # Must register domain state change callbacks *before* connecting Line 1047: # the pool, which starts domain monitor threads. Otherwise we will Line 1048: # miss the first event from the monitor thread. File vdsm/storage/sp.py Line 677: f.writelines(pers) Line 678: Line 679: @unsecured Line 680: def getPoolParams(self, hostID, scsiKey, msdUUID, masterVersion): Line 681: if all((hostID, scsiKey, msdUUID, masterVersion)): I still hate this behaviour Line 682: return hostID, scsiKey, msdUUID, masterVersion Line 683: Line 684: self.log.info( Line 685: Using saved connection parameters: hostID=%s, scsiKey=%s, -- To view, visit http://gerrit.ovirt.org/21424 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia57afc04db02b6c15633d09349a55b3ff5ae7fda Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: move reconnection info check to StoragePool
Sergey Gotliv has posted comments on this change. Change subject: sp: move reconnection info check to StoragePool .. Patch Set 8: (1 comment) Please, check if my suggestion in hsm.py is doable. File vdsm/storage/hsm.py Line 1028: pass # pool not connected yet Line 1029: else: Line 1030: with rmanager.acquireResource(STORAGE, spUUID, rm.LockType.shared): Line 1031: self.getPool(spUUID).update(hostID, scsiKey, msdUUID, Line 1032: masterVersion) The problem described below is already exists before that patch, so I will completely understand if you will ignore it. The problem: In this case connectStoragePool detects that this host is already connected to the pool, but it still has to verify links to all storage domains. Let's assume that host (HSM) is a Non Operational which means it connected to pool but its not connected to one of its storage domains. When this connectivity issue is resolved the Engine tries to send connectStoragePool but since the pool is already connected VDSM doesn't try to check/restore links to the storage domains... And this logic is skipped without any log message. The problem I described above is real and easily reproducible, I can even reffer to the bug. 1. I would like to see the log message here that the pool is already connected. OR even better: 2. let's call refresh of links from update! Line 1033: return True Line 1034: Line 1035: with rmanager.acquireResource(STORAGE, spUUID, rm.LockType.exclusive): Line 1036: try: -- To view, visit http://gerrit.ovirt.org/21424 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia57afc04db02b6c15633d09349a55b3ff5ae7fda Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: move reconnection info check to StoragePool
oVirt Jenkins CI Server has posted comments on this change. Change subject: sp: move reconnection info check to StoragePool .. Patch Set 8: Build Successful http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5949/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5153/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6041/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/21424 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia57afc04db02b6c15633d09349a55b3ff5ae7fda Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: move reconnection info check to StoragePool
oVirt Jenkins CI Server has posted comments on this change. Change subject: sp: move reconnection info check to StoragePool .. Patch Set 6: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4954/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5754/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5844/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/21424 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia57afc04db02b6c15633d09349a55b3ff5ae7fda Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: move reconnection info check to StoragePool
oVirt Jenkins CI Server has posted comments on this change. Change subject: sp: move reconnection info check to StoragePool .. Patch Set 7: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4984/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5784/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5874/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/21424 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia57afc04db02b6c15633d09349a55b3ff5ae7fda Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: move reconnection info check to StoragePool
Federico Simoncelli has posted comments on this change. Change subject: sp: move reconnection info check to StoragePool .. Patch Set 3: (4 comments) File vdsm/storage/hsm.py Line 1027: except se.StoragePoolUnknown: Line 1028: pass # pool not connected yet Line 1029: else: Line 1030: with rmanager.acquireResource(STORAGE, spUUID, rm.LockType.shared): Line 1031: pool.update(hostID, scsiKey, msdUUID, masterVersion) Done Line 1032: return True Line 1033: Line 1034: with rmanager.acquireResource(STORAGE, spUUID, rm.LockType.exclusive): Line 1035: try: Line 1036: pool = self.getPool(spUUID) Line 1037: except se.StoragePoolUnknown: Line 1038: pass # pool not connected yet Line 1039: else: Line 1040: pool.update(hostID, scsiKey, msdUUID, masterVersion) No, here it's correct, we're under the exclusive lock. Line 1041: return True Line 1042: Line 1043: pool = sp.StoragePool(spUUID, self.domainMonitor, self.taskMng) Line 1044: res = pool.connect(hostID, scsiKey, msdUUID, masterVersion) File vdsm/storage/sp.py Line 688: msg = Pools temp data dir: %s does not exist % ( Line 689: self._poolsTmpDir) Line 690: raise se.StoragePoolConnectionError(msg) Line 691: Line 692: self._saveReconnectInformation(hostID, scsiKey, msdUUID, masterVersion) I'm going to refactor out the pool persistency in a patch that I haven't pushed yet. We'll discuss this there. Line 693: self.id = hostID Line 694: self.scsiKey = scsiKey Line 695: # Make sure SDCache doesn't have stale data (it can be in case of FC) Line 696: sdCache.invalidateStorage() Line 741: Line 742: @unsecured Line 743: def getPoolParams(self, hostID, scsiKey, msdUUID, masterVersion): Line 744: if all(hostID, scsiKey, msdUUID, masterVersion): Line 745: return hostID, scsiKey, msdUUID, masterVersion this is as it was in hsm.py: if not hostID or not scsiKey or not msdUUID or not masterVersion: hostID, scsiKey, msdUUID, masterVersion = pool.getPoolParams() Line 746: Line 747: self.log.info( Line 748: Using saved connection parameters: hostID=%s, scsiKey=%s, Line 749: msdUUID=%s, masterVersion=%s, hostID, scsiKey, msdUUID, -- To view, visit http://gerrit.ovirt.org/21424 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia57afc04db02b6c15633d09349a55b3ff5ae7fda Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: move reconnection info check to StoragePool
oVirt Jenkins CI Server has posted comments on this change. Change subject: sp: move reconnection info check to StoragePool .. Patch Set 5: Code-Review-1 Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4919/ : FAILURE http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5719/ : UNSTABLE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5808/ : FAILURE -- To view, visit http://gerrit.ovirt.org/21424 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia57afc04db02b6c15633d09349a55b3ff5ae7fda Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: move reconnection info check to StoragePool
Federico Simoncelli has posted comments on this change. Change subject: sp: move reconnection info check to StoragePool .. Patch Set 3: Ayal, your comments are mostly correct, but sadly this patch is at the base of many other. The entire series is still under heavy development and I don't want to focus on fixing things that might be thrown away completely at the end. In my tests this is good enough (as my bigger concern atm is to decouple hsm and sp). Don't think I'm ignoring your comments, I'm just postponing them until I'll publish the draft. -- To view, visit http://gerrit.ovirt.org/21424 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia57afc04db02b6c15633d09349a55b3ff5ae7fda Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: move reconnection info check to StoragePool
oVirt Jenkins CI Server has posted comments on this change. Change subject: sp: move reconnection info check to StoragePool .. Patch Set 4: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4881/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5681/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5767/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/21424 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia57afc04db02b6c15633d09349a55b3ff5ae7fda Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: move reconnection info check to StoragePool
Ayal Baron has posted comments on this change. Change subject: sp: move reconnection info check to StoragePool .. Patch Set 3: Code-Review-1 (5 comments) File vdsm/storage/hsm.py Line 1027: except se.StoragePoolUnknown: Line 1028: pass # pool not connected yet Line 1029: else: Line 1030: with rmanager.acquireResource(STORAGE, spUUID, rm.LockType.shared): Line 1031: pool.update(hostID, scsiKey, msdUUID, masterVersion) after acquiring the lock we can't even be sure we're connected to the same pool (chances are slim, but still)... Line 1032: return True Line 1033: Line 1034: with rmanager.acquireResource(STORAGE, spUUID, rm.LockType.exclusive): Line 1035: try: Line 1036: pool = self.getPool(spUUID) Line 1037: except se.StoragePoolUnknown: Line 1038: pass # pool not connected yet Line 1039: else: Line 1040: pool.update(hostID, scsiKey, msdUUID, masterVersion) same Line 1041: return True Line 1042: Line 1043: pool = sp.StoragePool(spUUID, self.domainMonitor, self.taskMng) Line 1044: res = pool.connect(hostID, scsiKey, msdUUID, masterVersion) File vdsm/storage/sp.py Line 688: msg = Pools temp data dir: %s does not exist % ( Line 689: self._poolsTmpDir) Line 690: raise se.StoragePoolConnectionError(msg) Line 691: Line 692: self._saveReconnectInformation(hostID, scsiKey, msdUUID, masterVersion) I wonder if the mistake is not reverse than the purpose of this patch. i.e. not that reading the reconnection info belong inside connect but rather that persisting it belongs outside. this method has 2 different ways of getting its parameters and if someone called it and did not pass them then we fail back to the other method. Instead, maybe we should have a separate reconnect method and whenever connect here is called, the parameters have to be passed. The only place we actually call connectStoragePool without params is in the init of hsm. So we could just retrieve the info there and persist it also at hsm level in connect. Alternatively, I'm not even clear on whether this still works and engine automatically reconnects and we also support running VMs without being connected to a pool so I wonder if we shouldn't just drop all the reconnect logic and be done with it (esp. as what we're trying to do is get rid of the pool here). We will need to automatically connect to domains later on (acquire lock space) to cut on startup time, but that is pretty much a different issue entirely. Line 693: self.id = hostID Line 694: self.scsiKey = scsiKey Line 695: # Make sure SDCache doesn't have stale data (it can be in case of FC) Line 696: sdCache.invalidateStorage() Line 741: Line 742: @unsecured Line 743: def getPoolParams(self, hostID, scsiKey, msdUUID, masterVersion): Line 744: if all(hostID, scsiKey, msdUUID, masterVersion): Line 745: return hostID, scsiKey, msdUUID, masterVersion this makes things even more weird. if user passed params - return the params right back. if not - read from disk and return Line 746: Line 747: self.log.info( Line 748: Using saved connection parameters: hostID=%s, scsiKey=%s, Line 749: msdUUID=%s, masterVersion=%s, hostID, scsiKey, msdUUID, Line 752: with open(self._poolFile, r) as f: Line 753: poolInfo = dict((x.strip().split(=, 1) for x in f)) Line 754: Line 755: return tuple(poolInfo.get(x) for x in Line 756: (hostId, scsiKey, msdUUID, masterVersion)) same as patch set 1 or at least explain why I'm wrong? Line 757: Line 758: @unsecured Line 759: def createMaster(self, poolName, domain, masterVersion, leaseParams): Line 760: -- To view, visit http://gerrit.ovirt.org/21424 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia57afc04db02b6c15633d09349a55b3ff5ae7fda Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: move reconnection info check to StoragePool
oVirt Jenkins CI Server has posted comments on this change. Change subject: sp: move reconnection info check to StoragePool .. Patch Set 3: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4833/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5633/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5716/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/21424 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia57afc04db02b6c15633d09349a55b3ff5ae7fda Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: move reconnection info check to StoragePool
Federico Simoncelli has uploaded a new change for review. Change subject: sp: move reconnection info check to StoragePool .. sp: move reconnection info check to StoragePool The entire reconnection info logic belongs to the StoragePool class. Change-Id: Ia57afc04db02b6c15633d09349a55b3ff5ae7fda Signed-off-by: Federico Simoncelli fsimo...@redhat.com --- M vdsm/storage/hsm.py M vdsm/storage/sp.py 2 files changed, 11 insertions(+), 17 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/24/21424/1 diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py index 5e53f3a..68bb4ca 100644 --- a/vdsm/storage/hsm.py +++ b/vdsm/storage/hsm.py @@ -1056,9 +1056,8 @@ return pool = sp.StoragePool(spUUID, self.domainMonitor, self.taskMng) -if not hostID or not scsiKey or not msdUUID or not masterVersion: -hostID, scsiKey, msdUUID, masterVersion = pool.getPoolParams() res = pool.connect(hostID, scsiKey, msdUUID, masterVersion) + if res: self.pools[spUUID] = pool for callback in self.domainStateChangeCallbacks: diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py index 0e23e1d..bf49b79 100644 --- a/vdsm/storage/sp.py +++ b/vdsm/storage/sp.py @@ -677,6 +677,12 @@ domain: %s (ver = %s) % (hostID, self.spUUID, msdUUID, masterVersion)) +if not hostID or not scsiKey or not msdUUID or not masterVersion: +hostID, scsiKey, msdUUID, masterVersion = self.getPoolParams() +self.log.info(Using saved connection parameters: hostID=%s + scsiKey=%s, msdUUID=%s, masterVersion=%s, + hostID, scsiKey, msdUUID, masterVersion) + if not os.path.exists(self._poolsTmpDir): msg = (StoragePoolConnectionError for hostId: %s, on poolId: %s, Pools temp data dir: %s does not exist % @@ -732,21 +738,10 @@ @unsecured def getPoolParams(self): -file = open(self._poolFile, r) -for line in file: -pair = line.strip().split(=) -if len(pair) == 2: -if pair[0] == id: -hostId = int(pair[1]) -elif pair[0] == scsiKey: -scsiKey = pair[1] -elif pair[0] == sdUUID: -msdUUID = pair[1] -elif pair[0] == version: -masterVersion = pair[1] -file.close() - -return hostId, scsiKey, msdUUID, masterVersion +with open(self._poolFile, r) as f: +poolInfo = dict((x.strip().split(=, 1) for x in f)) +return tuple(poolInfo.get(x) for x in + (hostId, scsiKey, msdUUID, masterVersion)) @unsecured def createMaster(self, poolName, domain, masterVersion, leaseParams): -- To view, visit http://gerrit.ovirt.org/21424 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia57afc04db02b6c15633d09349a55b3ff5ae7fda Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: move reconnection info check to StoragePool
oVirt Jenkins CI Server has posted comments on this change. Change subject: sp: move reconnection info check to StoragePool .. Patch Set 1: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4720/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5520/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5600/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/21424 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia57afc04db02b6c15633d09349a55b3ff5ae7fda Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: move reconnection info check to StoragePool
Ayal Baron has posted comments on this change. Change subject: sp: move reconnection info check to StoragePool .. Patch Set 1: Code-Review-1 (2 comments) File vdsm/storage/sp.py Line 676: self.log.info(Connect host #%s to the storage pool %s with master Line 677: domain: %s (ver = %s) % Line 678: (hostID, self.spUUID, msdUUID, masterVersion)) Line 679: Line 680: if not hostID or not scsiKey or not msdUUID or not masterVersion: this should come after 'if not os.path.exists' and self._saveReconnectInformation should be in the 'else' if not hostID or ... hostID, ... ... else: self._saveReconnectInformation Line 681: hostID, scsiKey, msdUUID, masterVersion = self.getPoolParams() Line 682: self.log.info(Using saved connection parameters: hostID=%s Line 683: scsiKey=%s, msdUUID=%s, masterVersion=%s, Line 684: hostID, scsiKey, msdUUID, masterVersion) Line 740: def getPoolParams(self): Line 741: with open(self._poolFile, r) as f: Line 742: poolInfo = dict((x.strip().split(=, 1) for x in f)) Line 743: return tuple(poolInfo.get(x) for x in Line 744: (hostId, scsiKey, msdUUID, masterVersion)) previously the code exploded on the return statement if one of these was not available, now we return None. this means that in the least, connect should validate that all are present or something Line 745: Line 746: @unsecured Line 747: def createMaster(self, poolName, domain, masterVersion, leaseParams): Line 748: -- To view, visit http://gerrit.ovirt.org/21424 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia57afc04db02b6c15633d09349a55b3ff5ae7fda Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
