Change in vdsm[master]: storage: Hidden files filtered form listings of storage domains

Jakub Niedermertl has uploaded a new change for review.

Change subject: storage: Hidden files filtered form listings of storage domains
......................................................................

storage: Hidden files filtered form listings of storage domains

Listing of files in storage domain (FileStorageDomain#getFileList) now
returns only non-hidden files (not starting with '.'). It allows to
avoid various auxiliary hidden files like '.keep'. It is especially
useful in context of ISO doman that stores files for various purposes.

Change-Id: I0eebd8b9fe98c41df686330858b3292fa37ba245
Bug-Url: https://bugzilla.redhat.com/1259279
Signed-off-by: Jakub Niedermertl 
---
M vdsm/storage/fileSD.py
M vdsm/storage/hsm.py
2 files changed, 11 insertions(+), 6 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/45/45645/1

diff --git a/vdsm/storage/fileSD.py b/vdsm/storage/fileSD.py
index 5c9aa6d..85140ac 100644
--- a/vdsm/storage/fileSD.py
+++ b/vdsm/storage/fileSD.py
@@ -421,12 +421,15 @@

def getFileList(self, pattern, caseSensitive):
         """
-        Returns a list of all files in the domain filtered according to
-        extension.
+        Returns a list of all visible (not starting with '.') files in the
+        domain filtered according to extension.
         """
         basedir = self.getIsoDomainImagesDir()
         filesList = self.oop.simpleWalk(basedir)

+        nonHiddenFilesRE = '.*/[^.][^/]*$'
+        filesList = [x for x in filesList if re.match(nonHiddenFilesRE, x)]
+
         if pattern != '*':
             if caseSensitive:
                 filesList = fnmatch.filter(filesList, pattern)
diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index 82f4426..7674209 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -2305,8 +2305,8 @@
     def getFileStats(self, sdUUID, pattern='*', caseSensitive=False,
                      options=None):
         """
-        Returns statistics of all files in the domain filtered according to
-        pattern.
+        Returns statistics of all visible (not starting with '.') files in
+        the domain filtered according to pattern.

:param sdUUID: The UUID of the storage domain you want to query.
         :type sdUUID: UUID
@@ -2333,7 +2333,8 @@
     @public
     def getIsoList(self, spUUID, extension='iso', options=None):
         """
-        Gets a list of all ISO/Floppy volumes in a storage pool.
+        Gets a list of all visible (not starting with '.') ISO/Floppy volumes
+        in a storage pool.

:param spUUID: The UUID of the storage pool you want to query.
         :type spUUID: UUID
@@ -2361,7 +2362,8 @@
     @public
     def getFloppyList(self, spUUID, options=None):
         """
-        Gets a list of all Floppy volumes if a storage pool.
+        Gets a list of all visible (not starting with '.') Floppy volumes of
+        a storage pool.

:param spUUID: The UUID of the storage pool you want to query.
         :type spUUID: UUID

[e-mail] has posted comments on this change.

Change subject: storage: Hidden files filtered form listings of storage domains
......................................................................

Patch Set 1:

* Update tracker::#1259279::OK
* Check Bug-Url::OK
* Check Public Bug::#1259279::OK, public bug
* Check Product::#1259279::OK, Correct product oVirt
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

Jakub Niedermertl has posted comments on this change.

Change subject: storage: Hidden files filtered form listings of storage domains
......................................................................

Patch Set 1: Verified+1

[Martin Polednik]

Martin Polednik has posted comments on this change.

Change subject: storage: Hidden files filtered form listings of storage domains
......................................................................

Patch Set 1: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/45645/1/vdsm/storage/fileSD.py
File vdsm/storage/fileSD.py:

Line 427:         basedir = self.getIsoDomainImagesDir()
Line 428:         filesList = self.oop.simpleWalk(basedir)
Line 429: 
Line 430:         nonHiddenFilesRE = '.*/[^.][^/]*$'
Line 431:         filesList = [x for x in filesList if re.match(nonHiddenFilesRE, x)]
If I understand correctly, you can use built-in startswith method.

str.startswith(prefix[, start[, end]])
Line 432: 
Line 433:         if pattern != '*':
Line 434:             if caseSensitive:
Line 435:                 filesList = fnmatch.filter(filesList, pattern)

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: storage: Hidden files filtered form listings of storage domains
......................................................................

Patch Set 1: Code-Review-1

Thanks for the patch, but this is not the right place to filter the dot files. Vdsm is not a user level tool but a helper for engine, and it should not implement such policy. The right place to filter these files is in the engine.

[e-mail] has posted comments on this change.

Change subject: storage: Hidden files filtered form listings of storage domains
......................................................................

Patch Set 2:

* Update tracker::#1259279::OK
* Check Bug-Url::OK
* Check Public Bug::#1259279::OK, public bug
* Check Product::#1259279::OK, Correct product oVirt
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

Jakub Niedermertl has posted comments on this change.

Change subject: storage: Hidden files filtered form listings of storage domains
......................................................................

Patch Set 2: Verified+1

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: storage: Hidden files filtered form listings of storage domains
......................................................................

Patch Set 2: Code-Review-1

See my previous comment.

Jakub Niedermertl has posted comments on this change.

Change subject: storage: Hidden files filtered form listings of storage domains
......................................................................

Patch Set 2:

Hi Nir, please try to reconsider it. I understand that vdsm should provide general services however processing of hidden files would always cause unnecessary overhead in vdsm (reading file stats), network and engine. And it's improbable anyone will ever need them.

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: storage: Hidden files filtered form listings of storage domains
......................................................................

Patch Set 2:

Jakub, the overhead of calling stat() the non-existing dot files and the couple of additional bytes sent over the wire is negligible.

Michal Skrivanek has posted comments on this change.

Change subject: storage: Hidden files filtered form listings of storage domains
......................................................................

Patch Set 2:

actually, I find it quite useless to send data which are never going to be used. Regardless whether the overhead is negligible or not

Not so important though, filtering it out in the backend is also cheap and easy

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: storage: Hidden files filtered form listings of storage domains
......................................................................

Patch Set 2:

Michal, filtering dot files in vdsm will prevent us from using dot files in the future. For example, engine can store metadata in such files. Filtering this in the engine does not create such unneeded limits.

Michal Skrivanek has posted comments on this change.

Change subject: storage: Hidden files filtered form listings of storage domains
......................................................................

Patch Set 2:

getISOList is not supposed to be used for anything like that. The other one perhaps yes, fine then

Jakub Niedermertl has abandoned this change.

Change subject: storage: Hidden files filtered form listings of storage domains
......................................................................

Abandoned

[e-mail] has posted comments on this change.

Change subject: storage: Hidden files filtered form listings of storage domains
......................................................................

Patch Set 2:

* Update tracker::#1259279::OK