Linux CIFS filesystem development
 help / color / mirror / Atom feed
From: Enzo Matsumiya <ematsumiya@suse.de>
To: Tom Talpey <tom@talpey.com>
Cc: Rowland Penny <rpenny@samba.org>, linux-cifs@vger.kernel.org
Subject: Re: [RFC PATCH 0/3] Rename "cifs" module to "smbfs"
Date: Tue, 2 Aug 2022 10:52:01 -0300	[thread overview]
Message-ID: <20220802135201.4vm36drd5mp57nvv@cyberdelia> (raw)
In-Reply-To: <cc925d11-df62-fd92-f21f-4aca10e3a68d@talpey.com>

On 08/01, Tom Talpey wrote:
>On 8/1/2022 4:14 PM, Enzo Matsumiya wrote:
>>On 08/01, Rowland Penny wrote:
>>>On Mon, 2022-08-01 at 16:09 -0300, Enzo Matsumiya via samba-technical
>>>wrote:
>>>>Hi,
>>>>
>>>>As part of the ongoing effort to remove the "cifs" nomenclature from
>>>>the
>>>>Linux SMB client, I'm proposing the rename of the module to "smbfs".
>>>
>>>Hi, this has absolutely nothing to do with myself, but Linux used
>>>'smbfs' before it started to use 'cifs', so you are going back to an
>>>old term. This could be confusing.
>>
>>Hi Rowland, I'm aware of that. I had nothing to do with either
>>(choosing initial "smbfs" nor "cifs"), but, IMHO, I think it should've
>>stayed "smbfs". And TBH this is the most coherent name, of all
>
>I dug around the old tarballs and it looks like fs/smbfs was pulled from
>the kernel after 2.6.36, in early 2011. This was different from fs/cifs,
>which entered the kernel much earlier, so they previously coexisted.
>
>I don't think the name ambiguity is very important, but I do wonder if
>git might uncover some conflicts, when a previously removed directory
>suddenly reappears with new content? There wasn't a lot in fs/smbfs
>though.

I haven't considered that.
Doing a "git log --follow -- fs/smbfs" does show the older commits for
before the previous migration/rename:

----
commit 1e20c73a2935be2d9f19ebc63ddee1afccc42b07
Author: Enzo Matsumiya <ematsumiya@suse.de>
Date:   Mon Aug 1 15:05:23 2022 -0300

     smbfs: rename directory "fs/cifs" -> "fs/smbfs"

     Update fs/Kconfig and fs/Makefile to reflect the change.

     Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>

commit be9eee2e8b87e335531a3ae13abb8d26e834c438
Author: Christoph Hellwig <hch@infradead.org>
Date:   Sun Oct 10 05:36:29 2010 -0400

     smbfs: use dget_parent

     Use dget_parent instead of opencoding it.  This simplifies the code, but
     more importanly prepares for the more complicated locking for a parent
     dget in the dcache scale patch series.

     Note that the d_time assignment in smb_renew_times moves out of d_lock,
     but it's a single atomic 32-bit value, and that's what other sites
     setting it do already.

     Signed-off-by: Christoph Hellwig <hch@lst.de>
     Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
...
----

I don't know if that would cause a real problem though, someone more
experienced with renaming modules/directories could provide their
opinion.

Could we have an empty commit between old commits and the new rename to
serve as a marker maybe?

>Either way, I think the module name is the question here, and it doesn't
>have to be the same as the directory. I still prefer smbfs.
>
>Another possibility for the directory is "ksmb", which might rhyme with
>the server, and keep it close alphabetically too? OTOH it might be
>confusing to have two similar names.

My original idea of "ideal" was to have:

fs/smbfs/
fs/smbfs/common
fs/smbfs/client
fs/smbfs/server

Which aligns with e.g., drivers/nvme/ that has host/ and target/ subdirs
to accomodate client and server code. This would make things way more
manageable given the quantity of shared code between cifs.ko and
ksmbd.ko.

Another option is:
fs/smbfs_client/
fs/smbfs_common/ (already existing)
fs/smbfs_server/

But, personally, I'm not really a fan of the underscores.

My 2c only though.
Thoughts?

>And sorry but I hate the idea of adding "-client". Should we rename
>ksmbd to smb-server?? I don't think so.

Agreed.

>Tom.

Cheers,

Enzo

>>available/known choices; you know the protocol (SMB), you know it isn't
>>tied to any SMB version ("cifs", or "smb3" as sometimes suggested or
>>used (as a module alias)), it's a Linux filesystem module ("FS").
>>
>>Also the "fs/smbfs_common" directory was renamed as recent as last year
>>(from "cifs_common") (cf. commit 23e91d8b7).
>>
>>>Rowland
>>
>>Thanks for the input, though. As an RFC patch, I'm waiting for more
>>feedback and suggestions.
>>
>>
>>Cheers,
>>
>>Enzo
>>
>>>>
>>>>As it's widely known, CIFS is associated to SMB1.0, which, in turn,
>>>>is
>>>>associated with the security issues it presented in the past. Using
>>>>"SMBFS" makes clear what's the protocol in use for outsiders, but
>>>>also
>>>>unties it from any particular protocol version. It also fits in the
>>>>already existing "fs/smbfs_common" and "fs/ksmbd" naming scheme.
>>>>
>>>>This short patch series only changes directory names and
>>>>includes/ifdefs in
>>>>headers and source code, and updates docs to reflect the rename.
>>>>Other
>>>>than that, no source code/functionality is modified (WIP though).
>>>>
>>>>Patch 1/3: effectively changes the module name to "smbfs" and create
>>>>a
>>>>       "cifs" module alias to maintain compatibility (a warning
>>>>       should be added to indicate the complete removal/isolation
>>>>of
>>>>       CIFS/SMB1.0 code).
>>>>Patch 2/3: rename the source-code directory to align with the new
>>>>module
>>>>       name
>>>>Patch 3/3: update documentation references to "fs/cifs" or "cifs.ko"
>>>>or
>>>>       "cifs module" to use the new name
>>>>
>>>>Enzo Matsumiya (3):
>>>>  cifs: change module name to "smbfs.ko"
>>>>  smbfs: rename directory "fs/cifs" -> "fs/smbfs"
>>>>  smbfs: update doc references
>>>>
>>>> Documentation/admin-guide/index.rst           |   2 +-
>>>> .../admin-guide/{cifs => smbfs}/authors.rst   |   0
>>>> .../admin-guide/{cifs => smbfs}/changes.rst   |   4 +-
>>>> .../admin-guide/{cifs => smbfs}/index.rst     |   0
>>>> .../{cifs => smbfs}/introduction.rst          |   0
>>>> .../admin-guide/{cifs => smbfs}/todo.rst      |  12 +-
>>>> .../admin-guide/{cifs => smbfs}/usage.rst     | 168 +++++++++-------
>>>>-- 
>>>> .../{cifs => smbfs}/winucase_convert.pl       |   0
>>>> Documentation/filesystems/index.rst           |   2 +-
>>>> .../filesystems/{cifs => smbfs}/cifsroot.rst  |  14 +-
>>>> .../filesystems/{cifs => smbfs}/index.rst     |   0
>>>> .../filesystems/{cifs => smbfs}/ksmbd.rst     |   2 +-
>>>> Documentation/networking/dns_resolver.rst     |   2 +-
>>>> .../translations/zh_CN/admin-guide/index.rst  |   2 +-
>>>> .../translations/zh_TW/admin-guide/index.rst  |   2 +-
>>>> fs/Kconfig                                    |   6 +-
>>>> fs/Makefile                                   |   2 +-
>>>> fs/cifs/Makefile                              |  34 ----
>>>> fs/{cifs => smbfs}/Kconfig                    | 108 +++++------
>>>> fs/smbfs/Makefile                             |  34 ++++
>>>> fs/{cifs => smbfs}/asn1.c                     |   0
>>>> fs/{cifs => smbfs}/cifs_debug.c               |  72 ++++----
>>>> fs/{cifs => smbfs}/cifs_debug.h               |   4 +-
>>>> fs/{cifs => smbfs}/cifs_dfs_ref.c             |   2 +-
>>>> fs/{cifs => smbfs}/cifs_fs_sb.h               |   0
>>>> fs/{cifs => smbfs}/cifs_ioctl.h               |   0
>>>> fs/{cifs => smbfs}/cifs_spnego.c              |   4 +-
>>>> fs/{cifs => smbfs}/cifs_spnego.h              |   0
>>>> .../cifs_spnego_negtokeninit.asn1             |   0
>>>> fs/{cifs => smbfs}/cifs_swn.c                 |   0
>>>> fs/{cifs => smbfs}/cifs_swn.h                 |   4 +-
>>>> fs/{cifs => smbfs}/cifs_unicode.c             |   0
>>>> fs/{cifs => smbfs}/cifs_unicode.h             |   0
>>>> fs/{cifs => smbfs}/cifs_uniupr.h              |   0
>>>> fs/{cifs => smbfs}/cifsacl.c                  |   6 +-
>>>> fs/{cifs => smbfs}/cifsacl.h                  |   0
>>>> fs/{cifs => smbfs}/cifsencrypt.c              |   0
>>>> fs/{cifs => smbfs}/cifsglob.h                 |  26 +--
>>>> fs/{cifs => smbfs}/cifspdu.h                  |   6 +-
>>>> fs/{cifs => smbfs}/cifsproto.h                |  10 +-
>>>> fs/{cifs => smbfs}/cifsroot.c                 |   0
>>>> fs/{cifs => smbfs}/cifssmb.c                  |  14 +-
>>>> fs/{cifs => smbfs}/connect.c                  |  36 ++--
>>>> fs/{cifs/cifsfs.c => smbfs/core.c}            |  49 ++---
>>>> fs/{cifs => smbfs}/dfs_cache.c                |   2 +-
>>>> fs/{cifs => smbfs}/dfs_cache.h                |   0
>>>> fs/{cifs => smbfs}/dir.c                      |   2 +-
>>>> fs/{cifs => smbfs}/dns_resolve.c              |   0
>>>> fs/{cifs => smbfs}/dns_resolve.h              |   0
>>>> fs/{cifs => smbfs}/export.c                   |   8 +-
>>>> fs/{cifs => smbfs}/file.c                     |  16 +-
>>>> fs/{cifs => smbfs}/fs_context.c               |  20 +--
>>>> fs/{cifs => smbfs}/fs_context.h               |   0
>>>> fs/{cifs => smbfs}/fscache.c                  |   0
>>>> fs/{cifs => smbfs}/fscache.h                  |   6 +-
>>>> fs/{cifs => smbfs}/inode.c                    |  10 +-
>>>> fs/{cifs => smbfs}/ioctl.c                    |   6 +-
>>>> fs/{cifs => smbfs}/link.c                     |   2 +-
>>>> fs/{cifs => smbfs}/misc.c                     |  14 +-
>>>> fs/{cifs => smbfs}/netlink.c                  |   0
>>>> fs/{cifs => smbfs}/netlink.h                  |   0
>>>> fs/{cifs => smbfs}/netmisc.c                  |   2 +-
>>>> fs/{cifs => smbfs}/nterr.c                    |   0
>>>> fs/{cifs => smbfs}/nterr.h                    |   0
>>>> fs/{cifs => smbfs}/ntlmssp.h                  |   2 +-
>>>> fs/{cifs => smbfs}/readdir.c                  |   4 +-
>>>> fs/{cifs => smbfs}/rfc1002pdu.h               |   0
>>>> fs/{cifs => smbfs}/sess.c                     |  10 +-
>>>> fs/{cifs => smbfs}/smb1ops.c                  |   4 +-
>>>> fs/{cifs => smbfs}/smb2file.c                 |   2 +-
>>>> fs/{cifs => smbfs}/smb2glob.h                 |   0
>>>> fs/{cifs => smbfs}/smb2inode.c                |   2 +-
>>>> fs/{cifs => smbfs}/smb2maperror.c             |   0
>>>> fs/{cifs => smbfs}/smb2misc.c                 |   0
>>>> fs/{cifs => smbfs}/smb2ops.c                  |  32 ++--
>>>> fs/{cifs => smbfs}/smb2pdu.c                  |  22 +--
>>>> fs/{cifs => smbfs}/smb2pdu.h                  |   0
>>>> fs/{cifs => smbfs}/smb2proto.h                |   0
>>>> fs/{cifs => smbfs}/smb2status.h               |   0
>>>> fs/{cifs => smbfs}/smb2transport.c            |   2 +-
>>>> fs/{cifs => smbfs}/smbdirect.c                |   0
>>>> fs/{cifs => smbfs}/smbdirect.h                |   2 +-
>>>> fs/{cifs => smbfs}/smbencrypt.c               |   0
>>>> fs/{cifs => smbfs}/smberr.h                   |   0
>>>> fs/{cifs/cifsfs.h => smbfs/smbfs.h}           |  12 +-
>>>> fs/{cifs => smbfs}/trace.c                    |   0
>>>> fs/{cifs => smbfs}/trace.h                    |   0
>>>> fs/{cifs => smbfs}/transport.c                |   4 +-
>>>> fs/{cifs => smbfs}/unc.c                      |   0
>>>> fs/{cifs => smbfs}/winucase.c                 |   0
>>>> fs/{cifs => smbfs}/xattr.c                    |  18 +-
>>>> 91 files changed, 414 insertions(+), 417 deletions(-)
>>>> rename Documentation/admin-guide/{cifs => smbfs}/authors.rst (100%)
>>>> rename Documentation/admin-guide/{cifs => smbfs}/changes.rst (73%)
>>>> rename Documentation/admin-guide/{cifs => smbfs}/index.rst (100%)
>>>> rename Documentation/admin-guide/{cifs => smbfs}/introduction.rst
>>>>(100%)
>>>> rename Documentation/admin-guide/{cifs => smbfs}/todo.rst (95%)
>>>> rename Documentation/admin-guide/{cifs => smbfs}/usage.rst (87%)
>>>> rename Documentation/admin-guide/{cifs => smbfs}/winucase_convert.pl
>>>>(100%)
>>>> rename Documentation/filesystems/{cifs => smbfs}/cifsroot.rst (85%)
>>>> rename Documentation/filesystems/{cifs => smbfs}/index.rst (100%)
>>>> rename Documentation/filesystems/{cifs => smbfs}/ksmbd.rst (99%)
>>>> delete mode 100644 fs/cifs/Makefile
>>>> rename fs/{cifs => smbfs}/Kconfig (72%)
>>>> create mode 100644 fs/smbfs/Makefile
>>>> rename fs/{cifs => smbfs}/asn1.c (100%)
>>>> rename fs/{cifs => smbfs}/cifs_debug.c (96%)
>>>> rename fs/{cifs => smbfs}/cifs_debug.h (98%)
>>>> rename fs/{cifs => smbfs}/cifs_dfs_ref.c (99%)
>>>> rename fs/{cifs => smbfs}/cifs_fs_sb.h (100%)
>>>> rename fs/{cifs => smbfs}/cifs_ioctl.h (100%)
>>>> rename fs/{cifs => smbfs}/cifs_spnego.c (98%)
>>>> rename fs/{cifs => smbfs}/cifs_spnego.h (100%)
>>>> rename fs/{cifs => smbfs}/cifs_spnego_negtokeninit.asn1 (100%)
>>>> rename fs/{cifs => smbfs}/cifs_swn.c (100%)
>>>> rename fs/{cifs => smbfs}/cifs_swn.h (95%)
>>>> rename fs/{cifs => smbfs}/cifs_unicode.c (100%)
>>>> rename fs/{cifs => smbfs}/cifs_unicode.h (100%)
>>>> rename fs/{cifs => smbfs}/cifs_uniupr.h (100%)
>>>> rename fs/{cifs => smbfs}/cifsacl.c (99%)
>>>> rename fs/{cifs => smbfs}/cifsacl.h (100%)
>>>> rename fs/{cifs => smbfs}/cifsencrypt.c (100%)
>>>> rename fs/{cifs => smbfs}/cifsglob.h (99%)
>>>> rename fs/{cifs => smbfs}/cifspdu.h (99%)
>>>> rename fs/{cifs => smbfs}/cifsproto.h (99%)
>>>> rename fs/{cifs => smbfs}/cifsroot.c (100%)
>>>> rename fs/{cifs => smbfs}/cifssmb.c (99%)
>>>> rename fs/{cifs => smbfs}/connect.c (99%)
>>>> rename fs/{cifs/cifsfs.c => smbfs/core.c} (98%)
>>>> rename fs/{cifs => smbfs}/dfs_cache.c (99%)
>>>> rename fs/{cifs => smbfs}/dfs_cache.h (100%)
>>>> rename fs/{cifs => smbfs}/dir.c (99%)
>>>> rename fs/{cifs => smbfs}/dns_resolve.c (100%)
>>>> rename fs/{cifs => smbfs}/dns_resolve.h (100%)
>>>> rename fs/{cifs => smbfs}/export.c (91%)
>>>> rename fs/{cifs => smbfs}/file.c (99%)
>>>> rename fs/{cifs => smbfs}/fs_context.c (99%)
>>>> rename fs/{cifs => smbfs}/fs_context.h (100%)
>>>> rename fs/{cifs => smbfs}/fscache.c (100%)
>>>> rename fs/{cifs => smbfs}/fscache.h (98%)
>>>> rename fs/{cifs => smbfs}/inode.c (99%)
>>>> rename fs/{cifs => smbfs}/ioctl.c (99%)
>>>> rename fs/{cifs => smbfs}/link.c (99%)
>>>> rename fs/{cifs => smbfs}/misc.c (99%)
>>>> rename fs/{cifs => smbfs}/netlink.c (100%)
>>>> rename fs/{cifs => smbfs}/netlink.h (100%)
>>>> rename fs/{cifs => smbfs}/netmisc.c (99%)
>>>> rename fs/{cifs => smbfs}/nterr.c (100%)
>>>> rename fs/{cifs => smbfs}/nterr.h (100%)
>>>> rename fs/{cifs => smbfs}/ntlmssp.h (98%)
>>>> rename fs/{cifs => smbfs}/readdir.c (99%)
>>>> rename fs/{cifs => smbfs}/rfc1002pdu.h (100%)
>>>> rename fs/{cifs => smbfs}/sess.c (99%)
>>>> rename fs/{cifs => smbfs}/smb1ops.c (99%)
>>>> rename fs/{cifs => smbfs}/smb2file.c (99%)
>>>> rename fs/{cifs => smbfs}/smb2glob.h (100%)
>>>> rename fs/{cifs => smbfs}/smb2inode.c (99%)
>>>> rename fs/{cifs => smbfs}/smb2maperror.c (100%)
>>>> rename fs/{cifs => smbfs}/smb2misc.c (100%)
>>>> rename fs/{cifs => smbfs}/smb2ops.c (99%)
>>>> rename fs/{cifs => smbfs}/smb2pdu.c (99%)
>>>> rename fs/{cifs => smbfs}/smb2pdu.h (100%)
>>>> rename fs/{cifs => smbfs}/smb2proto.h (100%)
>>>> rename fs/{cifs => smbfs}/smb2status.h (100%)
>>>> rename fs/{cifs => smbfs}/smb2transport.c (99%)
>>>> rename fs/{cifs => smbfs}/smbdirect.c (100%)
>>>> rename fs/{cifs => smbfs}/smbdirect.h (99%)
>>>> rename fs/{cifs => smbfs}/smbencrypt.c (100%)
>>>> rename fs/{cifs => smbfs}/smberr.h (100%)
>>>> rename fs/{cifs/cifsfs.h => smbfs/smbfs.h} (97%)
>>>> rename fs/{cifs => smbfs}/trace.c (100%)
>>>> rename fs/{cifs => smbfs}/trace.h (100%)
>>>> rename fs/{cifs => smbfs}/transport.c (99%)
>>>> rename fs/{cifs => smbfs}/unc.c (100%)
>>>> rename fs/{cifs => smbfs}/winucase.c (100%)
>>>> rename fs/{cifs => smbfs}/xattr.c (98%)
>>>>
>>>
>>

  parent reply	other threads:[~2022-08-02 13:52 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-01 19:09 [RFC PATCH 0/3] Rename "cifs" module to "smbfs" Enzo Matsumiya
2022-08-01 19:09 ` [RFC PATCH 1/3] cifs: change module name to "smbfs.ko" Enzo Matsumiya
2023-07-26 20:28   ` Steve French
2023-07-26 20:29     ` Steve French
2023-07-26 20:31       ` Steve French
2022-08-01 19:09 ` [RFC PATCH 2/3] smbfs: rename directory "fs/cifs" -> "fs/smbfs" Enzo Matsumiya
2022-08-01 19:09 ` [RFC PATCH 3/3] smbfs: update doc references Enzo Matsumiya
2022-08-01 19:58 ` [RFC PATCH 0/3] Rename "cifs" module to "smbfs" Rowland Penny
2022-08-01 20:14   ` Enzo Matsumiya
2022-08-01 20:27     ` Steve French
2022-08-01 20:41       ` Enzo Matsumiya
2022-08-02  0:38     ` Tom Talpey
2022-08-02  0:42       ` Steve French
2022-08-02 13:52       ` Enzo Matsumiya [this message]
2022-08-02 15:35         ` Steve French
2022-08-02 15:58           ` Enzo Matsumiya
2022-08-02 16:20             ` Steve French
2022-08-02 17:43             ` Tom Talpey
2022-08-02 14:42 ` Jeff Layton
2022-08-02 19:36   ` Enzo Matsumiya
2022-08-02 20:07     ` Jeff Layton
2022-08-03  1:32       ` Tom Talpey
2022-08-03  1:56         ` Enzo Matsumiya
2022-08-04 19:03           ` Jeff Layton
2022-08-04 20:23             ` Matthew Wilcox
2022-08-04 20:48               ` Jeff Layton
2022-08-03  5:38         ` Steve French
2022-08-03 14:45       ` Enzo Matsumiya
2022-08-03 17:50         ` Paulo Alcantara

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220802135201.4vm36drd5mp57nvv@cyberdelia \
    --to=ematsumiya@suse.de \
    --cc=linux-cifs@vger.kernel.org \
    --cc=rpenny@samba.org \
    --cc=tom@talpey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox