Message ID | 20180710155043.27366-1-aaptel@suse.com |
---|---|
Headers | show |
Series | man page cleanup | expand |
Great, thanks! Will take a closer look and merge if no objections. -- Best regards, Pavel Shilovsky 2018-07-10 8:50 GMT-07:00 Aurelien Aptel <aaptel@suse.com>: > Hi, > > I've noticed it's hard to keep the documentation for the mount options > in sync with what the kernel implements. > > 1) Several options were not documented. > 2) Several options were documented but were not implemented in the > kernel > 3) Additionnaly there are some issues where the mount options are not > very consistent: we have negative options without positive ones > ("nosharesock" but no "sharesock"). > > To help with that I've written a script that parses cifs.ko connect.c > and mount.cifs.rst (nothing fancy but works) and tries to cross check > for the 3 problems described above. For (1) it also prints the kernel > code associated with the option. > > I've added some logic to skip a few cases where no documentation is OK: > > * If the option is mapped to Opt_ignored > * If the negative ("noX" for "X" or "X" for "noX") option is > documented > * If any of the option aliases (option mapped to the same Opt_X enum) > is documented > > Give this information I've fixed the man page by adding missing > options, removing 2 (they were deprecated and not implemented by the > kernel anymore for some time). I did not fix any problem from (3). > > There are still some undocumented options but I've left them as is as > they are either internal options added by mount.cifs or because there > is a better interface to the feature (sec vs. sign). > > Here's the output of the script against my master before this > patchset: > > $ ./checkopts ~/prog/linux/for-next/fs/cifs/connect.c mount.cifs.rst > UNDOCUMENTED OPTIONS > ==================== > # skipping _netdev (Opt_ignore) > > OPTION noac ("noac" -> Opt_noac): > | pr_warn("CIFS: Mount option noac not supported. Instead set /proc/fs/cifs/LookupCacheEnabled to 0\n"); > | break; > > # skipping acl (noacl is documented) > > # skipping noauto (Opt_ignore) > > OPTION noautotune ("noautotune" -> Opt_noautotune): > | vol->noautotune = 1; > | break; > > OPTION noblocksend ("noblocksend" -> Opt_noblocksend): > | vol->noblocksnd = 1; > | break; > > # skipping brl (nobrl is documented) > > # skipping nocifsacl (cifsacl is documented) > > # skipping nodev (Opt_ignore) > > # skipping dev (Opt_ignore) > > OPTION nodfs ("nodfs" -> Opt_nodfs): > | vol->nodfs = 1; > | break; > > # skipping dirmode (alias dir_mode is documented) > > OPTION domainauto ("domainauto" -> Opt_domainauto): > | vol->domainauto = true; > | break; > > # skipping nodynperm (dynperm is documented) > > # skipping exec (Opt_ignore) > > # skipping noexec (Opt_ignore) > > # skipping noforcegid (forcegid is documented) > > OPTION forcemand ("forcemand" -> Opt_forcemandatorylock): > | vol->mand_lock = 1; > | break; > > OPTION forcemandatorylock ("forcemandatorylock" -> Opt_forcemandatorylock): > | vol->mand_lock = 1; > | break; > > # skipping noforceuid (forceuid is documented) > > # skipping nohard (hard is documented) > > OPTION idsfromsid ("idsfromsid" -> Opt_setuidfromacl): > | vol->setuidfromacl = 1; > | break; > > OPTION linux ("linux" -> Opt_unix): > | if (vol->no_linux_ext) > | cifs_dbg(VFS, > | "conflicting unix mount options\n"); > | vol->linux_ext = 1; > | break; > > # skipping nolinux (alias nounix is documented) > > OPTION locallease ("locallease" -> Opt_locallease): > | vol->local_lease = 1; > | break; > > # skipping nolock (alias nobrl is documented) > > # skipping nomand (Opt_ignore) > > # skipping mand (Opt_ignore) > > OPTION nomapposix ("nomapposix" -> Opt_nomapposix): > | vol->remap = false; > | break; > > OPTION mapposix ("mapposix" -> Opt_mapposix): > | vol->remap = true; > | vol->sfu_remap = false; /* disable SFU mapping */ > | break; > > OPTION max_credits ("max_credits=%s" -> Opt_max_credits): > | if (get_option_ul(args, &option) || (option < 20) || > | (option > 60000)) { > | cifs_dbg(VFS, "%s: Invalid max_credits value\n", > | __func__); > | goto cifs_parse_mount_err; > | } > | vol->max_credits = option; > | break; > | > | /* String Arguments */ > | > > # skipping path (Opt_ignore) > > OPTION persistenthandles ("persistenthandles" -> Opt_persistent): > | vol->persistent = true; > | if ((vol->nopersistent) || (vol->resilient)) { > | cifs_dbg(VFS, > | "persistenthandles mount options conflict\n"); > | goto cifs_parse_mount_err; > | } > | break; > > OPTION nopersistenthandles ("nopersistenthandles" -> Opt_nopersistent): > | vol->nopersistent = true; > | if (vol->persistent) { > | cifs_dbg(VFS, > | "persistenthandles mount options conflict\n"); > | goto cifs_parse_mount_err; > | } > | break; > > # skipping noposix (alias nounix is documented) > > OPTION posix ("posix" -> Opt_unix): > | if (vol->no_linux_ext) > | cifs_dbg(VFS, > | "conflicting unix mount options\n"); > | vol->linux_ext = 1; > | break; > > OPTION rdma ("rdma" -> Opt_rdma): > | vol->rdma = true; > | break; > | > | /* Numeric Values */ > > OPTION noresilienthandles ("noresilienthandles" -> Opt_noresilient): > | vol->resilient = false; /* already the default */ > | break; > > OPTION resilienthandles ("resilienthandles" -> Opt_resilient): > | vol->resilient = true; > | if (vol->persistent) { > | cifs_dbg(VFS, > | "persistenthandles mount options conflict\n"); > | goto cifs_parse_mount_err; > | } > | break; > > # skipping nosfu (sfu is documented) > > OPTION nosharesock ("nosharesock" -> Opt_nosharesock): > | vol->nosharesock = true; > | break; > > OPTION sign ("sign" -> Opt_sign): > | vol->sign = true; > | break; > > OPTION sloppy ("sloppy" -> Opt_sloppy): > | sloppy = true; > | break; > > OPTION snapshot ("snapshot=%s" -> Opt_snapshot): > | if (get_option_ul(args, &option)) { > | cifs_dbg(VFS, "%s: Invalid snapshot time\n", > | __func__); > | goto cifs_parse_mount_err; > | } > | vol->snapshot_time = option; > | break; > > # skipping nosoft (soft is documented) > > OPTION srcaddr ("srcaddr=%s" -> Opt_srcaddr): > | string = match_strdup(args); > | if (string == NULL) > | goto out_nomem; > | > | if (!cifs_convert_address( > | (struct sockaddr *)&vol->srcaddr, > | string, strlen(string))) { > | pr_warn("CIFS: Could not parse srcaddr: %s\n", > | string); > | goto cifs_parse_mount_err; > | } > | break; > > OPTION strictsync ("strictsync" -> Opt_strictsync): > | vol->nostrictsync = 0; > | break; > > OPTION nostrictsync ("nostrictsync" -> Opt_nostrictsync): > | vol->nostrictsync = 1; > | break; > > # skipping nosuid (Opt_ignore) > > # skipping suid (Opt_ignore) > > # skipping target (Opt_ignore) > > # skipping unc (Opt_ignore) > > # skipping unix (nounix is documented) > > # skipping user_xattr (nouser_xattr is documented) > > OPTION ver ("ver=%s" -> Opt_ver): > | /* version of mount userspace tools, not dialect */ > | string = match_strdup(args); > | if (string == NULL) > | goto out_nomem; > | > | /* If interface changes in mount.cifs bump to new ver */ > | if (strncasecmp(string, "1", 1) == 0) { > | if (strlen(string) > 1) { > | pr_warn("Bad mount helper ver=%s. Did " > | "you want SMB1 (CIFS) dialect " > | "and mean to type vers=1.0 " > | "instead?\n", string); > | goto cifs_parse_mount_err; > | } > | /* This is the default */ > | break; > | } > | /* For all other value, error */ > | pr_warn("CIFS: Invalid mount helper version specified\n"); > | goto cifs_parse_mount_err; > > > DOCUMENTED BUT NON-EXISTING OPTIONS > =================================== > OPTION directio ("directio") > OPTION servernetbiosname ("servernetbiosname=arg") > OPTION strictcache ("strictcache") > > NEGATIVE OPTIONS WITHOUT POSITIVE > ================================= > OPTION noac exists but not ac > OPTION noauto exists but not auto > OPTION noautotune exists but not autotune > OPTION noblocksend exists but not blocksend > OPTION nocase exists but not case > OPTION nodfs exists but not dfs > OPTION nolock exists but not lock > OPTION nosharesock exists but not sharesock > > > Aurelien Aptel (2): > checkopts: add python script to cross check mount options > mount.cifs.rst: document missing options, correct wrong ones > > checkopts | 240 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > mount.cifs.rst | 111 ++++++++++++++++++-------- > 2 files changed, 319 insertions(+), 32 deletions(-) > create mode 100755 checkopts > > -- > 2.13.6 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Thanks, Aurelien for doing this. For rdma mount option as Steven suggested we can add "vers=3 which defaults to any of the 3.0 or later dialects" with my patch. https://patchwork.kernel.org/patch/1029030 On Tue, Jul 10, 2018 at 9:45 PM, Pavel Shilovsky <piastryyy@gmail.com> wrote: > Great, thanks! Will take a closer look and merge if no objections. > > -- > Best regards, > Pavel Shilovsky > > > 2018-07-10 8:50 GMT-07:00 Aurelien Aptel <aaptel@suse.com>: >> Hi, >> >> I've noticed it's hard to keep the documentation for the mount options >> in sync with what the kernel implements. >> >> 1) Several options were not documented. >> 2) Several options were documented but were not implemented in the >> kernel >> 3) Additionnaly there are some issues where the mount options are not >> very consistent: we have negative options without positive ones >> ("nosharesock" but no "sharesock"). >> >> To help with that I've written a script that parses cifs.ko connect.c >> and mount.cifs.rst (nothing fancy but works) and tries to cross check >> for the 3 problems described above. For (1) it also prints the kernel >> code associated with the option. >> >> I've added some logic to skip a few cases where no documentation is OK: >> >> * If the option is mapped to Opt_ignored >> * If the negative ("noX" for "X" or "X" for "noX") option is >> documented >> * If any of the option aliases (option mapped to the same Opt_X enum) >> is documented >> >> Give this information I've fixed the man page by adding missing >> options, removing 2 (they were deprecated and not implemented by the >> kernel anymore for some time). I did not fix any problem from (3). >> >> There are still some undocumented options but I've left them as is as >> they are either internal options added by mount.cifs or because there >> is a better interface to the feature (sec vs. sign). >> >> Here's the output of the script against my master before this >> patchset: >> >> $ ./checkopts ~/prog/linux/for-next/fs/cifs/connect.c mount.cifs.rst >> UNDOCUMENTED OPTIONS >> ==================== >> # skipping _netdev (Opt_ignore) >> >> OPTION noac ("noac" -> Opt_noac): >> | pr_warn("CIFS: Mount option noac not supported. Instead set /proc/fs/cifs/LookupCacheEnabled to 0\n"); >> | break; >> >> # skipping acl (noacl is documented) >> >> # skipping noauto (Opt_ignore) >> >> OPTION noautotune ("noautotune" -> Opt_noautotune): >> | vol->noautotune = 1; >> | break; >> >> OPTION noblocksend ("noblocksend" -> Opt_noblocksend): >> | vol->noblocksnd = 1; >> | break; >> >> # skipping brl (nobrl is documented) >> >> # skipping nocifsacl (cifsacl is documented) >> >> # skipping nodev (Opt_ignore) >> >> # skipping dev (Opt_ignore) >> >> OPTION nodfs ("nodfs" -> Opt_nodfs): >> | vol->nodfs = 1; >> | break; >> >> # skipping dirmode (alias dir_mode is documented) >> >> OPTION domainauto ("domainauto" -> Opt_domainauto): >> | vol->domainauto = true; >> | break; >> >> # skipping nodynperm (dynperm is documented) >> >> # skipping exec (Opt_ignore) >> >> # skipping noexec (Opt_ignore) >> >> # skipping noforcegid (forcegid is documented) >> >> OPTION forcemand ("forcemand" -> Opt_forcemandatorylock): >> | vol->mand_lock = 1; >> | break; >> >> OPTION forcemandatorylock ("forcemandatorylock" -> Opt_forcemandatorylock): >> | vol->mand_lock = 1; >> | break; >> >> # skipping noforceuid (forceuid is documented) >> >> # skipping nohard (hard is documented) >> >> OPTION idsfromsid ("idsfromsid" -> Opt_setuidfromacl): >> | vol->setuidfromacl = 1; >> | break; >> >> OPTION linux ("linux" -> Opt_unix): >> | if (vol->no_linux_ext) >> | cifs_dbg(VFS, >> | "conflicting unix mount options\n"); >> | vol->linux_ext = 1; >> | break; >> >> # skipping nolinux (alias nounix is documented) >> >> OPTION locallease ("locallease" -> Opt_locallease): >> | vol->local_lease = 1; >> | break; >> >> # skipping nolock (alias nobrl is documented) >> >> # skipping nomand (Opt_ignore) >> >> # skipping mand (Opt_ignore) >> >> OPTION nomapposix ("nomapposix" -> Opt_nomapposix): >> | vol->remap = false; >> | break; >> >> OPTION mapposix ("mapposix" -> Opt_mapposix): >> | vol->remap = true; >> | vol->sfu_remap = false; /* disable SFU mapping */ >> | break; >> >> OPTION max_credits ("max_credits=%s" -> Opt_max_credits): >> | if (get_option_ul(args, &option) || (option < 20) || >> | (option > 60000)) { >> | cifs_dbg(VFS, "%s: Invalid max_credits value\n", >> | __func__); >> | goto cifs_parse_mount_err; >> | } >> | vol->max_credits = option; >> | break; >> | >> | /* String Arguments */ >> | >> >> # skipping path (Opt_ignore) >> >> OPTION persistenthandles ("persistenthandles" -> Opt_persistent): >> | vol->persistent = true; >> | if ((vol->nopersistent) || (vol->resilient)) { >> | cifs_dbg(VFS, >> | "persistenthandles mount options conflict\n"); >> | goto cifs_parse_mount_err; >> | } >> | break; >> >> OPTION nopersistenthandles ("nopersistenthandles" -> Opt_nopersistent): >> | vol->nopersistent = true; >> | if (vol->persistent) { >> | cifs_dbg(VFS, >> | "persistenthandles mount options conflict\n"); >> | goto cifs_parse_mount_err; >> | } >> | break; >> >> # skipping noposix (alias nounix is documented) >> >> OPTION posix ("posix" -> Opt_unix): >> | if (vol->no_linux_ext) >> | cifs_dbg(VFS, >> | "conflicting unix mount options\n"); >> | vol->linux_ext = 1; >> | break; >> >> OPTION rdma ("rdma" -> Opt_rdma): >> | vol->rdma = true; >> | break; >> | >> | /* Numeric Values */ >> >> OPTION noresilienthandles ("noresilienthandles" -> Opt_noresilient): >> | vol->resilient = false; /* already the default */ >> | break; >> >> OPTION resilienthandles ("resilienthandles" -> Opt_resilient): >> | vol->resilient = true; >> | if (vol->persistent) { >> | cifs_dbg(VFS, >> | "persistenthandles mount options conflict\n"); >> | goto cifs_parse_mount_err; >> | } >> | break; >> >> # skipping nosfu (sfu is documented) >> >> OPTION nosharesock ("nosharesock" -> Opt_nosharesock): >> | vol->nosharesock = true; >> | break; >> >> OPTION sign ("sign" -> Opt_sign): >> | vol->sign = true; >> | break; >> >> OPTION sloppy ("sloppy" -> Opt_sloppy): >> | sloppy = true; >> | break; >> >> OPTION snapshot ("snapshot=%s" -> Opt_snapshot): >> | if (get_option_ul(args, &option)) { >> | cifs_dbg(VFS, "%s: Invalid snapshot time\n", >> | __func__); >> | goto cifs_parse_mount_err; >> | } >> | vol->snapshot_time = option; >> | break; >> >> # skipping nosoft (soft is documented) >> >> OPTION srcaddr ("srcaddr=%s" -> Opt_srcaddr): >> | string = match_strdup(args); >> | if (string == NULL) >> | goto out_nomem; >> | >> | if (!cifs_convert_address( >> | (struct sockaddr *)&vol->srcaddr, >> | string, strlen(string))) { >> | pr_warn("CIFS: Could not parse srcaddr: %s\n", >> | string); >> | goto cifs_parse_mount_err; >> | } >> | break; >> >> OPTION strictsync ("strictsync" -> Opt_strictsync): >> | vol->nostrictsync = 0; >> | break; >> >> OPTION nostrictsync ("nostrictsync" -> Opt_nostrictsync): >> | vol->nostrictsync = 1; >> | break; >> >> # skipping nosuid (Opt_ignore) >> >> # skipping suid (Opt_ignore) >> >> # skipping target (Opt_ignore) >> >> # skipping unc (Opt_ignore) >> >> # skipping unix (nounix is documented) >> >> # skipping user_xattr (nouser_xattr is documented) >> >> OPTION ver ("ver=%s" -> Opt_ver): >> | /* version of mount userspace tools, not dialect */ >> | string = match_strdup(args); >> | if (string == NULL) >> | goto out_nomem; >> | >> | /* If interface changes in mount.cifs bump to new ver */ >> | if (strncasecmp(string, "1", 1) == 0) { >> | if (strlen(string) > 1) { >> | pr_warn("Bad mount helper ver=%s. Did " >> | "you want SMB1 (CIFS) dialect " >> | "and mean to type vers=1.0 " >> | "instead?\n", string); >> | goto cifs_parse_mount_err; >> | } >> | /* This is the default */ >> | break; >> | } >> | /* For all other value, error */ >> | pr_warn("CIFS: Invalid mount helper version specified\n"); >> | goto cifs_parse_mount_err; >> >> >> DOCUMENTED BUT NON-EXISTING OPTIONS >> =================================== >> OPTION directio ("directio") >> OPTION servernetbiosname ("servernetbiosname=arg") >> OPTION strictcache ("strictcache") >> >> NEGATIVE OPTIONS WITHOUT POSITIVE >> ================================= >> OPTION noac exists but not ac >> OPTION noauto exists but not auto >> OPTION noautotune exists but not autotune >> OPTION noblocksend exists but not blocksend >> OPTION nocase exists but not case >> OPTION nodfs exists but not dfs >> OPTION nolock exists but not lock >> OPTION nosharesock exists but not sharesock >> >> >> Aurelien Aptel (2): >> checkopts: add python script to cross check mount options >> mount.cifs.rst: document missing options, correct wrong ones >> >> checkopts | 240 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> mount.cifs.rst | 111 ++++++++++++++++++-------- >> 2 files changed, 319 insertions(+), 32 deletions(-) >> create mode 100755 checkopts >> >> -- >> 2.13.6 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html