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@xxxxxxxx> wrote:
>
> From: Paulo Alcantara <paulo@xxxxxxxx>
>
> 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@xxxxxxxx>
> ---
> 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
>
--
Thanks,
Steve
