cfs checks #2428

afni-dglen · 2018-02-07T00:34:20Z

Made the cfs checks a little more robust

Fixes # .

Changes proposed in this pull request

effigies

Thanks for this. A couple comments.

effigies · 2018-02-07T00:50:16Z

nipype/utils/filemanip.py

-    >>> filepath = os.path.dirname(os.path.realpath( __file__ ))
-    >>> datadir = os.path.realpath(os.path.join(filepath, '../testing/data'))
-    >>> os.chdir(datadir)
-


Is there a reason to drop the doctests?

effigies · 2018-02-07T00:54:46Z

nipype/utils/filemanip.py

+                    break
+                crypto_obj.update(data)
+        hex = crypto_obj.hexdigest()
+    return hex


If we're dropping the raise_notfound check, I'd prefer to leave it:

if not op.isfile(afile): return None crypto_obj = crypto() ...

This checks against bad input, and then runs the function as normal, rather than wrapping the entire behavior in an if block. I don't see a compelling reason to move from this approach.

If we need to move to a big if statement, you should not use the keyword hex as a variable name.

effigies · 2018-02-07T00:57:20Z

nipype/utils/filemanip.py

+				fmlogger.debug('mount file system types not described by fstype')
+    except:
+		fmlogger.debug('mount file system type check for CIFS error')
+		return []


What about just:

cifs_paths = [entry[0] for entry in mount_info if len(entry) > 1 and entry[1] == 'cifs']

Unless there's a case where a CIFS mount would fall into the case where len(entry) == 1, the warning seems like it'll be noise in the log.

Under what cases were you hitting this error, by the way?

Most of the removed lines were just a mistake on my part because I had moved the file from an older version of the nipype source. I think those lines should now be back in. Really the only part I introduced was the cifs table checking. It seems to work with my own mount table which now has about a dozen entries. This seems to be run only on the initial import, so the debug lines shouldn't be too great and should only be for those mount entries that only have a single item length.

The error shows up like this for anyone else who might run into this:

import nipype
...
File "nipype/utils/filemanip.py", line 272, in _generate_cifs_table
cifs_paths = [path for path, fstype in mount_info if fstype == 'cifs']
ValueError: need more than 1 value to unpack

I mean what does your mount output look like that doesn't have 5 entries in a given row?

the "mount" output and the mount_info in python looks like this:

% mount
/dev/disk2 on / (hfs, local, journaled)
devfs on /dev (devfs, local, nobrowse)
map -hosts on /net (autofs, nosuid, automounted, nobrowse)
map auto_home on /home (autofs, automounted, nobrowse)
map -fstab on /Network/Servers (autofs, automounted, nobrowse)
/dev/disk4s2 on /Volumes/DG TimeMachine (hfs, local, nodev, nosuid, journaled)
/dev/disk3s2 on /Volumes/MyBookData (hfs, local, nodev, nosuid, journaled)
afni:/elrond0 on /Volumes/afni (nfs)
afni:/var/www/INCOMING on /Volumes/INCOMING (nfs)
afni:/fraid on /Volumes/afni (nfs, asynchronous)
boromir:/raid.bot on /Volumes/raid.bot (nfs)
elros:/volume2/AFNI_SHARE on /Volumes/AFNI_SHARE (nfs)
map -static on /Volumes/safni (autofs, automounted, nobrowse)
map -static on /Volumes/raid.top (autofs, automounted, nobrowse)
/dev/disk1s3 on /Volumes/Boot OS X (hfs, local, journaled, nobrowse)

ipdb> mount_info
[['/Volumes/MyBookData', 'local,'], ['/Volumes/AFNI_SHARE'], ['/Volumes/INCOMING'], ['/Volumes/raid.bot'], ['/Volumes/afni'], ['/Volumes/afni', 'asynchronous)'], ['/Volumes/Boot', 'X'], ['/Volumes/DG', '(hfs,'], ['/dev', 'local,'], ['on', '(autofs,'], ['on', '(autofs,'], ['on', '(autofs,'], ['on', '(autofs,'], ['on', '(autofs,'], ['/', 'local,']]

effigies · 2018-02-08T23:39:47Z

Okay, so the issue seems to be that OSX has different mount output from Linux. I think we can make another case where we build the (mount point, filesystem type) pairs for OSX.

…

On Feb 8, 2018 18:28, "Daniel Glen" ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In nipype/utils/filemanip.py <#2428 (comment)>: > + # find which mount points are CIFS + # init to empty list + cifs_paths = [] + + try: + for path_and_fstype in mount_info: + # need to check for tables that have only path and no fstype + if len(path_and_fstype) == 2: + # if this entry is cifs, add it to list + if path_and_fstype[1] == 'cifs': + cifs_paths.append(path_and_fstype[0]) + else: + fmlogger.debug('mount file system types not described by fstype') + except: + fmlogger.debug('mount file system type check for CIFS error') + return [] the "mount" output and the mount_info in python looks like this: % mount /dev/disk2 on / (hfs, local, journaled) devfs on /dev (devfs, local, nobrowse) map -hosts on /net (autofs, nosuid, automounted, nobrowse) map auto_home on /home (autofs, automounted, nobrowse) map -fstab on /Network/Servers (autofs, automounted, nobrowse) /dev/disk4s2 on /Volumes/DG TimeMachine (hfs, local, nodev, nosuid, journaled) /dev/disk3s2 on /Volumes/MyBookData (hfs, local, nodev, nosuid, journaled) afni:/elrond0 on /Volumes/afni (nfs) afni:/var/www/INCOMING on /Volumes/INCOMING (nfs) afni:/fraid on /Volumes/afni (nfs, asynchronous) boromir:/raid.bot on /Volumes/raid.bot (nfs) elros:/volume2/AFNI_SHARE on /Volumes/AFNI_SHARE (nfs) map -static on /Volumes/safni (autofs, automounted, nobrowse) map -static on /Volumes/raid.top (autofs, automounted, nobrowse) /dev/disk1s3 on /Volumes/Boot OS X (hfs, local, journaled, nobrowse) ipdb> mount_info [['/Volumes/MyBookData', 'local,'], ['/Volumes/AFNI_SHARE'], ['/Volumes/INCOMING'], ['/Volumes/raid.bot'], ['/Volumes/afni'], ['/Volumes/afni', 'asynchronous)'], ['/Volumes/Boot', 'X'], ['/Volumes/DG', '(hfs,'], ['/dev', 'local,'], ['on', '(autofs,'], ['on', '(autofs,'], ['on', '(autofs,'], ['on', '(autofs,'], ['on', '(autofs,'], ['/', 'local,']] — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2428 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFF8iLmrf0k9E2jv1eFixPLrkCq08Nrks5tS4M7gaJpZM4R787H> .

afni-dglen · 2018-02-08T23:47:20Z

Yes, that seems like the case. I have a couple mount points that aren't working properly at the moment, and those are the ones that aren't passing the double string minimum. There are a few problems like spaces in the mounted directory names. The file system strings are probably not the ones you're looking for. The way they are currently parsed gets a parenthesis and a comma. In the end, you're only looking for cifs file systems here. I'm not sure these are used on Macs at all, but I can't say they never exist on Macs.

effigies · 2018-02-09T02:59:15Z

So looks like we have two formats:

Linux: sysfs on /sys type sysfs (rw,nosuid,nodev,noexec,relatime)
OSX: /dev/disk2 on / (hfs, local, journaled)

They're only a little different, so I think we can come up with a regular expression that will match either case:

pattern = re.compile(r'.*? on (/.*?) (?:type |\()(\S+)(?:, |\)| )')

Now mount_info construction can be simplified to the following, and we can revert the CIFS check:

mount_info = sorted((pattern.match(l).groups() for l in output.splitlines()),
                    key=lambda x: len(x[0]), reverse=True)
cifs_paths = [path for path, fstype in mount_info if fstype.lower() == 'cifs']

(I added in a .lower() just in case somebody's mount command outputs CIFS.)

Running it on your output, I get:

>>> mount_info
[('/Volumes/DG TimeMachine ', 'hfs'),
 ('/Volumes/MyBookData', 'hfs'),
 ('/Volumes/AFNI_SHARE', 'nfs'),
 ('/Volumes/Boot OS X', 'hfs'),
 ('/Volumes/INCOMING', 'nfs'),
 ('/Volumes/raid.bot', 'nfs'),
 ('/Volumes/raid.top', 'autofs'),
 ('/Network/Servers', 'autofs'),
 ('/Volumes/safni', 'autofs'),
 ('/Volumes/afni', 'nfs'),
 ('/Volumes/afni', 'nfs'),
 ('/home', 'autofs'),
 ('/dev', 'devfs'),
 ('/net', 'autofs'),
 ('/', 'hfs')]

On my system I get:

>>> mount_info
[('/sys/fs/cgroup/net_cls,net_prio', 'cgroup'),
 ('/sys/fs/cgroup/cpu,cpuacct', 'cgroup'),
 ('/sys/firmware/efi/efivars', 'efivarfs'),
 ('/sys/fs/cgroup/perf_event', 'cgroup'),
 ('/proc/sys/fs/binfmt_misc', 'autofs'),
 ('/sys/fs/fuse/connections', 'fusectl'),
 ('/sys/fs/cgroup/systemd', 'cgroup'),
 ('/sys/fs/cgroup/hugetlb', 'cgroup'),
 ('/sys/fs/cgroup/freezer', 'cgroup'),
 ('/sys/fs/cgroup/devices', 'cgroup'),
 ('/sys/fs/cgroup/memory', 'cgroup'),
 ('/sys/fs/cgroup/cpuset', 'cgroup'),
 ('/sys/kernel/security', 'securityfs'),
 ('/sys/fs/cgroup/blkio', 'cgroup'),
 ('/var/lib/docker/aufs', 'ext4'),
 ('/sys/fs/cgroup/pids', 'cgroup'),
 ('/run/user/1002/gvfs', 'fuse.gvfsd-fuse'),
 ('/sys/kernel/debug', 'debugfs'),
 ('/sys/fs/cgroup', 'tmpfs'),
 ('/sys/fs/pstore', 'pstore'),
 ('/dev/hugepages', 'hugetlbfs'),
 ('/run/user/1002', 'tmpfs'),
 ('/dev/mqueue', 'mqueue'),
 ('/run/lock', 'tmpfs'),
 ('/boot/efi', 'vfat'),
 ('/dev/pts', 'devpts'),
 ('/dev/shm', 'tmpfs'),
 ('/proc', 'proc'),
 ('/sys', 'sysfs'),
 ('/dev', 'devtmpfs'),
 ('/run', 'tmpfs'),
 ('/', 'ext4')]

I suspect if we end up needing to deal with more formats, it won't make sense to keep maintaining the one regex, but for now it's simple enough.

effigies · 2018-02-13T16:26:24Z

Hi @afni-dglen any thoughts on the approach I outlined above? (And any time to finish this off?)

effigies · 2018-02-15T22:54:03Z

I think this is worth fixing for 1.0.1, so I'll go ahead and make the changes I suggested, and add some tests.

I wonder if someone using Windows in Docker (@chrisfilo?) could quickly get a Linux mount output that includes a CIFS mount:

docker run --rm -it -v <some_path>:/data busybox mount

And OSX too, for that matter, but I don't know how easy that is.

chrisgorgo · 2018-02-15T22:58:51Z

Only cool kids can do this

PS C:\Users\filo> docker run --rm -it -v C:\:/data busybox mount
overlay on / type overlay (rw,relatime,lowerdir=/var/lib/docker/overlay2/l/26UTYITLF24YE7KEGTMHUNHPPG:/var/lib/docker/overlay2/l/SWGNP3T2EEB4CNBJFN3SDZLXHP,upperdir=/var/lib/docker/overlay2/a4c54ab1aa031bb5a14a424abd655510521e183ee4fa4158672e8376c89df394/diff,workdir=/var/lib/docker/overlay2/a4c54ab1aa031bb5a14a424abd655510521e183ee4fa4158672e8376c89df394/work)
proc on /proc type proc (rw,nosuid,nodev,noexec,relatime)
tmpfs on /dev type tmpfs (rw,nosuid,size=65536k,mode=755)
devpts on /dev/pts type devpts (rw,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=666)
sysfs on /sys type sysfs (ro,nosuid,nodev,noexec,relatime)
tmpfs on /sys/fs/cgroup type tmpfs (ro,nosuid,nodev,noexec,relatime,mode=755)
cpuset on /sys/fs/cgroup/cpuset type cgroup (ro,nosuid,nodev,noexec,relatime,cpuset)
cpu on /sys/fs/cgroup/cpu type cgroup (ro,nosuid,nodev,noexec,relatime,cpu)
cpuacct on /sys/fs/cgroup/cpuacct type cgroup (ro,nosuid,nodev,noexec,relatime,cpuacct)
blkio on /sys/fs/cgroup/blkio type cgroup (ro,nosuid,nodev,noexec,relatime,blkio)
memory on /sys/fs/cgroup/memory type cgroup (ro,nosuid,nodev,noexec,relatime,memory)
devices on /sys/fs/cgroup/devices type cgroup (ro,nosuid,nodev,noexec,relatime,devices)
freezer on /sys/fs/cgroup/freezer type cgroup (ro,nosuid,nodev,noexec,relatime,freezer)
net_cls on /sys/fs/cgroup/net_cls type cgroup (ro,nosuid,nodev,noexec,relatime,net_cls)
perf_event on /sys/fs/cgroup/perf_event type cgroup (ro,nosuid,nodev,noexec,relatime,perf_event)
net_prio on /sys/fs/cgroup/net_prio type cgroup (ro,nosuid,nodev,noexec,relatime,net_prio)
hugetlb on /sys/fs/cgroup/hugetlb type cgroup (ro,nosuid,nodev,noexec,relatime,hugetlb)
pids on /sys/fs/cgroup/pids type cgroup (ro,nosuid,nodev,noexec,relatime,pids)
cgroup on /sys/fs/cgroup/systemd type cgroup (ro,nosuid,nodev,noexec,relatime,name=systemd)
mqueue on /dev/mqueue type mqueue (rw,nosuid,nodev,noexec,relatime)
//10.0.75.1/C on /data type cifs (rw,relatime,vers=3.02,sec=ntlmsspi,cache=strict,username=filo,domain=MSI,uid=0,noforceuid,gid=0,noforcegid,addr=10.0.75.1,file_mode=0755,dir_mode=0755,iocharset=utf8,nounix,serverino,mapposix,nobrl,mfsymlinks,noperm,rsize=1048576,wsize=1048576,echo_interval=60,actimeo=1)
/dev/sda1 on /etc/resolv.conf type ext4 (rw,relatime,data=ordered)
/dev/sda1 on /etc/hostname type ext4 (rw,relatime,data=ordered)
/dev/sda1 on /etc/hosts type ext4 (rw,relatime,data=ordered)
shm on /dev/shm type tmpfs (rw,nosuid,nodev,noexec,relatime,size=65536k)
devpts on /dev/console type devpts (rw,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=666)
proc on /proc/bus type proc (ro,relatime)
proc on /proc/fs type proc (ro,relatime)
proc on /proc/irq type proc (ro,relatime)
proc on /proc/sys type proc (ro,relatime)
proc on /proc/sysrq-trigger type proc (ro,relatime)
tmpfs on /proc/kcore type tmpfs (rw,nosuid,size=65536k,mode=755)
tmpfs on /proc/timer_list type tmpfs (rw,nosuid,size=65536k,mode=755)
tmpfs on /proc/sched_debug type tmpfs (rw,nosuid,size=65536k,mode=755)
tmpfs on /proc/scsi type tmpfs (ro,relatime)
tmpfs on /sys/firmware type tmpfs (ro,relatime)

afni-dglen added 2 commits February 6, 2018 19:20

fstype parsing for cifs can fail on Mac OS

e511566

fstype parsing for cifs can fail on Mac OS

547e2de

effigies reviewed Feb 7, 2018

View reviewed changes

afni-dglen added 2 commits February 7, 2018 12:44

more proper integration

a0d48fd

tab/space fix

9e763ba

effigies added this to the 1.0.1 milestone Feb 13, 2018

effigies mentioned this pull request Feb 16, 2018

RF: Update and factor mount table parsing #2444

Merged

satra closed this in #2444 Feb 16, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

cfs checks #2428

cfs checks #2428

Uh oh!

afni-dglen commented Feb 7, 2018

Uh oh!

effigies left a comment

Uh oh!

effigies Feb 7, 2018

Uh oh!

effigies Feb 7, 2018

Uh oh!

effigies Feb 7, 2018

Uh oh!

afni-dglen Feb 7, 2018 •

edited

Loading

Uh oh!

effigies Feb 8, 2018

Uh oh!

afni-dglen Feb 8, 2018

Uh oh!

effigies commented Feb 8, 2018 via email

Uh oh!

afni-dglen commented Feb 8, 2018

Uh oh!

effigies commented Feb 9, 2018

Uh oh!

effigies commented Feb 13, 2018

Uh oh!

effigies commented Feb 15, 2018

Uh oh!

chrisgorgo commented Feb 15, 2018

Uh oh!

Uh oh!

cfs checks #2428

cfs checks #2428

Uh oh!

Conversation

afni-dglen commented Feb 7, 2018

Changes proposed in this pull request

Uh oh!

effigies left a comment

Choose a reason for hiding this comment

Uh oh!

effigies Feb 7, 2018

Choose a reason for hiding this comment

Uh oh!

effigies Feb 7, 2018

Choose a reason for hiding this comment

Uh oh!

effigies Feb 7, 2018

Choose a reason for hiding this comment

Uh oh!

afni-dglen Feb 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

effigies Feb 8, 2018

Choose a reason for hiding this comment

Uh oh!

afni-dglen Feb 8, 2018

Choose a reason for hiding this comment

Uh oh!

effigies commented Feb 8, 2018 via email

Uh oh!

afni-dglen commented Feb 8, 2018

Uh oh!

effigies commented Feb 9, 2018

Uh oh!

effigies commented Feb 13, 2018

Uh oh!

effigies commented Feb 15, 2018

Uh oh!

chrisgorgo commented Feb 15, 2018

Uh oh!

Uh oh!

afni-dglen Feb 7, 2018 •

edited

Loading