cifs: flush before set-info if we have writeable handles
diff mbox series

Message ID 20190716032349.10711-1-lsahlber@redhat.com
State New
Headers show
Series
  • cifs: flush before set-info if we have writeable handles
Related show

Commit Message

Ronnie Sahlberg July 16, 2019, 3:23 a.m. UTC
Servers can defer destaging any data and updating the mtime until close().
This means that if we do a setinfo to modify the mtime while other handles
are open for write the server may overwrite our setinfo timestamps when
if flushes the file on close() of the writeable handle.

To avoid this we add an explicit flush() before any attempts to use setinfo
and update the mtime IF we have writeable handles open.

This makes "cp -p" preserve the mtime when copying files to some smb servers.

Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/smb2inode.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

Comments

Pavel Shilovsky July 17, 2019, 6:44 p.m. UTC | #1
пн, 15 июл. 2019 г. в 20:24, Ronnie Sahlberg <lsahlber@redhat.com>:
>
> Servers can defer destaging any data and updating the mtime until close().
> This means that if we do a setinfo to modify the mtime while other handles
> are open for write the server may overwrite our setinfo timestamps when
> if flushes the file on close() of the writeable handle.
>
> To avoid this we add an explicit flush() before any attempts to use setinfo
> and update the mtime IF we have writeable handles open.
>
> This makes "cp -p" preserve the mtime when copying files to some smb servers.
>
> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> ---
>  fs/cifs/smb2inode.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c
> index 278405d26c47..a3603ec3a086 100644
> --- a/fs/cifs/smb2inode.c
> +++ b/fs/cifs/smb2inode.c
> @@ -516,7 +516,11 @@ smb2_set_file_info(struct inode *inode, const char *full_path,
>  {
>         struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
>         struct tcon_link *tlink;
> +       struct cifs_tcon *tcon;
> +       struct TCP_Server_Info *server;
>         int rc;
> +       struct cifsInodeInfo *cifsi;
> +       struct cifsFileInfo *wrcfile;
>
>         if ((buf->CreationTime == 0) && (buf->LastAccessTime == 0) &&
>             (buf->LastWriteTime == 0) && (buf->ChangeTime == 0) &&
> @@ -527,7 +531,19 @@ smb2_set_file_info(struct inode *inode, const char *full_path,
>         if (IS_ERR(tlink))
>                 return PTR_ERR(tlink);
>
> -       rc = smb2_compound_op(xid, tlink_tcon(tlink), cifs_sb, full_path,
> +       tcon = tlink_tcon(tlink);
> +       server = tcon->ses->server;
> +
> +       if (buf->LastWriteTime) {
> +               cifsi = CIFS_I(inode);
> +               wrcfile = find_writable_file(cifsi, false);

use get_writable_file instead - this will allow us to get a real RC.
if -EBADF is returned then no writable handle and should proceed
normally, otherwise let's fail setattr.

> +               if (wrcfile) {
> +                       filemap_write_and_wait(inode->i_mapping);

If the above function returns error, the same problem that you are
trying to fix appears. If an error happen, we should fail setattr to
avoid corruption of timestamps.

> +                       server->ops->flush(xid, tcon, &wrcfile->fid);

the same logic applies to flush call - need to fail setattr if an error occured.

> +                       cifsFileInfo_put(wrcfile);
> +               }
> +       }
> +       rc = smb2_compound_op(xid, tcon, cifs_sb, full_path,
>                               FILE_WRITE_ATTRIBUTES, FILE_OPEN, 0, buf,
>                               SMB2_OP_SET_INFO);
>         cifs_put_tlink(tlink);
> --
> 2.13.6
>


--
Best regards,
Pavel Shilovsky

Patch
diff mbox series

diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c
index 278405d26c47..a3603ec3a086 100644
--- a/fs/cifs/smb2inode.c
+++ b/fs/cifs/smb2inode.c
@@ -516,7 +516,11 @@  smb2_set_file_info(struct inode *inode, const char *full_path,
 {
 	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
 	struct tcon_link *tlink;
+	struct cifs_tcon *tcon;
+	struct TCP_Server_Info *server;
 	int rc;
+	struct cifsInodeInfo *cifsi;
+	struct cifsFileInfo *wrcfile;
 
 	if ((buf->CreationTime == 0) && (buf->LastAccessTime == 0) &&
 	    (buf->LastWriteTime == 0) && (buf->ChangeTime == 0) &&
@@ -527,7 +531,19 @@  smb2_set_file_info(struct inode *inode, const char *full_path,
 	if (IS_ERR(tlink))
 		return PTR_ERR(tlink);
 
-	rc = smb2_compound_op(xid, tlink_tcon(tlink), cifs_sb, full_path,
+	tcon = tlink_tcon(tlink);
+	server = tcon->ses->server;
+
+	if (buf->LastWriteTime) {
+		cifsi = CIFS_I(inode);
+		wrcfile = find_writable_file(cifsi, false);
+		if (wrcfile) {
+			filemap_write_and_wait(inode->i_mapping);
+			server->ops->flush(xid, tcon, &wrcfile->fid);
+			cifsFileInfo_put(wrcfile);
+		}
+	}
+	rc = smb2_compound_op(xid, tcon, cifs_sb, full_path,
 			      FILE_WRITE_ATTRIBUTES, FILE_OPEN, 0, buf,
 			      SMB2_OP_SET_INFO);
 	cifs_put_tlink(tlink);