Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mount.mounted does not completely unmount NFS mounts when options change #18907

Closed
wwentland opened this issue Dec 11, 2014 · 12 comments · Fixed by #65351
Closed

mount.mounted does not completely unmount NFS mounts when options change #18907

wwentland opened this issue Dec 11, 2014 · 12 comments · Fixed by #65351
Assignees
Labels
Bug broken, incorrect, or confusing behavior Platform Relates to OS, containers, platform-based utilities like FS, system based apps severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around

Comments

@wwentland
Copy link
Contributor

As mentioned in #18474 mount.mounted needs to unmount NFS mounts and then mount them again if their mount options change. This requires NFS to be special cased as it behaves differently than other filesystems in that regard.

In particular the following code requires some tweaks to allow for a complete "mount cycle":

       if opts:                                                             
                mount_invisible_options = ['defaults', 'comment', 'nobootwait', 'reconnect', 'delay_connect']
                for opt in opts:                                                 
                    comment_option = opt.split('=')[0]                           
                    if comment_option == 'comment':                              
                        opt = comment_option                                     
                    if opt not in active[real_name]['opts'] and opt not in mount_invisible_options:
                        if __opts__['test']:                                     
                            ret['result'] = None                                 
                            ret['comment'] = "Remount would be forced because options changed"
                            return ret                                           
                        else:                                                    
                            ret['changes']['umount'] = "Forced remount because " \
                                                        + "options changed"      
                            remount_result = __salt__['mount.remount'](real_name, device, mkmnt=mkmnt, fstype=fstype, opts=opts)
                            ret['result'] = remount_result

Here an additional check for the actual filesystem needs to be performed before remount_result = __salt__['mount.remount'](...) to use mount.unmount and mount.mount in lieu of mount.remount if NFS specific options changed.

@rallytime rallytime added Bug broken, incorrect, or confusing behavior severity-low 4th level, cosemtic problems, work around exists labels Dec 11, 2014
@rallytime rallytime added this to the Approved milestone Dec 11, 2014
@rallytime
Copy link
Contributor

Thanks for the report @BABILEN.

@garethgreenaway - I think you've been doing the most work with mount lately. Just wanted to ping you here in case you wanted to check this one out, too. If not, we can take a look as well.

@garethgreenaway
Copy link
Contributor

Once I get the other mount issue resolved I should be ale to take a look at this one.

@garethgreenaway
Copy link
Contributor

Hope to have a pull request for this sometime tomorrow.

@rallytime
Copy link
Contributor

Awesome! Thanks @garethgreenaway!

@ckochenower ckochenower added Core relates to code central or existential to Salt P2 Priority 2 severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around Platform Relates to OS, containers, platform-based utilities like FS, system based apps and removed severity-low 4th level, cosemtic problems, work around exists Core relates to code central or existential to Salt labels Jun 2, 2015
@Reiner030
Copy link

Hi, it seems that this problems is not fixed yet. Perhaps you can done a lazy unmount for NFS mountpoints ?

# lsb_release -d
Description:    Debian GNU/Linux 8.2 (jessie)
# salt-call --version
salt-call 2015.5.3 (Lithium)
# salt-call state.highstate --state-output=changes
...
----------
          ID: /data/storage
    Function: mount.mounted
      Result: False
     Comment: Unable to unmount /data/storage: umount.nfs: /data/storage: device is busy.
     Started: 11:02:54.681981
    Duration: 99.953 ms
     Changes:
              ----------
              umount:
                  Forced unmount and mount because options (async) changed
...

@schlagify
Copy link

schlagify commented Mar 27, 2017

+1 for fixing this, still seeing this issue with 2016.3.5+ds-1 Debian, Jessie

@stale
Copy link

stale bot commented Sep 28, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

@stale stale bot added the stale label Sep 28, 2018
@stale stale bot closed this as completed Oct 6, 2018
@Reiner030
Copy link

Nice how prooved bugs are not fixed for over three years and stale bot automatically closes them "silently" ...
This problem occurs also for all FUSE based filesystems because they can't be remounted and is still a problem as far I know.

@garethgreenaway
Copy link
Contributor

Stalebot is closing the issue because there hasn't been any additional comments. Issues can always be reopened 😄 if you can provide some more specifics about how the state module fails on fuse based file systems, I can take a look.

@stale
Copy link

stale bot commented Oct 7, 2018

Thank you for updating this issue. It is no longer marked as stale.

@stale stale bot removed the stale label Oct 7, 2018
@sagetherage sagetherage removed the P2 Priority 2 label Jun 3, 2020
@sagetherage sagetherage modified the milestones: Approved, Aluminium Jul 29, 2020
@sagetherage sagetherage added the Aluminium Release Post Mg and Pre Si label Jul 29, 2020
@Ch3LL
Copy link
Contributor

Ch3LL commented Aug 3, 2023

Bump ^ We need more information on how this is failing on fuse systems

@Reiner030
Copy link

Bump ^ We need more information on how this is failing on fuse systems

yes, reading this thread could help ;-)

As I wrote ... let me check ... 8 years ago there is the problem that FUSE has only 2 possible tasks.

  • mount
  • unmount

so your module which does a "mount -r" must fail...
In this case it could be a nice option to do on FUSE mount points which can be recognized as FUSE based mount points instead an "unmount -l ... && mount ... "

Quick research shows e.g. here similar answers / no "-r" mount option for FUSE mounts:
https://www.kernel.org/doc/html/next/filesystems/fuse.html
https://fuse-devel.narkive.com/z81wTi5H/lazy-unmount

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior Platform Relates to OS, containers, platform-based utilities like FS, system based apps severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants