Change in vdsm[ovirt-3.6]: storageServer: Normalize local path before escaping

sbonazzo at redhat.com sbonazzo at redhat.com
Tue Feb 9 10:27:00 UTC 2016


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 <nsoffer at redhat.com>
Reviewed-on: https://gerrit.ovirt.org/52873
Continuous-Integration: Jenkins CI
Reviewed-by: Roy Golan <rgolan at redhat.com>
Reviewed-by: Martin Sivák <msivak at redhat.com>
Reviewed-by: Simone Tiraboschi <stirabos at redhat.com>
Reviewed-by: Sandro Bonazzola <sbonazzo at redhat.com>
Reviewed-by: Dan Kenigsberg <danken at 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)
 
 
+ at 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 <sbonazzo at redhat.com>


More information about the vdsm-patches mailing list