Sandro Bonazzola has uploaded a new change for review. Change subject: storageServer: Normalize local path before escaping ......................................................................
storageServer: Normalize local path before escaping Hosted engine setup does not normalize mount path, so hosted engine storage domain with trailing slash: /foo/ will be mounted at: /rhev/data-center/mnt/_foo_ This is harmless until hosted engine try to import the hosted engine storage domain, recreating the storage domain path from the storage domain information, returning a normalized remote path. After importing the domain, the host will have the same remote path mounted twice at two different local paths: /rhev/data-center/mnt/_foo /rhev/data-center/mnt/_foo_ Mounting the same domain twice does not work, leading to failure. Storage domains created in engine do not have this issue since engine does not allow path with trailing slash. We are normalizing mount remote and local path, both before mount, and when reading mount information from /proc/mounts and /etc/mtab. However, we were escaping slashes in the normal path *before* normalizing it, so the normalization had no effect. This patch normalize the local path before escaping slashes, to match the intent of the original code, and avoid such issues in the future. This change does not fix hosted engine system with unnormalized local path. The only way to fix this is to disconnect the storage domain and connect it again with a normalized path. Change-Id: I4db1fb7dfe4627442f87502563a578bb1f184c1e Bug-Url: https://bugzilla.redhat.com/1300749 Signed-off-by: Nir Soffer <nsof...@redhat.com> Reviewed-on: https://gerrit.ovirt.org/52873 Continuous-Integration: Jenkins CI Reviewed-by: Roy Golan <rgo...@redhat.com> Reviewed-by: Martin Sivák <msi...@redhat.com> Reviewed-by: Simone Tiraboschi <stira...@redhat.com> Reviewed-by: Sandro Bonazzola <sbona...@redhat.com> Reviewed-by: Dan Kenigsberg <dan...@redhat.com> (cherry picked from commit b1b60ec663a8d599f3f889421bcc8bdd04030d9e) --- M tests/storageServerTests.py M vdsm/storage/storageServer.py 2 files changed, 16 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/76/53276/1 diff --git a/tests/storageServerTests.py b/tests/storageServerTests.py index 3ad5230..2e9b4a6 100644 --- a/tests/storageServerTests.py +++ b/tests/storageServerTests.py @@ -57,6 +57,7 @@ self.assertEqual(str(errors), expected) +@expandPermutations class MountConnectionTests(VdsmTestCase): def test_mountpoint(self): @@ -64,6 +65,18 @@ self.assertEquals(mount_con._mount.fs_spec, "dummy-spec") self.assertEquals(mount_con._mount.fs_file, "/tmp/dummy-spec") + @permutations([ + # spec, localpath + ("/a/", "/tmp/_a"), + ("/a//", "/tmp/_a"), + ("/a/b", "/tmp/_a_b"), + ("/a//b", "/tmp/_a_b"), + ("/a/b_c", "/tmp/_a_b__c"), + ]) + def test_normalize_local_path(self, spec, localpath): + con = MountConnection(spec, mountClass=FakeMount) + self.assertEqual(con._mount.fs_file, localpath) + @expandPermutations class TestMountConnectionEquality(VdsmTestCase): diff --git a/vdsm/storage/storageServer.py b/vdsm/storage/storageServer.py index 25f0d46..a805f4e 100644 --- a/vdsm/storage/storageServer.py +++ b/vdsm/storage/storageServer.py @@ -206,7 +206,9 @@ def __init__(self, spec, vfsType=None, options="", mountClass=mount.Mount): self._vfsType = vfsType - self._remotePath = spec + # Note: must be normalized before we escape "/" in _getLocalPath. + # See https://bugzilla.redhat.com/1300749 + self._remotePath = normpath(spec) self._options = options self._mount = mountClass(spec, self._getLocalPath()) -- To view, visit https://gerrit.ovirt.org/53276 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4db1fb7dfe4627442f87502563a578bb1f184c1e Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.6 Gerrit-Owner: Sandro Bonazzola <sbona...@redhat.com> _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches