Nir Soffer has posted comments on this change. Change subject: Move multipath configuration to vdsm-tool configurator ......................................................................
Patch Set 16: (4 comments) http://gerrit.ovirt.org/#/c/30909/16/lib/vdsm/tool/configurators/multipath.py File lib/vdsm/tool/configurators/multipath.py: Line 73: "# RHEV REVISION 0.4", "# RHEV REVISION 0.5", Line 74: "# RHEV REVISION 0.6", "# RHEV REVISION 0.7", Line 75: "# RHEV REVISION 0.8", "# RHEV REVISION 0.9"] Line 76: Line 77: _MPATH_CONF_TAG = "# RHEV REVISION 1.0" You need to rebase - see http://gerrit.ovirt.org/#/c/35072/3/vdsm/storage/multipath.py Line 78: Line 79: # Having the PRIVATE_TAG in the conf file means Line 80: # vdsm-tool should never change the conf file Line 81: # even when using the --force flag Line 107: supported state. The original configuration, if any, is saved Line 108: """ Line 109: Line 110: if os.path.exists(self._MPATH_CONF): Line 111: backup = MPATH_CONF + '.' + datetime.now().strftime("%Y%m%d%H%M") self._MPATH_CONF? Any reason to use datetime, when the simpler time.strftime() give the same results? >>> datetime.now().strftime("%Y%m%d%H%M") '201411232052' >>> time.strftime("%Y%m%d%H%M") '201411232052' Line 112: try: Line 113: shutil.copyfile(self._MPATH_CONF, backup) Line 114: except IOError: Line 115: raise RuntimeError("Failed to copy conf to backup") Line 111: backup = MPATH_CONF + '.' + datetime.now().strftime("%Y%m%d%H%M") Line 112: try: Line 113: shutil.copyfile(self._MPATH_CONF, backup) Line 114: except IOError: Line 115: raise RuntimeError("Failed to copy conf to backup") You are hiding the original error, which will make it impossible to debug. Since this error is not expected (root cannot copy file in /etc?!), the simplest way to handle it would be to *not* handle it and let the tool fail with clear traceback. Line 116: else: Line 117: utils.persist(backup) Line 118: Line 119: with tempfile.NamedTemporaryFile() as f: Line 162: sys.stdout.write("This manual override for multipath.conf " Line 163: "was based on downrevved template. " Line 164: "You are strongly advised to " Line 165: "contact your support representatives\n") Line 166: return CONFIGURED You need to rebase, the constants are different now (YES, NO, MAYBE) Line 167: Line 168: if self._MPATH_CONF_TAG in first: Line 169: sys.stdout.write("Current revision of multipath.conf detected," Line 170: " preserving\n") -- To view, visit http://gerrit.ovirt.org/30909 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ife045908dc6e2aea9829b51482b909af1faf79da Gerrit-PatchSet: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan <ykap...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@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: Saggi Mizrahi <smizr...@redhat.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: Yeela Kaplan <ykap...@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
