| Message ID | 20181115142103.24617-8-aaptel@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DFS failover | expand |
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 >
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"); + } }