Nir Soffer has posted comments on this change. Change subject: storage: fix spec normalization when mounting ......................................................................
Patch Set 9: (7 comments) https://gerrit.ovirt.org/#/c/55182/9/lib/vdsm/storage/fileUtils.py File lib/vdsm/storage/fileUtils.py: Line 89: def transform_remote_path(remote_path): Line 90: """ Line 91: Transforms an unnormalized remote path to a new one for local mount Line 92: """ Line 93: return transformPath(normalize_remote_path(remote_path)) Without this method the code will be more consistent and clear - storageServer, nfsSD and hsm will use both normalize_remove_path and transformPath. Line 94: Line 95: Line 96: def normalize_remote_path(remote_path): Line 97: if ":" in remote_path: Line 92: """ Line 93: return transformPath(normalize_remote_path(remote_path)) Line 94: Line 95: Line 96: def normalize_remote_path(remote_path): Lets call it normalize_path since it supports both remote and local path. Add docstrings explaining what this does and mention handling both remote and local path. Line 97: if ":" in remote_path: Line 98: host, path = address.hosttail_split(remote_path) Line 99: return address.hosttail_join(host, normpath(path)) Line 100: return normpath(remote_path) https://gerrit.ovirt.org/#/c/55182/9/lib/vdsm/storage/mount.py File lib/vdsm/storage/mount.py: Line 181 Line 182 Line 183 Line 184 Line 185 This normpath is also not needed, the caller of this code should normalize paths if needed. Line 58: fs_freq, fs_passno) = line.split()[:6] Line 59: fs_mntops = fs_mntops.split(",") Line 60: fs_freq = int(fs_freq) Line 61: fs_passno = int(fs_passno) Line 62: fs_spec = fileUtils.normalize_remote_path(_parseFstabPath(fs_spec)) We don't need normalzation here, since we mounted the paths using normalized paths, and we don't expect the kernel to play with the paths. Both fs_spec and fs_file. Line 63: Line 64: fs_file = normpath(_parseFstabPath(fs_file)) Line 65: for suffix in (" (deleted)", ): Line 66: if not fs_file.endswith(suffix): Line 204: Line 205: log = logging.getLogger("storage.Mount") Line 206: Line 207: def __init__(self, fs_spec, fs_file): Line 208: self.fs_spec = fileUtils.normalize_remote_path(fs_spec) The caller should normalize paths if needed. We must normalize fs_spec for creating fs_file as done in storageServer.MountConnection, so it does not make sense to normalize twice. (both fs_file and fs_spec) Line 209: self.fs_file = normpath(fs_file) Line 210: Line 211: def __eq__(self, other): Line 212: return (self.__class__ == other.__class__ and https://gerrit.ovirt.org/#/c/55182/9/tests/fileUtilTests.py File tests/fileUtilTests.py: Line 279: self.assertRaises(OSError, fileUtils.atomic_symlink, target, link) Line 280: Line 281: Line 282: @expandPermutations Line 283: class TestTransformRemotePath(TestCaseBase): If we remove tranform_remote_path, we don't need the tests. Line 284: Line 285: @permutations([ Line 286: ("server:/path", "server:_path"), Line 287: ("server:/path/", "server:_path"), https://gerrit.ovirt.org/#/c/55182/9/vdsm/storage/storageServer.py File vdsm/storage/storageServer.py: Line 211 Line 212 Line 213 Line 214 Line 215 We need to send the normalized spec to mount. -- To view, visit https://gerrit.ovirt.org/55182 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9f244eb903fc049c726efba69f37b3b5fb01b561 Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Idan Shaby <ish...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Freddy Rolland <froll...@redhat.com> Gerrit-Reviewer: Idan Shaby <ish...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
