Change in vdsm[master]: Move multipath configuration to vdsm-tool configurator

nsoffer at redhat.com nsoffer at redhat.com
Sun Sep 21 13:45:31 UTC 2014


Nir Soffer has posted comments on this change.

Change subject: Move multipath configuration to vdsm-tool configurator
......................................................................


Patch Set 9:

(4 comments)

http://gerrit.ovirt.org/#/c/30909/9/lib/vdsm/tool/configurators/multipath.py
File lib/vdsm/tool/configurators/multipath.py:

Line 62:         "}\n"
Line 63:         "}"
Line 64:     )
Line 65: 
Line 66:     _MAX_CONF_COPIES = 5
> MAX_OLD_CONF_FILES_COPY
Lets not change this in this patch. I don't understand why this is better anyway.
Line 67: 
Line 68:     # conf file configured by vdsm should contain a tag
Line 69:     # in form of "RHEV REVISION X.Y"
Line 70:     _OLD_TAGS = ["# RHAT REVISION 0.2", "# RHEV REVISION 0.3",


Line 96:         '''
Line 97:         If multipathd is up, it will be reloaded after configuration,
Line 98:         or started before vdsm starts, so service should not be stopped
Line 99:         during configuration.
Line 100:         '''
> but if vdsm is up and we update the multipath version only, than we run vds
We do not restart multipathd, only reload it. If it is not running, we do nothing, as it will run when vdsm starts because vdsm requires multipathd (systemd) or starts it (sysv).
Line 101:         return []
Line 102: 
Line 103:     def configure(self):
Line 104:         """


Line 125:         if utils.isOvirtNode():
Line 126:             utils.persistFile(self._MPATH_CONF)
Line 127: 
Line 128:         # Flush all unused multipath device maps
Line 129:         utils.execCmd([constants.EXT_MULTIPATH, "-F"])
> don't you need to check the rc of this command?
We need, but this is how this code worked for years. Lets keep it in this patch, and check later if we should ignore the return value here or not.

Remember that this patch is trying to move the same logic from vdsm to vdsm-tool, not fix the world. Lets focus on this goal.
Line 130: 
Line 131:         rc = service.service_reload("multipathd")
Line 132:         if rc != 0:
Line 133:             status = service.service_status("multipathd", False)


Line 169: 
Line 170:             for tag in self._OLD_TAGS:
Line 171:                 if tag in first:
Line 172:                     sys.stdout.write("Downrev multipath.conf detected, "
Line 173:                                      "upgrade required\n")
> Downrev? do you mean: old conf file is detected, please run "yum upgrade mu
This means that multipath.conf was configured by old vdsm version, and we need to run vdsm-tool configure to update multipath.conf.

I agree that this is not clear, but this is not related to Yeela changes. Lets get this merged first and then improve the texts.
Line 174:                     return NOT_CONFIGURED
Line 175: 
Line 176:         sys.stdout.write("multipath requires configuration\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: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list