Idan Shaby has posted comments on this change. Change subject: storage: fix spec normalization when mounting ......................................................................
Patch Set 9: (5 comments) https://gerrit.ovirt.org/#/c/55182/7/vdsm/storage/fileUtils.py File vdsm/storage/fileUtils.py: Line 38 Line 39 Line 40 Line 41 Line 42 > Put it here: Done Line 30 Line 31 Line 32 Line 33 Line 34 > This import should be near the other vdsm imports. Also do not call this ip Done https://gerrit.ovirt.org/#/c/55182/7/vdsm/storage/mount.py File vdsm/storage/mount.py: Line 51 Line 52 Line 53 Line 54 Line 55 > This looks good since this is one of the places where spec enter the system I added a test in mountTests (MountTests.test_is_mounted). You can't see that it's failing since the fix and the test are in the same patch (like they should be). The test cannot be added (and fail) before this patch since it will actually work (you write "server:" instead of "server:/" due to a wrong normalization, read it from proc/mounts and the wrong normalization there doesn't change anything). If you like, I can move the test to the previous patch and we will see that this patch doesn't break it. Line 194 Line 195 Line 196 Line 197 Line 198 > We normalize the spec in storageServer.MountConnection, so this normalizati Already explained to you f2f why it should be here. https://gerrit.ovirt.org/#/c/55182/7/vdsm/storage/storageServer.py File vdsm/storage/storageServer.py: Line 209: def __init__(self, spec, vfsType=None, options="", mountClass=mount.Mount): Line 210: self._vfsType = vfsType Line 211: # Note: must be normalized before we escape "/" in _getLocalPath. Line 212: # See https://bugzilla.redhat.com/1300749 Line 213: self._remotePath = fileUtils.normalize_remote_path(spec) > This looks good, but may be the wrong place for normalization, since we kno Already explained to you f2f why it should be here. Line 214: self._options = options Line 215: self._mount = mountClass(spec, self._getLocalPath()) Line 216: Line 217: def _getLocalPath(self): -- 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
