Yaniv Bronhaim has posted comments on this change. Change subject: Move multipath configuration to vdsm-tool configurator ......................................................................
Patch Set 4: Code-Review-1 (3 comments) I'm sorry, i don't understand how the tags work. I know this patch here just to move things around, but I want to do it right and understand the content of the conf file if you do. please also verify that the behavior with "vdsm-tool configure --module multipath" , and with --force flag, is as you expected, because I really not sure about it. and explain with the desired behavior about overriding the conf and all .. the semantic very important. keep in mind that on upgrade and host deploy we call configure with --force for all modules, make sure it won't disturb your logic http://gerrit.ovirt.org/#/c/30909/4/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 668: Line 669: class MultipathModuleConfigure(_ModuleConfigure): Line 670: Line 671: _MPATH_CONF = "/etc/multipath.conf" Line 672: what with my comment :( to me its not redundant . I don't understand why you put 2 empty lines before the conf itself and I prefer to understand. otherwise I might post a patch that removes and waste time without knowing the reason. why not to agree..? its only 2 line for you, and great pleasure for me . please add it.. Line 673: _STRG_MPATH_CONF = ( Line 674: "\n\n" Line 675: "defaults {\n" Line 676: " polling_interval 5\n" Line 701: Line 702: _MAX_CONF_COPIES = 5 Line 703: Line 704: # conf file configured by vdsm should contain private tag Line 705: # in form of "RHEV REVISION X.Y" what is those private tags? if you understand, just explain.. that's all im asking Line 706: _OLD_TAGS = ["# RHAT REVISION 0.2", "# RHEV REVISION 0.3", Line 707: "# RHEV REVISION 0.4", "# RHEV REVISION 0.5", Line 708: "# RHEV REVISION 0.6", "# RHEV REVISION 0.7", Line 709: "# RHEV REVISION 0.8", "# RHEV REVISION 0.9"] Line 710: Line 711: _MPATH_CONF_TAG = "# RHEV REVISION 1.0" Line 712: Line 713: # Having the PRIVATE_TAG in the conf file means Line 714: # we should never change the conf file who is we? vdsm-tool won't touch it ? manually I can't modify it? I really don't understand ... as far as I understand, when putting "# RHEV REVISION 1.0" before the configuration, vdsm-tool configure call will assume that the following conf is valid and won't override it even with --force flag. is that true? Line 715: _MPATH_CONF_PRIVATE_TAG = "# RHEV PRIVATE" Line 716: Line 717: _MPATH_CONF_TEMPLATE = _MPATH_CONF_TAG + _STRG_MPATH_CONF Line 718: -- 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: 4 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