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

enable Nfs autoreconnect #19513

Merged
merged 1 commit into from Apr 5, 2021
Merged

enable Nfs autoreconnect #19513

merged 1 commit into from Apr 5, 2021

Conversation

wsnipex
Copy link
Member

@wsnipex wsnipex commented Apr 4, 2021

Description

enables autoreconnect in libnfs.
Defaults to unlimited retries, just like standard NFS clients

configurable through AV.xml:

  <network>
    <nfsretries>10</nfsretries> <!-- 0 to disable, default: -1 (unlimited) -->
  </network>

quote from libnfs.h

* autoreconnect=<-1|0|>=1>
 *                   : Control the auto-reconnect behaviour to the NFS session.
 *                    -1 : Try to reconnect forever on session failures.
 *                         Just like normal NFS clients do.
 *                     0 : Disable auto-reconnect completely and immediately
 *                         return a failure to the application.
 *                   >=1 : Retry to connect back to the server this many
 *                         times before failing and returing an error back
 *                         to the application.

Motivation and Context

NFS should behave like smb and retry intermittent connection failures

How Has This Been Tested?

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

Checklist:

  • My code follows the Code Guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the Contributing document
  • I have added tests to cover my change
  • All new and existing tests passed

@arnova
Copy link
Member

arnova commented Apr 4, 2021

Shouldn't we implement an <nfs> tag (in as.xml) that contains all the nfs options, like we did for curl?

@wsnipex
Copy link
Member Author

wsnipex commented Apr 4, 2021

is it worth it for just 2 options?

@wsnipex
Copy link
Member Author

wsnipex commented Apr 5, 2021

jenkins build this please

@arnova
Copy link
Member

arnova commented Apr 5, 2021

is it worth it for just 2 options?

Perhaps not yet, if it's for only 2 options atm. But let's keep it in mind for the future when more stuff gets added...

@wsnipex wsnipex merged commit 8326a68 into xbmc:master Apr 5, 2021
@wsnipex wsnipex mentioned this pull request Apr 5, 2021
13 tasks
@arnova
Copy link
Member

arnova commented Apr 6, 2021

@wsnipex : Forgot to mention that it may be wise to disable autoreconnect when caching is enabled, like we do in smb/curl. Else the fs may hang for a longer time than desirable from a user's perspective. See https://github.com/arnova/xbmc/blob/f44fdfbf675f30c01e7639177a34544e6a6b9dad/xbmc/platform/posix/filesystem/SMBFile.cpp#L708-L712

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants