Change in vdsm[master]: storage: fix spec normalization when mounting

nsoffer at redhat.com nsoffer at redhat.com
Sun Apr 3 10:56:07 UTC 2016


Nir Soffer has posted comments on this change.

Change subject: storage: fix spec normalization when mounting
......................................................................


Patch Set 7:

(6 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:

    from vdsm.common.network import address


Line 30: from StringIO import StringIO
Line 31: from contextlib import closing
Line 32: import ctypes
Line 33: from contextlib import contextmanager
Line 34: from vdsm.common.network import address as ipaddress
This import should be near the other vdsm imports. Also do not call this ipaddress, it has nothing to do with ip address.
Line 35: import subprocess
Line 36: import shutil
Line 37: import logging
Line 38: import errno


https://gerrit.ovirt.org/#/c/55182/7/vdsm/storage/mount.py
File vdsm/storage/mount.py:

Line 51:      fs_freq, fs_passno) = line.split()[:6]
Line 52:     fs_mntops = fs_mntops.split(",")
Line 53:     fs_freq = int(fs_freq)
Line 54:     fs_passno = int(fs_passno)
Line 55:     fs_spec = fileUtils.normalize_spec(_parseFstabPath(fs_spec))
This looks good since this is one of the places where spec enter the system. 

We need a test failing without this change.
Line 56: 
Line 57:     fs_file = normpath(_parseFstabPath(fs_file))
Line 58:     for suffix in (" (deleted)", ):
Line 59:         if not fs_file.endswith(suffix):


Line 194: 
Line 195: 
Line 196: class Mount(object):
Line 197:     def __init__(self, fs_spec, fs_file):
Line 198:         self.fs_spec = fileUtils.normalize_spec(fs_spec)
We normalize the spec in storageServer.MountConnection, so this normalization should not needed.

We also need a failing test that this change fix.
Line 199:         self.fs_file = normpath(fs_file)
Line 200: 
Line 201:     def __eq__(self, other):
Line 202:         return (self.__class__ == other.__class__ and


https://gerrit.ovirt.org/#/c/55182/7/vdsm/storage/nfsSD.py
File vdsm/storage/nfsSD.py:

Line 72:         if not misc.isAscii(domainName) and not sd.supportsUnicode(version):
Line 73:             raise se.UnicodeArgumentException()
Line 74: 
Line 75:         # Create local path
Line 76:         mntPath = fileUtils.transformSpec(remotePath)
The spec should be normalized when entering the system, not in this layer. Find how this value got here unnormalized.

Having to do this in nfs storage domain is strange, I would expect that this is needed for all file based storage domains.
Line 77: 
Line 78:         mntPoint = cls.getMountPoint(mntPath)
Line 79: 
Line 80:         cls._preCreateValidation(sdUUID, mntPoint, remotePath, storageType,


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_spec(spec)
This looks good, but may be the wrong place for normalization, since we know that nfsSD is using unnormalized remotePath.

The normalization may be needed in higher layer like hsm.py.
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: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Idan Shaby <ishaby at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland at redhat.com>
Gerrit-Reviewer: Idan Shaby <ishaby at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Tal Nisan <tnisan at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list