ENH: Disable symlinks on CIFS filesystems

[effigies]

There appear to be cases where hacks to make symlinks work on filesystems that don't directly support them can cause FileNotFoundErrors. In particular, the CIFS network filesystem driver for Linux supports the mfsymlinks option, which enables symlinks at the VFS level, having the effect that symlinking can be subject to network timeouts or temporary inconsistencies.

CIFS+mfsymlinks is the Docker-supported way to mount filesystems into a container from a Windows host, which means this may be increasingly a problem for Nipype users, as containers are becoming a more popular environment for running workflows. An error reported on nipreps/fmriprep#398 ran into this issue when running an interface that requires 4 symlinks, which split into two mapflow directories (per dataset being analyzed), using the MultiProc plugin.

This PR permits users to disable symlinking in the config file with the disable_symbolic_links option.

Closes #1938.

[codecov-io]

Codecov Report

Merging #1941 into master will increase coverage by 0.01%.
The diff coverage is 93.33%.

@@            Coverage Diff            @@
##           master   #1941      +/-   ##
=========================================
+ Coverage   72.49%   72.5%   +0.01%     
=========================================
  Files        1063    1063              
  Lines       54159   54189      +30     
  Branches     7811    7820       +9     
=========================================
+ Hits        39260   39288      +28     
- Misses      13680   13682       +2     
  Partials     1219    1219

Flag	Coverage Δ
#smoketests	72.5% <93.33%> (+0.01%)	⬆️
#unittests	70.04% <90%> (+0.01%)	⬆️

Impacted Files	Coverage Δ
nipype/utils/tests/test_filemanip.py	93.61% <100%> (+0.4%)	⬆️
nipype/utils/filemanip.py	84.89% <87.5%> (+0.14%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3107a6d...ae5c6e1. Read the comment docs.

[chrisgorgo]

Determining the filesystem might be easier than we thought http://stackoverflow.com/a/25287521/616300

[effigies]

That requires psutil, but if it seems worth it to add the dependency, we can check for CIFS. Could you install psutil on your docker container and get a working is_cifs(path) function?

[chrisgorgo]

Does not require psutil.

def is_cifs(path):
     exit_code, mount_str = subprocess.getstatusoutput("mount")
     if not exit_code:
         mount_points = [(line.split(" ")[2], line.split(" ")[4]) for line in mount_str.split("\n")]
         sorted_mount_points = sorted(mount_points, key=lambda x: len(x[0]), reverse=True)
         for k, v in sorted_mount_points:
             print(k,v)
             if path.startswith(k):
                 if v == "cifs":
                     return True
                 else:
                     return False
     else:
         return False

[chrisgorgo]

I also run the workflow with working directory inside the container (not mounted) and I did not get an error.

This means we should avoid symlinks only if the destination is on cifs.

[effigies]

@chrisfilo I refactored your test a little. Want to grab 0d5359d from me and verify that this works?

from nipype.utils.filemanip import _on_cifs
assert (_on_cifs('/'), _on_cifs('/scratch')) == (False, True)

I think creating a CIFS mount in the docker image for testing might be excessive.

[chrisgorgo]

Works as advertised! Thanks for cleaning up my napkin quality code!

[chrisgorgo]

I would make it public - could be useful in other places

[chrisgorgo]

For performance reasons, I would only run this check if we are about to make a symlink.

[effigies]

The performance issue is a good point. If this function's going to get called a lot, it might make sense to do something a little more persistent. We could do something like:

• At program start (or first call), call mount, and get (mountpoint, fstype) tuples.
• Prune all but superstrings of CIFS mounts
• Check target files against pruned list

[effigies]

Latest commit may be breaking some style guidelines. Let me know if I should reorganize.

@chrisfilo could you verify this still works? Updated test:

from nipype.utils.filemanip import on_cifs
assert (on_cifs('/'), on_cifs('/scratch')) == (False, True)

[chrisgorgo]

I'm seeing huge performance hit when using MultiProc. It seems that mount is called each time a Node is being run (or checked in it needs to be rerun) - even if no files are being copied.

[effigies]

On the most recent commit? The latest changes should make the mount call occur only once per run. Unless I misunderstand how things are being run...

[chrisgorgo]

Yeah I pulled to get the latest commit. I can double check, but I see the slowdown even for checking precomputed inputs so I think I go the right commit.

…

On Apr 7, 2017 3:15 PM, "Chris Markiewicz" ***@***.***> wrote: On the most recent commit? The latest changes should make the mount call occur only once per run. Unless I misunderstand how things are being run... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1941 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAOkp6POcFzPB-r2SU4cXoTm9lDO3Jf5ks5rtrWbgaJpZM4M01Sq> .

[chrisgorgo]

I'm doing more debugging. It seems that rechecking EPIToT1Transform (nothing gets rewritten) takes longer than usual (it's really slow) however the mri_expand is happening in the background so I don't know if the total running time will differ.

But you are right - the table is generated only once. I'll do more debugging.

[effigies]

Re: EPIToT1, have you recently switched to content hashing, or using an EPI with more volumes? Because I've found that's always been slow when I used content hashing...

[effigies]

Unable to replicate the slowdown on mine.

[chrisgorgo]

I think the slowdown is unrelated to the change in this PR. Everything looks good to me, but since this is a change at the core of nipype it would be nice to have @satra hava a look at it as well.

[effigies]

Do you have remove_unnecessary_outputs disabled? If not, that might be your slowdown.

[afni-dglen]

John Rogers-Lee and I spent some time looking at the cifs parsing because it ended up blocking the installation on my local Mac. I'll create a pull request with my suggested changes in filemanip.py.