[v1,07/14] cifs: auto disable 'serverino' in dfs mounts

Message ID 20181115142103.24617-8-aaptel@suse.com
State New
Headers show
Series
  • DFS failover
Related show

Commit Message

Aurélien Aptel Nov. 15, 2018, 2:20 p.m.
From: Paulo Alcantara <paulo@paulo.ac>

Different servers have different set of file ids.

After failover, unique IDs will be different so we can't validate
them.

Signed-off-by: Aurelien Aptel <aaptel@suse.com>
---
 fs/cifs/connect.c |  6 ++++++
 fs/cifs/inode.c   | 42 +++++++++++++++++++-----------------------
 fs/cifs/misc.c    | 12 ++++++++++--
 3 files changed, 35 insertions(+), 25 deletions(-)

Comments

Steve French Dec. 23, 2018, 9:21 p.m. | #1
What if we hashed the server or the tcon->vol serial number
(vol->create_time might also be unique in a reasonable server
implementation)?

On Thu, Nov 15, 2018 at 8:21 AM Aurelien Aptel <aaptel@suse.com> wrote:
>
> From: Paulo Alcantara <paulo@paulo.ac>
>
> Different servers have different set of file ids.
>
> After failover, unique IDs will be different so we can't validate
> them.
>
> Signed-off-by: Aurelien Aptel <aaptel@suse.com>
> ---
>  fs/cifs/connect.c |  6 ++++++
>  fs/cifs/inode.c   | 42 +++++++++++++++++++-----------------------
>  fs/cifs/misc.c    | 12 ++++++++++--
>  3 files changed, 35 insertions(+), 25 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 7022678be860..86649230b4f2 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -4219,6 +4219,12 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb_vol *vol)
>         if (rc)
>                 goto error;
>
> +       /*
> +        * After reconnecting to a different server, unique ids won't
> +        * match anymore, so we disable serverino. This prevents
> +        * dentry revalidation to think the dentry are stale (ESTALE).
> +        */
> +       cifs_autodisable_serverino(cifs_sb);
>  out:
>         free_xid(xid);
>         return mount_setup_tlink(cifs_sb, ses, tcon);
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index 44a76e26f21f..8bb7b1b45f74 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -730,7 +730,6 @@ cifs_get_inode_info(struct inode **inode, const char *full_path,
>                     FILE_ALL_INFO *data, struct super_block *sb, int xid,
>                     const struct cifs_fid *fid)
>  {
> -       bool validinum = false;
>         __u16 srchflgs;
>         int rc = 0, tmprc = ENOSYS;
>         struct cifs_tcon *tcon;
> @@ -824,7 +823,6 @@ cifs_get_inode_info(struct inode **inode, const char *full_path,
>                         (FILE_DIRECTORY_INFO *)data, cifs_sb);
>                         fattr.cf_uniqueid = le64_to_cpu(
>                         ((SEARCH_ID_FULL_DIR_INFO *)data)->UniqueId);
> -                       validinum = true;
>
>                         cifs_buf_release(srchinf->ntwrk_buf_start);
>                 }
> @@ -843,31 +841,29 @@ cifs_get_inode_info(struct inode **inode, const char *full_path,
>          */
>         if (*inode == NULL) {
>                 if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM) {
> -                       if (validinum == false) {
> -                               if (server->ops->get_srv_inum)
> -                                       tmprc = server->ops->get_srv_inum(xid,
> -                                               tcon, cifs_sb, full_path,
> -                                               &fattr.cf_uniqueid, data);
> -                               if (tmprc) {
> -                                       cifs_dbg(FYI, "GetSrvInodeNum rc %d\n",
> -                                                tmprc);
> -                                       fattr.cf_uniqueid = iunique(sb, ROOT_I);
> -                                       cifs_autodisable_serverino(cifs_sb);
> -                               } else if ((fattr.cf_uniqueid == 0) &&
> -                                               strlen(full_path) == 0) {
> -                                       /* some servers ret bad root ino ie 0 */
> -                                       cifs_dbg(FYI, "Invalid (0) inodenum\n");
> -                                       fattr.cf_flags |=
> -                                               CIFS_FATTR_FAKE_ROOT_INO;
> -                                       fattr.cf_uniqueid =
> -                                               simple_hashstr(tcon->treeName);
> -                               }
> +                       if (server->ops->get_srv_inum)
> +                               tmprc = server->ops->get_srv_inum(xid,
> +                                                                 tcon, cifs_sb, full_path,
> +                                                                 &fattr.cf_uniqueid, data);
> +                       if (tmprc) {
> +                               cifs_dbg(FYI, "GetSrvInodeNum rc %d\n",
> +                                        tmprc);
> +                               fattr.cf_uniqueid = iunique(sb, ROOT_I);
> +                               cifs_autodisable_serverino(cifs_sb);
> +                       } else if ((fattr.cf_uniqueid == 0) &&
> +                                  strlen(full_path) == 0) {
> +                               /* some servers ret bad root ino ie 0 */
> +                               cifs_dbg(FYI, "Invalid (0) inodenum\n");
> +                               fattr.cf_flags |=
> +                                       CIFS_FATTR_FAKE_ROOT_INO;
> +                               fattr.cf_uniqueid =
> +                                       simple_hashstr(tcon->treeName);
>                         }
>                 } else
>                         fattr.cf_uniqueid = iunique(sb, ROOT_I);
>         } else {
> -               if ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM) &&
> -                   validinum == false && server->ops->get_srv_inum) {
> +               if ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)
> +                   && server->ops->get_srv_inum) {
>                         /*
>                          * Pass a NULL tcon to ensure we don't make a round
>                          * trip to the server. This only works for SMB2+.
> diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
> index 7d8d9423d5ba..5e315e4009e2 100644
> --- a/fs/cifs/misc.c
> +++ b/fs/cifs/misc.c
> @@ -525,9 +525,17 @@ void
>  cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb)
>  {
>         if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM) {
> +               struct cifs_tcon *tcon = NULL;
> +
> +               if (cifs_sb->master_tlink)
> +                       tcon = cifs_sb_master_tcon(cifs_sb);
> +
>                 cifs_sb->mnt_cifs_flags &= ~CIFS_MOUNT_SERVER_INUM;
> -               cifs_dbg(VFS, "Autodisabling the use of server inode numbers on %s. This server doesn't seem to support them properly. Hardlinks will not be recognized on this mount. Consider mounting with the \"noserverino\" option to silence this message.\n",
> -                        cifs_sb_master_tcon(cifs_sb)->treeName);
> +               cifs_dbg(VFS, "Autodisabling the use of server inode numbers on %s.\n",
> +                        tcon ? tcon->treeName : "new server");
> +               cifs_dbg(VFS, "The server doesn't seem to support them properly or the files might be on different servers (DFS).\n");
> +               cifs_dbg(VFS, "Hardlinks will not be recognized on this mount. Consider mounting with the \"noserverino\" option to silence this message.\n");
> +
>         }
>  }
>
> --
> 2.13.7
>

Patch

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 7022678be860..86649230b4f2 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -4219,6 +4219,12 @@  int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb_vol *vol)
 	if (rc)
 		goto error;
 
+	/*
+	 * After reconnecting to a different server, unique ids won't
+	 * match anymore, so we disable serverino. This prevents
+	 * dentry revalidation to think the dentry are stale (ESTALE).
+	 */
+	cifs_autodisable_serverino(cifs_sb);
 out:
 	free_xid(xid);
 	return mount_setup_tlink(cifs_sb, ses, tcon);
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 44a76e26f21f..8bb7b1b45f74 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -730,7 +730,6 @@  cifs_get_inode_info(struct inode **inode, const char *full_path,
 		    FILE_ALL_INFO *data, struct super_block *sb, int xid,
 		    const struct cifs_fid *fid)
 {
-	bool validinum = false;
 	__u16 srchflgs;
 	int rc = 0, tmprc = ENOSYS;
 	struct cifs_tcon *tcon;
@@ -824,7 +823,6 @@  cifs_get_inode_info(struct inode **inode, const char *full_path,
 			(FILE_DIRECTORY_INFO *)data, cifs_sb);
 			fattr.cf_uniqueid = le64_to_cpu(
 			((SEARCH_ID_FULL_DIR_INFO *)data)->UniqueId);
-			validinum = true;
 
 			cifs_buf_release(srchinf->ntwrk_buf_start);
 		}
@@ -843,31 +841,29 @@  cifs_get_inode_info(struct inode **inode, const char *full_path,
 	 */
 	if (*inode == NULL) {
 		if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM) {
-			if (validinum == false) {
-				if (server->ops->get_srv_inum)
-					tmprc = server->ops->get_srv_inum(xid,
-						tcon, cifs_sb, full_path,
-						&fattr.cf_uniqueid, data);
-				if (tmprc) {
-					cifs_dbg(FYI, "GetSrvInodeNum rc %d\n",
-						 tmprc);
-					fattr.cf_uniqueid = iunique(sb, ROOT_I);
-					cifs_autodisable_serverino(cifs_sb);
-				} else if ((fattr.cf_uniqueid == 0) &&
-						strlen(full_path) == 0) {
-					/* some servers ret bad root ino ie 0 */
-					cifs_dbg(FYI, "Invalid (0) inodenum\n");
-					fattr.cf_flags |=
-						CIFS_FATTR_FAKE_ROOT_INO;
-					fattr.cf_uniqueid =
-						simple_hashstr(tcon->treeName);
-				}
+			if (server->ops->get_srv_inum)
+				tmprc = server->ops->get_srv_inum(xid,
+								  tcon, cifs_sb, full_path,
+								  &fattr.cf_uniqueid, data);
+			if (tmprc) {
+				cifs_dbg(FYI, "GetSrvInodeNum rc %d\n",
+					 tmprc);
+				fattr.cf_uniqueid = iunique(sb, ROOT_I);
+				cifs_autodisable_serverino(cifs_sb);
+			} else if ((fattr.cf_uniqueid == 0) &&
+				   strlen(full_path) == 0) {
+				/* some servers ret bad root ino ie 0 */
+				cifs_dbg(FYI, "Invalid (0) inodenum\n");
+				fattr.cf_flags |=
+					CIFS_FATTR_FAKE_ROOT_INO;
+				fattr.cf_uniqueid =
+					simple_hashstr(tcon->treeName);
 			}
 		} else
 			fattr.cf_uniqueid = iunique(sb, ROOT_I);
 	} else {
-		if ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM) &&
-		    validinum == false && server->ops->get_srv_inum) {
+		if ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)
+		    && server->ops->get_srv_inum) {
 			/*
 			 * Pass a NULL tcon to ensure we don't make a round
 			 * trip to the server. This only works for SMB2+.
diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
index 7d8d9423d5ba..5e315e4009e2 100644
--- a/fs/cifs/misc.c
+++ b/fs/cifs/misc.c
@@ -525,9 +525,17 @@  void
 cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb)
 {
 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM) {
+		struct cifs_tcon *tcon = NULL;
+
+		if (cifs_sb->master_tlink)
+			tcon = cifs_sb_master_tcon(cifs_sb);
+
 		cifs_sb->mnt_cifs_flags &= ~CIFS_MOUNT_SERVER_INUM;
-		cifs_dbg(VFS, "Autodisabling the use of server inode numbers on %s. This server doesn't seem to support them properly. Hardlinks will not be recognized on this mount. Consider mounting with the \"noserverino\" option to silence this message.\n",
-			 cifs_sb_master_tcon(cifs_sb)->treeName);
+		cifs_dbg(VFS, "Autodisabling the use of server inode numbers on %s.\n",
+			 tcon ? tcon->treeName : "new server");
+		cifs_dbg(VFS, "The server doesn't seem to support them properly or the files might be on different servers (DFS).\n");
+		cifs_dbg(VFS, "Hardlinks will not be recognized on this mount. Consider mounting with the \"noserverino\" option to silence this message.\n");
+
 	}
 }