Change in vdsm[master]: remove irs_enable configuration attribute
Yaniv Bronhaim has posted comments on this change. Change subject: remove irs_enable configuration attribute .. Patch Set 3: (1 comment) http://gerrit.ovirt.org/#/c/21141/3/lib/vdsm/config.py.in File lib/vdsm/config.py.in: Line 183 Line 184 Line 185 Line 186 Line 187 I'd add at least description why we need it, and maybe example for dan's usage. because otherwise its hard understand why we need this config -- To view, visit http://gerrit.ovirt.org/21141 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7520f0691d10c7d79377724c1aadcae0173e5bae Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Barak Azulay Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Yaniv Bronhaim 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]: remove irs_enable configuration attribute
Arik Hadas has abandoned this change. Change subject: remove irs_enable configuration attribute .. Abandoned -- To view, visit http://gerrit.ovirt.org/21141 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I7520f0691d10c7d79377724c1aadcae0173e5bae Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Barak Azulay Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Yaniv Bronhaim 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]: remove irs_enable configuration attribute
Itamar Heim has posted comments on this change. Change subject: remove irs_enable configuration attribute .. Patch Set 3: ping - still relevant? -- To view, visit http://gerrit.ovirt.org/21141 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7520f0691d10c7d79377724c1aadcae0173e5bae Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Barak Azulay Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Yaniv Bronhaim 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]: remove irs_enable configuration attribute
Dan Kenigsberg has posted comments on this change. Change subject: remove irs_enable configuration attribute .. Patch Set 3: Code-Review-1 If this patch was a part of a patch series about modularization, i'd take it. But as it stands, it removes a configurable that is still useful. I enjoy setting it to false and testing getCaps or networking verbs - without Engine of course. -- To view, visit http://gerrit.ovirt.org/21141 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7520f0691d10c7d79377724c1aadcae0173e5bae Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Barak Azulay Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Michal Skrivanek 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]: remove irs_enable configuration attribute
Arik Hadas has posted comments on this change. Change subject: remove irs_enable configuration attribute .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.ovirt.org/21141 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7520f0691d10c7d79377724c1aadcae0173e5bae Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Barak Azulay Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Michal Skrivanek 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]: remove irs_enable configuration attribute
Eduardo has posted comments on this change. Change subject: remove irs_enable configuration attribute .. Patch Set 3: Code-Review+1 Dan we already agreed on the need of modularize vdsm. This configuration variable is not related to that, is only an echo the past (2008) which not delivers the modularization you want and is misleading. In addition is not supported by the engine. About actual use cases for it I think that nobody has seen such thing . -- To view, visit http://gerrit.ovirt.org/21141 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7520f0691d10c7d79377724c1aadcae0173e5bae Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Barak Azulay Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Michal Skrivanek 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]: remove irs_enable configuration attribute
Arik Hadas has posted comments on this change. Change subject: remove irs_enable configuration attribute .. Patch Set 2: (1 comment) Commit Message Line 5: CommitDate: 2013-11-11 10:11:05 -0500 Line 6: Line 7: remove irs_enable configuration attribute Line 8: Line 9: The configuration of vdsm included an attribute called 'irs_enable' ok, maybe the comment above is incorrect since I'm not familiar with the fencing flow with proxy and don't understand how the host should be configured in the system to use the networking-only capabilities.. Anyway, in those cases, shouldn't vdsm report to the engine that the host is configured in such a way so that it won't be selected as SPM and the engine will never try to run VMs on it? Line 10: which was supposed to make it possible to set a specific host as non Line 11: eligible to serve as SPM. Line 12: Line 13: This configuration is not supported (the engine will just try to -- To view, visit http://gerrit.ovirt.org/21141 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7520f0691d10c7d79377724c1aadcae0173e5bae Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Barak Azulay Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Michal Skrivanek 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]: remove irs_enable configuration attribute
Arik Hadas has posted comments on this change. Change subject: remove irs_enable configuration attribute .. Patch Set 2: (3 comments) Commit Message Line 5: CommitDate: 2013-11-11 10:11:05 -0500 Line 6: Line 7: remove irs_enable configuration attribute Line 8: Line 9: The configuration of vdsm included an attribute called 'irs_enable' I changed the commit message, sticking more to the facts of how it is today. I came across this attribute accidentally, and I realized that setting it to false might conflict with a change I already made in the engine: http://gerrit.ovirt.org/#/c/17815/ (the engine tries to fetch the iso prefix from the host the VM is about to run on if it's not already defined, but if that host is set with irs_enable=false then we'll have a problem) So maybe there's such a use-case where it can be useful to set irs_enable=false but currently it will be problematic for the flow described above and when the engine tries to set such host as SPM. so as long as it is not supported well in the system I think we should drop it and maybe add it in a proper way once we'll find that it might be useful. Line 10: which was supposed to make it possible to set a specific host as non Line 11: eligible to serve as SPM. Line 12: Line 13: This configuration is not supported (the engine will just try to File vdsm/clientIF.py Line 247: while self._enabled: Line 248: time.sleep(3) Line 249: Line 250: def _initIRS(self): Line 251: self.irs = None right, Done Line 252: try: Line 253: self.irs = Dispatcher(HSM()) Line 254: except: Line 255: self.log.error("Error initializing IRS", exc_info=True) Line 250: def _initIRS(self): Line 251: self.irs = None Line 252: try: Line 253: self.irs = Dispatcher(HSM()) Line 254: except: I'll need to detect all the possible exceptions which can be thrown when calling HSM constructor and Dispatcher method. it is not too complex but it is not trivial either and I think it is not really related to this change, so let's not couple it with this patch. Line 255: self.log.error("Error initializing IRS", exc_info=True) Line 256: else: Line 257: self.irs.registerDomainStateChangeCallback(self.contEIOVms) Line 258: -- To view, visit http://gerrit.ovirt.org/21141 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7520f0691d10c7d79377724c1aadcae0173e5bae Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Barak Azulay Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Michal Skrivanek 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]: remove irs_enable configuration attribute
oVirt Jenkins CI Server has posted comments on this change. Change subject: remove irs_enable configuration attribute .. Patch Set 3: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4548/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5348/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5427/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/21141 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7520f0691d10c7d79377724c1aadcae0173e5bae Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas Gerrit-Reviewer: Barak Azulay Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Michal Skrivanek 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]: remove irs_enable configuration attribute
Dan Kenigsberg has posted comments on this change. Change subject: remove irs_enable configuration attribute .. Patch Set 2: Code-Review-1 (1 comment) Commit Message Line 5: CommitDate: 2013-11-11 10:11:05 -0500 Line 6: Line 7: remove irs_enable configuration attribute Line 8: Line 9: The configuration of vdsm included an attribute called 'irs_enable' the commit message is not historically exact, as irs_enable existed before the SPM concept. I love to delete dead code, but in this case, there may be a use case for running Vdsm without the storage subsystem (e.g. someone wants to use only its networking or fencing capabilities) Line 10: which was supposed to make it possible to set a specific host as non Line 11: eligible to serve as SPM. Line 12: Line 13: This configuration is not supported (the engine will just try to -- To view, visit http://gerrit.ovirt.org/21141 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7520f0691d10c7d79377724c1aadcae0173e5bae Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas Gerrit-Reviewer: Barak Azulay Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Michal Skrivanek Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: remove irs_enable configuration attribute
Eduardo has posted comments on this change. Change subject: remove irs_enable configuration attribute .. Patch Set 2: Code-Review+1 (2 comments) File vdsm/clientIF.py Line 247: while self._enabled: Line 248: time.sleep(3) Line 249: Line 250: def _initIRS(self): Line 251: self.irs = None Can this be removed? If not, it should be under the except clause. Line 252: try: Line 253: self.irs = Dispatcher(HSM()) Line 254: except: Line 255: self.log.error("Error initializing IRS", exc_info=True) Line 250: def _initIRS(self): Line 251: self.irs = None Line 252: try: Line 253: self.irs = Dispatcher(HSM()) Line 254: except: I would like a narrow except only because you are touching it, but... Line 255: self.log.error("Error initializing IRS", exc_info=True) Line 256: else: Line 257: self.irs.registerDomainStateChangeCallback(self.contEIOVms) Line 258: -- To view, visit http://gerrit.ovirt.org/21141 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7520f0691d10c7d79377724c1aadcae0173e5bae Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas Gerrit-Reviewer: Barak Azulay Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Michal Skrivanek 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]: remove irs_enable configuration attribute
oVirt Jenkins CI Server has posted comments on this change. Change subject: remove irs_enable configuration attribute .. Patch Set 2: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4530/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5330/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5408/ : FAILURE -- To view, visit http://gerrit.ovirt.org/21141 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7520f0691d10c7d79377724c1aadcae0173e5bae Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas 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]: remove irs_enable configuration attribute
oVirt Jenkins CI Server has posted comments on this change. Change subject: remove irs_enable configuration attribute .. Patch Set 1: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4529/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5329/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5407/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/21141 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7520f0691d10c7d79377724c1aadcae0173e5bae Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas 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]: Remove irs_enable configuration attribute
Arik Hadas has uploaded a new change for review. Change subject: Remove irs_enable configuration attribute .. Remove irs_enable configuration attribute The configuration of vdsm included an attribute called 'irs_enable' which was supposed to make it possible to set a specific host as non eligible to serve as SPM. This configuration is not supported (the engine will just try to initialize such host as SPM over and over again). Thus, this configuration attribute is removed. Change-Id: I7520f0691d10c7d79377724c1aadcae0173e5bae Signed-off-by: Arik Hadas --- M lib/vdsm/config.py.in M vdsm/clientIF.py 2 files changed, 6 insertions(+), 9 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/41/21141/1 diff --git a/lib/vdsm/config.py.in b/lib/vdsm/config.py.in index c0bdd53..391d771 100644 --- a/lib/vdsm/config.py.in +++ b/lib/vdsm/config.py.in @@ -183,8 +183,6 @@ # Section: [irs] ('irs', [ -('irs_enable', 'true', None), - ('repository', '@VDSMREPO@', 'Image repository.'), diff --git a/vdsm/clientIF.py b/vdsm/clientIF.py index 0b67e45..e696b01 100644 --- a/vdsm/clientIF.py +++ b/vdsm/clientIF.py @@ -249,13 +249,12 @@ def _initIRS(self): self.irs = None -if config.getboolean('irs', 'irs_enable'): -try: -self.irs = Dispatcher(HSM()) -except: -self.log.error("Error initializing IRS", exc_info=True) -else: -self.irs.registerDomainStateChangeCallback(self.contEIOVms) +try: +self.irs = Dispatcher(HSM()) +except: +self.log.error("Error initializing IRS", exc_info=True) +else: +self.irs.registerDomainStateChangeCallback(self.contEIOVms) def _getUUIDSpecPath(self, uuid): try: -- To view, visit http://gerrit.ovirt.org/21141 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I7520f0691d10c7d79377724c1aadcae0173e5bae Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches