Change in vdsm[master]: domainMonitor: Fix unsafe iteration
Dan Kenigsberg has submitted this change and it was merged. Change subject: domainMonitor: Fix unsafe iteration .. domainMonitor: Fix unsafe iteration Using dict.iteritems() is safe only if nobody else can modify the dictionary while iterating. However in DomainMonitor, others threads can add or remove items during the iteration, so we must use dict.items(), which copies the keys and values atomically. dict.iteritems() is useful when we have huge dict and we don't want to create a huge list of items, but in case of the domain monitor, where we have less then 100 monitors, there is not need for fancy iteration. While modifying this line, I also improve the names to make the code more clear, this can be separated to another patch but I don't think it is needed in this case. Change-Id: I5313d25c7148e4a0362d19fe31d2e78f94c26e39 Signed-off-by: Nir Soffer nsof...@redhat.com Reviewed-on: http://gerrit.ovirt.org/29007 Reviewed-by: Yoav Kleinberger yklei...@redhat.com Reviewed-by: Allon Mureinik amure...@redhat.com Reviewed-by: Federico Simoncelli fsimo...@redhat.com --- M vdsm/storage/domainMonitor.py 1 file changed, 2 insertions(+), 1 deletion(-) Approvals: Nir Soffer: Verified Federico Simoncelli: Looks good to me, approved Yoav Kleinberger: Looks good to me, but someone else must approve Allon Mureinik: Looks good to me, but someone else must approve -- To view, visit http://gerrit.ovirt.org/29007 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I5313d25c7148e4a0362d19fe31d2e78f94c26e39 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Xavi Francisco xfran...@redhat.com Gerrit-Reviewer: Yoav Kleinberger yklei...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: domainMonitor: Fix unsafe iteration
oVirt Jenkins CI Server has posted comments on this change. Change subject: domainMonitor: Fix unsafe iteration .. Patch Set 2: Build Successful http://jenkins.ovirt.org/job/vdsm_master_create-rpms_merged/1555/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5540/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3698/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/29007 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5313d25c7148e4a0362d19fe31d2e78f94c26e39 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Xavi Francisco xfran...@redhat.com Gerrit-Reviewer: Yoav Kleinberger yklei...@redhat.com Gerrit-Reviewer: automat...@ovirt.org 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]: domainMonitor: Fix unsafe iteration of domain monitor status
Dan Kenigsberg has submitted this change and it was merged. Change subject: domainMonitor: Fix unsafe iteration of domain monitor status .. domainMonitor: Fix unsafe iteration of domain monitor status When we generate repoStat response, we used to iterate over the monitored domains, getting the status for each monitor. This naïve approach does not consider that a monitor may be removed from the domain monitor dict during the iteration. Since the only usage now is getting the status for all domains, I changed getStatus to return an iterator returning all domains status in a safe manner. Change-Id: I083c4b1925ee529c0993a291cad6076539d4ea66 Signed-off-by: Nir Soffer nsof...@redhat.com Reviewed-on: http://gerrit.ovirt.org/29009 Reviewed-by: Yoav Kleinberger yklei...@redhat.com Reviewed-by: Federico Simoncelli fsimo...@redhat.com --- M vdsm/storage/domainMonitor.py M vdsm/storage/hsm.py 2 files changed, 4 insertions(+), 5 deletions(-) Approvals: Nir Soffer: Verified Federico Simoncelli: Looks good to me, approved Yoav Kleinberger: Looks good to me, but someone else must approve -- To view, visit http://gerrit.ovirt.org/29009 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I083c4b1925ee529c0993a291cad6076539d4ea66 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Xavi Francisco xfran...@redhat.com Gerrit-Reviewer: Yoav Kleinberger yklei...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: domainMonitor: Fix unsafe iteration of domain monitor status
oVirt Jenkins CI Server has posted comments on this change. Change subject: domainMonitor: Fix unsafe iteration of domain monitor status .. Patch Set 2: Build Successful http://jenkins.ovirt.org/job/vdsm_master_create-rpms_merged/1557/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5542/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3700/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/29009 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I083c4b1925ee529c0993a291cad6076539d4ea66 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Xavi Francisco xfran...@redhat.com Gerrit-Reviewer: Yoav Kleinberger yklei...@redhat.com Gerrit-Reviewer: automat...@ovirt.org 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]: domainMonitor: Fix unsafe iteration of domain monitor status
Federico Simoncelli has posted comments on this change. Change subject: domainMonitor: Fix unsafe iteration of domain monitor status .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/29009 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I083c4b1925ee529c0993a291cad6076539d4ea66 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Xavi Francisco xfran...@redhat.com Gerrit-Reviewer: Yoav Kleinberger yklei...@redhat.com Gerrit-Reviewer: automat...@ovirt.org 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]: domainMonitor: Fix unsafe iteration
Federico Simoncelli has posted comments on this change. Change subject: domainMonitor: Fix unsafe iteration .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/29007 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5313d25c7148e4a0362d19fe31d2e78f94c26e39 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Xavi Francisco xfran...@redhat.com Gerrit-Reviewer: Yoav Kleinberger yklei...@redhat.com Gerrit-Reviewer: automat...@ovirt.org 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]: domainMonitor: Fix unsafe iteration
Allon Mureinik has posted comments on this change. Change subject: domainMonitor: Fix unsafe iteration .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/29007 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5313d25c7148e4a0362d19fe31d2e78f94c26e39 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Xavi Francisco xfran...@redhat.com Gerrit-Reviewer: Yoav Kleinberger yklei...@redhat.com Gerrit-Reviewer: automat...@ovirt.org 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]: domainMonitor: Fix unsafe iteration of domain monitor status
Yoav Kleinberger has posted comments on this change. Change subject: domainMonitor: Fix unsafe iteration of domain monitor status .. Patch Set 1: Code-Review-1 (2 comments) http://gerrit.ovirt.org/#/c/29009/1/vdsm/storage/domainMonitor.py File vdsm/storage/domainMonitor.py: Line 119 Line 120 Line 121 Line 122 Line 123 as long as we're changing a public function, how about less name inflation? getStatus instead of getMonitoredDomainsStatus Line 118: Line 119: del self._domains[sdUUID] Line 120: Line 121: def getMonitoredDomainsStatus(self): Line 122: for sdUUID, monitor in self._domains.items(): how is this safer? if _domains changes, the list you get from items is invalid, and you will yield invalid values. What am I missing? Line 123: yield sdUUID, monitor.getStatus() Line 124: Line 125: def close(self): Line 126: self.log.info(Stopping domain monitors) -- To view, visit http://gerrit.ovirt.org/29009 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I083c4b1925ee529c0993a291cad6076539d4ea66 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Xavi Francisco xfran...@redhat.com Gerrit-Reviewer: Yoav Kleinberger yklei...@redhat.com Gerrit-Reviewer: automat...@ovirt.org 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]: domainMonitor: Fix unsafe iteration of domain monitor status
Nir Soffer has posted comments on this change. Change subject: domainMonitor: Fix unsafe iteration of domain monitor status .. Patch Set 1: (2 comments) http://gerrit.ovirt.org/#/c/29009/1/vdsm/storage/domainMonitor.py File vdsm/storage/domainMonitor.py: Line 119 Line 120 Line 121 Line 122 Line 123 as long as we're changing a public function, how about less name inflation? getStatus was used to get one domain status, we are returning here all domains status, and this is what the new name is trying to make clear. Line 118: Line 119: del self._domains[sdUUID] Line 120: Line 121: def getMonitoredDomainsStatus(self): Line 122: for sdUUID, monitor in self._domains.items(): how is this safer? The list you get is valid for the time you ask about it, and this is the list you should report to the engine back. If a domain was stopped 1 second or 1 millisecond after you asked it is not interesting. This is like doing stat on a file - anything you return may be wrong because the file may have changed after you checked its stats. Line 123: yield sdUUID, monitor.getStatus() Line 124: Line 125: def close(self): Line 126: self.log.info(Stopping domain monitors) -- To view, visit http://gerrit.ovirt.org/29009 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I083c4b1925ee529c0993a291cad6076539d4ea66 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Xavi Francisco xfran...@redhat.com Gerrit-Reviewer: Yoav Kleinberger yklei...@redhat.com Gerrit-Reviewer: automat...@ovirt.org 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]: domainMonitor: Fix unsafe iteration of domain monitor status
Yoav Kleinberger has posted comments on this change. Change subject: domainMonitor: Fix unsafe iteration of domain monitor status .. Patch Set 1: (2 comments) http://gerrit.ovirt.org/#/c/29009/1/vdsm/storage/domainMonitor.py File vdsm/storage/domainMonitor.py: Line 119 Line 120 Line 121 Line 122 Line 123 getStatus was used to get one domain status, we are returning here all doma the object you call getStatus on gives you the proper context to understand the meaning of getStatus. Look at the usage of getMonitoredDomainsStatus - the domainMonitor object clarifies that you get the status for the monitored domains. the same goes for this bit: it's clear that you get the status of a single domain. Line 118: Line 119: del self._domains[sdUUID] Line 120: Line 121: def getMonitoredDomainsStatus(self): Line 122: for sdUUID, monitor in self._domains.items(): The list you get is valid for the time you ask about it, and this is the li OK Line 123: yield sdUUID, monitor.getStatus() Line 124: Line 125: def close(self): Line 126: self.log.info(Stopping domain monitors) -- To view, visit http://gerrit.ovirt.org/29009 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I083c4b1925ee529c0993a291cad6076539d4ea66 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Xavi Francisco xfran...@redhat.com Gerrit-Reviewer: Yoav Kleinberger yklei...@redhat.com Gerrit-Reviewer: automat...@ovirt.org 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]: domainMonitor: Fix unsafe iteration
Yoav Kleinberger has posted comments on this change. Change subject: domainMonitor: Fix unsafe iteration .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/29007 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5313d25c7148e4a0362d19fe31d2e78f94c26e39 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Xavi Francisco xfran...@redhat.com Gerrit-Reviewer: Yoav Kleinberger yklei...@redhat.com Gerrit-Reviewer: automat...@ovirt.org 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]: domainMonitor: Fix unsafe iteration of domain monitor status
Nir Soffer has posted comments on this change. Change subject: domainMonitor: Fix unsafe iteration of domain monitor status .. Patch Set 1: (1 comment) http://gerrit.ovirt.org/#/c/29009/1/vdsm/storage/domainMonitor.py File vdsm/storage/domainMonitor.py: Line 119 Line 120 Line 121 Line 122 Line 123 the object you call getStatus on gives you the proper context to understand When you getStatus, you expect it ot return one value. We cannot use this to return a generator. Note that we have: domainMonitor.monitoredDomains domainMonitor.poolMonitoredDomains So the new method use the same convention: domainMonitor.getMonitoredDomainsStatus If we go with your suggestion we have to rename monitoredDomains to domains and poolDomains. This may be better I don't want to fix the names in this patch, only fix the issue with the iteration. I think we can fix the names in a following patch, including the privacy changes you asked in the other patch. -- To view, visit http://gerrit.ovirt.org/29009 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I083c4b1925ee529c0993a291cad6076539d4ea66 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Xavi Francisco xfran...@redhat.com Gerrit-Reviewer: Yoav Kleinberger yklei...@redhat.com Gerrit-Reviewer: automat...@ovirt.org 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]: domainMonitor: Fix unsafe iteration of domain monitor status
Yoav Kleinberger has posted comments on this change. Change subject: domainMonitor: Fix unsafe iteration of domain monitor status .. Patch Set 1: Code-Review+1 (1 comment) http://gerrit.ovirt.org/#/c/29009/1/vdsm/storage/domainMonitor.py File vdsm/storage/domainMonitor.py: Line 119 Line 120 Line 121 Line 122 Line 123 When you getStatus, you expect it ot return one value. We cannot use this t OK -- To view, visit http://gerrit.ovirt.org/29009 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I083c4b1925ee529c0993a291cad6076539d4ea66 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Xavi Francisco xfran...@redhat.com Gerrit-Reviewer: Yoav Kleinberger yklei...@redhat.com Gerrit-Reviewer: automat...@ovirt.org 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]: domainMonitor: Fix unsafe iteration
Nir Soffer has uploaded a new change for review. Change subject: domainMonitor: Fix unsafe iteration .. domainMonitor: Fix unsafe iteration Using dict.iteritems() is safe only if nobody else can modify the dictionary while iterating. However in DomainMonitor, others threads can add or remove items during the iteration, so we must use dict.items(), which copies the keys and values atomically. dict.iteritems() is useful when we have huge dict and we don't want to create a huge list of items, but in case of the domain monitor, where we have less then 100 monitors, there is not need for fancy iteration. While modifying this line, I also improve the names to make the code more clear, this can be separated to another patch but I don't think it is needed in this case. Change-Id: I5313d25c7148e4a0362d19fe31d2e78f94c26e39 Signed-off-by: Nir Soffer nsof...@redhat.com --- M vdsm/storage/domainMonitor.py 1 file changed, 2 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/07/29007/1 diff --git a/vdsm/storage/domainMonitor.py b/vdsm/storage/domainMonitor.py index 8a50b4f..76afafa 100644 --- a/vdsm/storage/domainMonitor.py +++ b/vdsm/storage/domainMonitor.py @@ -87,7 +87,8 @@ @property def poolMonitoredDomains(self): -return [k for k, v in self._domains.iteritems() if v.poolDomain] +return [sdUUID for sdUUID, monitor in self._domains.items() +if monitor.poolDomain] def startMonitoring(self, sdUUID, hostId, poolDomain=True): domainThread = self._domains.get(sdUUID) -- To view, visit http://gerrit.ovirt.org/29007 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I5313d25c7148e4a0362d19fe31d2e78f94c26e39 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: domainMonitor: Fix unsafe iteration
oVirt Jenkins CI Server has posted comments on this change. Change subject: domainMonitor: Fix unsafe iteration .. Patch Set 1: Build Successful http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9505/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10289/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10445/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5371/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3529/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/29007 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5313d25c7148e4a0362d19fe31d2e78f94c26e39 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Xavi Francisco xfran...@redhat.com Gerrit-Reviewer: Yoav Kleinberger yklei...@redhat.com Gerrit-Reviewer: automat...@ovirt.org 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]: domainMonitor: Fix unsafe iteration
Nir Soffer has posted comments on this change. Change subject: domainMonitor: Fix unsafe iteration .. Patch Set 1: Verified+1 Verified by activating and deactivating a host with 30 storage domains, operation which uses the modified property. -- To view, visit http://gerrit.ovirt.org/29007 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5313d25c7148e4a0362d19fe31d2e78f94c26e39 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Xavi Francisco xfran...@redhat.com Gerrit-Reviewer: Yoav Kleinberger yklei...@redhat.com Gerrit-Reviewer: automat...@ovirt.org 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]: domainMonitor: Fix unsafe iteration of domain monitor status
Nir Soffer has uploaded a new change for review.
Change subject: domainMonitor: Fix unsafe iteration of domain monitor status
..
domainMonitor: Fix unsafe iteration of domain monitor status
When we generate repoStat response, we used to iterate over the
monitored domains, getting the status for each monitor. This naïve
approach does not consider that a monitor may be removed from the domain
monitor dict during the iteration.
Since the only usage now is getting the status for all domains, I
changed getStatus to return an iterator returning all domains status in
a safe manner.
Change-Id: I083c4b1925ee529c0993a291cad6076539d4ea66
Signed-off-by: Nir Soffer nsof...@redhat.com
---
M vdsm/storage/domainMonitor.py
M vdsm/storage/hsm.py
2 files changed, 4 insertions(+), 5 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/09/29009/1
diff --git a/vdsm/storage/domainMonitor.py b/vdsm/storage/domainMonitor.py
index 8a50b4f..c8207d9 100644
--- a/vdsm/storage/domainMonitor.py
+++ b/vdsm/storage/domainMonitor.py
@@ -118,8 +118,9 @@
del self._domains[sdUUID]
-def getStatus(self, sdUUID):
-return self._domains[sdUUID].getStatus()
+def getMonitoredDomainsStatus(self):
+for sdUUID, monitor in self._domains.items():
+yield sdUUID, monitor.getStatus()
def close(self):
self.log.info(Stopping domain monitors)
diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index cc2d8a0..f0c3afb 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -3538,9 +3538,7 @@
repoStats = {}
statsGenTime = time.time()
-for sdUUID in domainMonitor.monitoredDomains:
-domStatus = domainMonitor.getStatus(sdUUID)
-
+for sdUUID, domStatus in domainMonitor.getMonitoredDomainsStatus():
if domStatus.error is None:
code = 0
elif isinstance(domStatus.error, se.StorageException):
--
To view, visit http://gerrit.ovirt.org/29009
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I083c4b1925ee529c0993a291cad6076539d4ea66
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer nsof...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: domainMonitor: Fix unsafe iteration of domain monitor status
Nir Soffer has posted comments on this change. Change subject: domainMonitor: Fix unsafe iteration of domain monitor status .. Patch Set 1: Verified+1 Verified by running a host with 30 storage domains, repoStats still works. -- To view, visit http://gerrit.ovirt.org/29009 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I083c4b1925ee529c0993a291cad6076539d4ea66 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Xavi Francisco xfran...@redhat.com Gerrit-Reviewer: Yoav Kleinberger yklei...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: domainMonitor: Fix unsafe iteration of domain monitor status
oVirt Jenkins CI Server has posted comments on this change. Change subject: domainMonitor: Fix unsafe iteration of domain monitor status .. Patch Set 1: Build Successful http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9506/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10290/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10446/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5372/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3530/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/29009 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I083c4b1925ee529c0993a291cad6076539d4ea66 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Xavi Francisco xfran...@redhat.com Gerrit-Reviewer: Yoav Kleinberger yklei...@redhat.com Gerrit-Reviewer: automat...@ovirt.org 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
