Alon Bar-Lev has posted comments on this change. Change subject: packaging: setup: iso domain nfs export fixes ......................................................................
Patch Set 4: (6 comments) better! I beginning to understand. although I think we can simplify this farther. http://gerrit.ovirt.org/#/c/23975/4/packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/system/exportfs.py File packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/system/exportfs.py: Line 66: Line 67: def _nfs_export_path( Line 68: self, Line 69: conf, Line 70: path, I still unsure why you insist of adding path/acl etc... you can just have the line, split and get the path out of it. Line 71: acl=None, Line 72: old_line=None, # Used if acl is None Line 73: overwrite=False Line 74: ): Line 68: self, Line 69: conf, Line 70: path, Line 71: acl=None, Line 72: old_line=None, # Used if acl is None if you had to comment, probably something wrong. Line 73: overwrite=False Line 74: ): Line 75: uninstall_files = [] Line 76: exports_uninstall_group = self.environment[ Line 90: path=path, Line 91: acl=acl, Line 92: ) Line 93: ) Line 94: if overwrite: why overwrite? if we find our path, we replace with ours... it can be same algorithm in both cases. the only 'opened issue' is that header... which can be provided as parameter as well. Line 95: # overwrite unconditionally with a suitable comment Line 96: content = [ Line 97: '# This file is automatically generated by engine-setup.', Line 98: '# Please do not edit manually.', Line 105: if index is not None: Line 106: changes['removed'] = content[index] Line 107: content[index] = new_line Line 108: else: Line 109: content.append(new_line) I do not understand, we do not want to restore the line if moved/overwritten. Line 110: exports_uninstall_group.addChanges( Line 111: group='exportfs', Line 112: filename=conf, Line 113: changes=[changes], Line 119: modifiedList=uninstall_files, Line 120: ) Line 121: ) Line 122: Line 123: def _nfs_unexport_path_no_uninstall(self, conf, path): not sure why we need this, as I just wrote above, I do not see any reason to store the removal at uninstall anyway. Line 124: # The transaction below does not pass modifiedList Line 125: # nor do we call addChanges - we do not revert this Line 126: # fix on cleanup. Line 127: content, index = self._read_and_find_path(conf, path) Line 208: Line 209: if self._move: Line 210: self._nfs_unexport_path_no_uninstall( Line 211: conf=osetupcons.FileLocations.NFS_EXPORT_FILE, Line 212: path=path comma Line 213: ) Line 214: Line 215: if self._generate or self._move: Line 216: self._nfs_export_path( -- To view, visit http://gerrit.ovirt.org/23975 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia12c50b30c5df9f0da446ddbbf54c7ca73b7dd3d Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yedidyah Bar David <d...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Yedidyah Bar David <d...@redhat.com> Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
