Yeela Kaplan 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/m
Done
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?
Done
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.
Done
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)
Done
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

Reply via email to