10:37 a.m.
Federico Simoncelli has uploaded a new change for review.
Change subject: sdcache: add refresh to connectStoragePool
......................................................................
sdcache: add refresh to connectStoragePool
Change-Id: I2d3adcff7bb0e97be5c797cd720c6353920d9db0
Signed-off-by: Federico Simoncelli <fsimonce(a)redhat.com>
---
M vdsm/storage/sp.py
1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/22/9422/1
diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py
index ac5e1db..2577db7 100644
--- a/vdsm/storage/sp.py
+++ b/vdsm/storage/sp.py
@@ -640,6 +640,7 @@
self.scsiKey = scsiKey
# Make sure SDCache doesn't have stale data (it can be in case of FC)
sdCache.invalidateStorage()
+ sdCache.refresh()
# Rebuild whole Pool
self.__rebuild(msdUUID=msdUUID, masterVersion=masterVersion)
self.__createMailboxMonitor()
--
To view, visit http://gerrit.ovirt.org/9422
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2d3adcff7bb0e97be5c797cd720c6353920d9db0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>
4:08 a.m.
Dan Kenigsberg has submitted this change and it was merged.
Change subject: sdcache: add refresh to connectStoragePool
......................................................................
sdcache: add refresh to connectStoragePool
When connecting to a storage pool we should clear the domain cache to
pick up all the changes that might have been introduced by other hosts.
Additionally since StoragePool.refresh has a similar behavior to
connectStoragePool it should also contain the invalidateStorage call to
force an iscsi rescan.
In this patch:
* reinstate the sdCache.refresh call in connectStoragePool
* add invalidateStorage to StoragePool.refresh
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=879253
Change-Id: I2d3adcff7bb0e97be5c797cd720c6353920d9db0
Signed-off-by: Federico Simoncelli <fsimonce(a)redhat.com>
---
M vdsm/storage/sp.py
1 file changed, 2 insertions(+), 0 deletions(-)
Approvals:
Ayal Baron: Looks good to me, approved
Daniel Paikov: Verified
Dan Kenigsberg:
--
To view, visit http://gerrit.ovirt.org/9422
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I2d3adcff7bb0e97be5c797cd720c6353920d9db0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(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: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
4:08 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: sdcache: add refresh to connectStoragePool
......................................................................
Patch Set 2:
I cannot say that I understand this, but at least I'm convinced that Federico does.
--
To view, visit http://gerrit.ovirt.org/9422
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3adcff7bb0e97be5c797cd720c6353920d9db0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(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: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
3:42 a.m.
Federico Simoncelli has posted comments on this change.
Change subject: sdcache: add refresh to connectStoragePool
......................................................................
Patch Set 2: (1 inline comment)
....................................................
File vdsm/storage/sp.py
Line 639: self.id = hostID
Line 640: self.scsiKey = scsiKey
Line 641: # Make sure SDCache doesn't have stale data (it can be in case of
FC)
Line 642: sdCache.invalidateStorage()
Line 643: sdCache.refresh()
At least for me this doesn't look like running in circles. You probably didn't
notice that in 9275 refresh was substituted by invalidateStorage (rescan multipath). That
would have been correct if it wasn't for a special case (when you start the spm on a
different node where you created the pool). In such case you also need to clean up
(refresh) the domain cache. That is why we now need both. I agree that this looks very
similar to self.refresh(). If you want I can resend the patch calling self.refresh() here,
I think it's cleaner.
Line 644: # Rebuild whole Pool
Line 645: self.__rebuild(msdUUID=msdUUID, masterVersion=masterVersion)
Line 646: self.__createMailboxMonitor()
Line 647:
--
To view, visit http://gerrit.ovirt.org/9422
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3adcff7bb0e97be5c797cd720c6353920d9db0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(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: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
12:39 p.m.
Dan Kenigsberg has posted comments on this change.
Change subject: sdcache: add refresh to connectStoragePool
......................................................................
Patch Set 2: (1 inline comment)
....................................................
File vdsm/storage/sp.py
Line 639: self.id = hostID
Line 640: self.scsiKey = scsiKey
Line 641: # Make sure SDCache doesn't have stale data (it can be in case of
FC)
Line 642: sdCache.invalidateStorage()
Line 643: sdCache.refresh()
Sorry, I find it very hard to follow. was http://gerrit.ovirt.org/9275 a mistake? is bug
https://bugzilla.redhat.com/show_bug.cgi?id=870768 still solved?
Besides that, the code here looks very similar to self.refresh().
I know this patch has all the acks, but it seems that we're running in circles.
Line 644: # Rebuild whole Pool
Line 645: self.__rebuild(msdUUID=msdUUID, masterVersion=masterVersion)
Line 646: self.__createMailboxMonitor()
Line 647:
--
To view, visit http://gerrit.ovirt.org/9422
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3adcff7bb0e97be5c797cd720c6353920d9db0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(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: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
7:46 a.m.
Daniel Paikov has posted comments on this change.
Change subject: sdcache: add refresh to connectStoragePool
......................................................................
Patch Set 2: Verified
--
To view, visit http://gerrit.ovirt.org/9422
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3adcff7bb0e97be5c797cd720c6353920d9db0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Daniel Paikov <paikov(a)gmail.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
5:19 a.m.
Ayal Baron has posted comments on this change.
Change subject: sdcache: add refresh to connectStoragePool
......................................................................
Patch Set 2: Looks good to me, approved
It looks good to me, but it needs thorough testing with many storage domains to make sure
it's behaving properly.
--
To view, visit http://gerrit.ovirt.org/9422
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3adcff7bb0e97be5c797cd720c6353920d9db0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
10:23 a.m.
Federico Simoncelli has posted comments on this change.
Change subject: sdcache: add refresh to connectStoragePool
......................................................................
Patch Set 2: (1 inline comment)
....................................................
File vdsm/storage/sp.py
Line 1239: """
Line 1240: Refresh storage pool.
Line 1241: 'msdUUID' - master storage domain UUID
Line 1242: """
Line 1243: sdCache.invalidateStorage()
Now I'm actually worried about this change. We're injecting a "random"
delay (iscsi-rescan time) somewhere in any following domain call/property-evaluation.
Having it in connectStoragePool is safe because we're not connected to the pool yet
but here we might have tasks going on (and the domain monitors).
Line 1244: sdCache.refresh()
Line 1245: self.__rebuild(msdUUID=msdUUID, masterVersion=masterVersion)
Line 1246:
Line 1247:
--
To view, visit http://gerrit.ovirt.org/9422
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3adcff7bb0e97be5c797cd720c6353920d9db0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
10:21 a.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: sdcache: add refresh to connectStoragePool
......................................................................
Patch Set 2:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/174/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/208/ : SUCCESS
--
To view, visit http://gerrit.ovirt.org/9422
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3adcff7bb0e97be5c797cd720c6353920d9db0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
10:19 a.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: sdcache: add refresh to connectStoragePool
......................................................................
Patch Set 2:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/174/ (2/2)
--
To view, visit http://gerrit.ovirt.org/9422
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3adcff7bb0e97be5c797cd720c6353920d9db0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
10:19 a.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: sdcache: add refresh to connectStoragePool
......................................................................
Patch Set 2:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/208/ (1/2)
--
To view, visit http://gerrit.ovirt.org/9422
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3adcff7bb0e97be5c797cd720c6353920d9db0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
3:29 p.m.
Federico Simoncelli has posted comments on this change.
Change subject: sdcache: add refresh to connectStoragePool
......................................................................
Patch Set 1: (2 inline comments)
....................................................
File vdsm/storage/sp.py
Line 638: self._saveReconnectInformation(hostID, scsiKey, msdUUID, masterVersion)
Line 639: self.id = hostID
Line 640: self.scsiKey = scsiKey
Line 641: # Make sure SDCache doesn't have stale data (it can be in case of
FC)
Line 642: sdCache.invalidateStorage()
Yes it might be useful to have it there too to trigger a rescan.
Line 643: sdCache.refresh()
Line 644: # Rebuild whole Pool
Line 645: self.__rebuild(msdUUID=msdUUID, masterVersion=masterVersion)
Line 646: self.__createMailboxMonitor()
Line 639: self.id = hostID
Line 640: self.scsiKey = scsiKey
Line 641: # Make sure SDCache doesn't have stale data (it can be in case of
FC)
Line 642: sdCache.invalidateStorage()
Line 643: sdCache.refresh()
This is not needed in connectStorageServer because you would delete the entire domain
cache when you connect to a new storage server. E.g.: you want to add a new storage =>
you would clean the cache of all the know domains.
I think you're looking forward to unify the methods, and I actually agree, I thought
about it a couple of times. We might try to do so and add a parameter to skip the domain
cache clean up in the few cases we don't want it.
Line 644: # Rebuild whole Pool
Line 645: self.__rebuild(msdUUID=msdUUID, masterVersion=masterVersion)
Line 646: self.__createMailboxMonitor()
Line 647:
--
To view, visit http://gerrit.ovirt.org/9422
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3adcff7bb0e97be5c797cd720c6353920d9db0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
5:26 p.m.
Ayal Baron has posted comments on this change.
Change subject: sdcache: add refresh to connectStoragePool
......................................................................
Patch Set 1: (2 inline comments)
....................................................
File vdsm/storage/sp.py
Line 638: self._saveReconnectInformation(hostID, scsiKey, msdUUID, masterVersion)
Line 639: self.id = hostID
Line 640: self.scsiKey = scsiKey
Line 641: # Make sure SDCache doesn't have stale data (it can be in case of
FC)
Line 642: sdCache.invalidateStorage()
why is this not needed in refresh storage pool? (i.e. sp.py - refresh method) which is the
only other method which calls sdCache.refresh(0
Line 643: sdCache.refresh()
Line 644: # Rebuild whole Pool
Line 645: self.__rebuild(msdUUID=msdUUID, masterVersion=masterVersion)
Line 646: self.__createMailboxMonitor()
Line 639: self.id = hostID
Line 640: self.scsiKey = scsiKey
Line 641: # Make sure SDCache doesn't have stale data (it can be in case of
FC)
Line 642: sdCache.invalidateStorage()
Line 643: sdCache.refresh()
why isn't this needed in connectStorageServer? (the only other place which calls
invalidateStorage)?
Line 644: # Rebuild whole Pool
Line 645: self.__rebuild(msdUUID=msdUUID, masterVersion=masterVersion)
Line 646: self.__createMailboxMonitor()
Line 647:
--
To view, visit http://gerrit.ovirt.org/9422
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3adcff7bb0e97be5c797cd720c6353920d9db0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
10:37 a.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: sdcache: add refresh to connectStoragePool
......................................................................
Patch Set 1:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/112/ (2/2)
--
To view, visit http://gerrit.ovirt.org/9422
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3adcff7bb0e97be5c797cd720c6353920d9db0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
10:39 a.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: sdcache: add refresh to connectStoragePool
......................................................................
Patch Set 1:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/112/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/146/ : SUCCESS
--
To view, visit http://gerrit.ovirt.org/9422
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3adcff7bb0e97be5c797cd720c6353920d9db0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
10:37 a.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: sdcache: add refresh to connectStoragePool
......................................................................
Patch Set 1:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/146/ (1/2)
--
To view, visit http://gerrit.ovirt.org/9422
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3adcff7bb0e97be5c797cd720c6353920d9db0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
4192
days inactive
4206
days old
vdsm-patches@lists.fedorahosted.org
15 comments
5 participants
participants (5)
-
abaron＠redhat.com -
Dan Kenigsberg -
Federico Simoncelli -
Jenkins CI RO -
paikov＠gmail.com