As suggested by Jeff Layton:
--------
"Now that I look though, it's clear to me that cifs_set_file_size is
just wrong. Currently it calls ops->set_path_size and if that fails
with certain error codes it tries to do a SMBLegacyOpen, etc. That's
going to fall on its face if this is a SMB2/3 connection.
Here's what should be done instead, I think:
- get rid of both set_path_size and set_file_size operations. The
version specific abstraction was implemented at the wrong level, IMO.
- implement a new protocol level operation that basically takes the
same args as cifs_set_file_size. cifs_setattr_unix/_nounix should
call this operation.
- the set_path_size operation for SMB1 should basically be the function
that is cifs_set_file_size today (though that should probably be
renamed). That function should be restructured to do the following:
+ look for an open filehandle
+ if there isn't one, open the file
+ call CIFSSMBSetFileSize
+ fall back to zero-length write if that fails
+ close the file if we opened it"
--------
This patch also moves all of the SMBv1 legacy hacks from cifs_set_file_size() into
the SMBv1 specific handler smb_set_file_size() more or less intact. SMBv2 and higher are
more regular and orthogonal, so the v2 handler smb2_set_file_size() no longer contains
the legacy hacks.
Move cifsFileInfo_put(open_file) below the first call to
cifs_legacy_set_file_size() so that the reference on open_file stays valid
throughout.
Cc: Steve French <sfrench@xxxxxxxxx>
Cc: Jeff Layton <jlayton@xxxxxxxxxx>
Cc: Dean Gehnert <deang@xxxxxxx>
Signed-off-by: Tim Gardner <timg@xxxxxxx>
---
V3 - dropped 'cifs: fix incorrect reference count check' which impacted this patch.
Also moved cifsFileInfo_put(open_file) according to V2 review comments from Jeff Layton.
V2 - this is a new patch in the V2 series.
I know this is kind of a giant patch, but there really doesn't seem to be any
intermediate steps. Its pretty much all or nothing.
I've only tested against a Linux CIFS server running a 3.2 kernel since I no longer
have easy access to smbv2 servers (iOS 10.8 or Windows 7).
fs/cifs/cifsglob.h | 9 ++---
fs/cifs/inode.c | 108 +++++----------------------------------------------
fs/cifs/smb1ops.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++++++-
fs/cifs/smb2ops.c | 57 ++++++++++++++++++++++++---
4 files changed, 170 insertions(+), 113 deletions(-)
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 8fd8eb2..6113ce8 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -269,12 +269,9 @@ struct smb_version_operations {
int (*get_srv_inum)(const unsigned int, struct cifs_tcon *,
struct cifs_sb_info *, const char *,
u64 *uniqueid, FILE_ALL_INFO *);
- /* set size by path */
- int (*set_file_size_by_path)(const unsigned int, struct cifs_tcon *,
- const char *, __u64, struct cifs_sb_info *, bool);
- /* set size by file handle */
- int (*set_file_size_by_handle)(const unsigned int, struct cifs_tcon *,
- struct cifsFileInfo *, __u64, bool);
+ /* set file size */
+ int (*set_file_size)(struct inode *inode, struct iattr *attrs,
+ unsigned int xid, char *full_path);
/* set attributes */
int (*set_file_info)(struct inode *, const char *, FILE_BASIC_INFO *,
const unsigned int);
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 3f710c6..3edeafd 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -1899,118 +1899,28 @@ static void cifs_setsize(struct inode *inode, loff_t offset)
truncate_pagecache(inode, offset);
}
-/*
- * Legacy hack to set file length.
- */
-static int
-cifs_legacy_set_file_size(unsigned int xid, __u16 netfid, __u32 pid,
- unsigned int length, struct cifs_tcon *tcon,
- char *full_path)
-{
- int rc;
- unsigned int bytes_written;
- struct cifs_io_parms io_parms;
-
- io_parms.netfid = netfid;
- io_parms.pid = pid;
- io_parms.tcon = tcon;
- io_parms.offset = 0;
- io_parms.length = length;
- rc = CIFSSMBWrite(xid, &io_parms, &bytes_written,
- NULL, NULL, 1);
- cifs_dbg(FYI, "%s len %d rc %d on %s\n", __func__, length, rc,
- full_path);
- return rc;
-}
-
static int
cifs_set_file_size(struct inode *inode, struct iattr *attrs,
unsigned int xid, char *full_path)
{
- int rc;
- struct cifsFileInfo *open_file;
+ int rc = -ENOSYS;
struct cifsInodeInfo *cifsInode = CIFS_I(inode);
struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
struct tcon_link *tlink = NULL;
struct cifs_tcon *tcon = NULL;
struct TCP_Server_Info *server;
- /*
- * To avoid spurious oplock breaks from server, in the case of
- * inodes that we already have open, avoid doing path based
- * setting of file size if we can do it by handle.
- * This keeps our caching token (oplock) and avoids timeouts
- * when the local oplock break takes longer to flush
- * writebehind data than the SMB timeout for the SetPathInfo
- * request would allow
- */
- open_file = find_writable_file(cifsInode, true);
- if (open_file) {
- tcon = tlink_tcon(open_file->tlink);
- server = tcon->ses->server;
- if (server->ops->set_file_size_by_handle)
- rc = server->ops->set_file_size_by_handle(xid, tcon,
- open_file,
- attrs->ia_size,
- false);
- else
- rc = -ENOSYS;
- cifsFileInfo_put(open_file);
- cifs_dbg(FYI, "SetFSize for attrs rc = %d\n", rc);
- if ((rc == -EINVAL) || (rc == -EOPNOTSUPP)) {
- rc = cifs_legacy_set_file_size(xid,
- open_file->fid.netfid,
- open_file->pid,
- attrs->ia_size,
- tcon, full_path);
- }
- } else
- rc = -EINVAL;
-
- if (!rc)
- goto set_size_out;
+ tlink = cifs_sb_tlink(cifs_sb);
+ if (IS_ERR(tlink))
+ return PTR_ERR(tlink);
+ tcon = tlink_tcon(tlink);
+ server = tcon->ses->server;
- if (tcon == NULL) {
- tlink = cifs_sb_tlink(cifs_sb);
- if (IS_ERR(tlink))
- return PTR_ERR(tlink);
- tcon = tlink_tcon(tlink);
- server = tcon->ses->server;
- }
+ if (server->ops->set_file_size)
+ rc = server->ops->set_file_size(inode, attrs, xid, full_path);
- /*
- * Set file size by pathname rather than by handle either because no
- * valid, writeable file handle for it was found or because there was
- * an error setting it by handle.
- */
- if (server->ops->set_file_size_by_path)
- rc = server->ops->set_file_size_by_path(xid, tcon, full_path,
- attrs->ia_size, cifs_sb,
- false);
- else
- rc = -ENOSYS;
- cifs_dbg(FYI, "SetEOF by path (setattrs) rc = %d\n", rc);
- if ((rc == -EINVAL) || (rc == -EOPNOTSUPP)) {
- __u16 netfid;
- int oplock = 0;
-
- rc = SMBLegacyOpen(xid, tcon, full_path, FILE_OPEN,
- GENERIC_WRITE, CREATE_NOT_DIR, &netfid,
- &oplock, NULL, cifs_sb->local_nls,
- cifs_sb->mnt_cifs_flags &
- CIFS_MOUNT_MAP_SPECIAL_CHR);
- if (rc == 0) {
- rc = cifs_legacy_set_file_size(xid, netfid,
- current->tgid,
- attrs->ia_size, tcon,
- full_path);
- CIFSSMBClose(xid, tcon, netfid);
- }
- }
- if (tlink)
- cifs_put_tlink(tlink);
+ cifs_put_tlink(tlink);
-set_size_out:
if (rc == 0) {
cifsInode->server_eof = attrs->ia_size;
cifs_setsize(inode, attrs->ia_size);
diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index c5d9ec6..fd0c124 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -755,6 +755,30 @@ cifs_sync_write(const unsigned int xid, struct cifsFileInfo *cfile,
return CIFSSMBWrite2(xid, parms, written, iov, nr_segs);
}
+/*
+ * Legacy hack to set file length.
+ */
+static int
+cifs_legacy_set_file_size(unsigned int xid, __u16 netfid, __u32 pid,
+ unsigned int length, struct cifs_tcon *tcon,
+ char *full_path)
+{
+ int rc;
+ unsigned int bytes_written;
+ struct cifs_io_parms io_parms;
+
+ io_parms.netfid = netfid;
+ io_parms.pid = pid;
+ io_parms.tcon = tcon;
+ io_parms.offset = 0;
+ io_parms.length = length;
+ rc = CIFSSMBWrite(xid, &io_parms, &bytes_written,
+ NULL, NULL, 1);
+ cifs_dbg(FYI, "%s len %d rc %d on %s\n", __func__, length, rc,
+ full_path);
+ return rc;
+}
+
static int
smb_set_file_size_by_path(const unsigned int xid, struct cifs_tcon *tcon,
const char *full_path, __u64 size, struct cifs_sb_info *cifs_sb,
@@ -790,6 +814,88 @@ smb_set_file_size_by_path(const unsigned int xid, struct cifs_tcon *tcon,
}
static int
+smb_set_file_size(struct inode *inode, struct iattr *attrs, unsigned int xid,
+ char *full_path)
+{
+ int rc;
+ struct cifsFileInfo *open_file;
+ struct cifsInodeInfo *cifsInode = CIFS_I(inode);
+ struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
+ struct tcon_link *tlink = NULL;
+ struct cifs_tcon *tcon = NULL;
+ struct TCP_Server_Info *server;
+
+ /*
+ * To avoid spurious oplock breaks from server, in the case of
+ * inodes that we already have open, avoid doing path based
+ * setting of file size if we can do it by handle.
+ * This keeps our caching token (oplock) and avoids timeouts
+ * when the local oplock break takes longer to flush
+ * writebehind data than the SMB timeout for the SetPathInfo
+ * request would allow
+ */
+ open_file = find_writable_file(cifsInode, true);
+ if (open_file) {
+ tcon = tlink_tcon(open_file->tlink);
+ server = tcon->ses->server;
+ rc = CIFSSMBSetFileSize(xid, tcon, open_file, attrs->ia_size,
+ false);
+ cifs_dbg(FYI, "%s by handle rc = %d for %s\n", __func__, rc,
+ full_path);
+ if ((rc == -EINVAL) || (rc == -EOPNOTSUPP))
+ rc = cifs_legacy_set_file_size(xid,
+ open_file->fid.netfid,
+ open_file->pid,
+ attrs->ia_size,
+ tcon, full_path);
+ cifsFileInfo_put(open_file);
+ } else
+ rc = -EINVAL;
+
+ if (!rc)
+ goto set_size_out;
+
+ if (tcon == NULL) {
+ tlink = cifs_sb_tlink(cifs_sb);
+ if (IS_ERR(tlink))
+ return PTR_ERR(tlink);
+ tcon = tlink_tcon(tlink);
+ server = tcon->ses->server;
+ }
+
+ /*
+ * Set file size by pathname rather than by handle either because no
+ * valid, writeable file handle for it was found or because there was
+ * an error setting it by handle.
+ */
+ rc = smb_set_file_size_by_path(xid, tcon, full_path, attrs->ia_size,
+ cifs_sb, false);
+ cifs_dbg(FYI, "%s by path rc = %d for %s\n", __func__, rc, full_path);
+ if ((rc == -EINVAL) || (rc == -EOPNOTSUPP)) {
+ __u16 netfid;
+ int oplock = 0;
+
+ rc = SMBLegacyOpen(xid, tcon, full_path, FILE_OPEN,
+ GENERIC_WRITE, CREATE_NOT_DIR, &netfid,
+ &oplock, NULL, cifs_sb->local_nls,
+ cifs_sb->mnt_cifs_flags &
+ CIFS_MOUNT_MAP_SPECIAL_CHR);
+ if (rc == 0) {
+ rc = cifs_legacy_set_file_size(xid, netfid,
+ current->tgid,
+ attrs->ia_size, tcon,
+ full_path);
+ CIFSSMBClose(xid, tcon, netfid);
+ }
+ }
+ if (tlink)
+ cifs_put_tlink(tlink);
+
+set_size_out:
+ return rc;
+}
+
+static int
smb_set_file_info(struct inode *inode, const char *full_path,
FILE_BASIC_INFO *buf, const unsigned int xid)
{
@@ -1013,8 +1119,7 @@ struct smb_version_operations smb1_operations = {
.query_path_info = cifs_query_path_info,
.query_file_info = cifs_query_file_info,
.get_srv_inum = cifs_get_srv_inum,
- .set_file_size_by_path = smb_set_file_size_by_path,
- .set_file_size_by_handle = CIFSSMBSetFileSize,
+ .set_file_size = smb_set_file_size,
.set_file_info = smb_set_file_info,
.set_compression = cifs_set_compression,
.echo = CIFSSMBEcho,
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index dc7b1cb..206b45f 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -697,6 +697,54 @@ smb2_set_file_size_by_handle(const unsigned int xid, struct cifs_tcon *tcon,
}
static int
+smb2_set_file_size(struct inode *inode, struct iattr *attrs, unsigned int xid,
+ char *full_path)
+{
+ int rc;
+ struct cifsFileInfo *open_file;
+ struct cifsInodeInfo *cifsInode = CIFS_I(inode);
+ struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
+ struct tcon_link *tlink;
+ struct cifs_tcon *tcon;
+ struct TCP_Server_Info *server;
+
+ /*
+ * To avoid spurious oplock breaks from server, in the case of
+ * inodes that we already have open, avoid doing path based
+ * setting of file size if we can do it by handle.
+ * This keeps our caching token (oplock) and avoids timeouts
+ * when the local oplock break takes longer to flush
+ * writebehind data than the SMB timeout for the SetPathInfo
+ * request would allow
+ */
+ open_file = find_writable_file(cifsInode, true);
+ if (open_file) {
+ tcon = tlink_tcon(open_file->tlink);
+ server = tcon->ses->server;
+ rc = smb2_set_file_size_by_handle(xid, tcon, open_file,
+ attrs->ia_size, false);
+ cifsFileInfo_put(open_file);
+ cifs_dbg(FYI, "%s by handle rc = %d on %s\n", __func__, rc,
+ full_path);
+ return rc;
+ }
+
+ tlink = cifs_sb_tlink(cifs_sb);
+ if (IS_ERR(tlink))
+ return PTR_ERR(tlink);
+ tcon = tlink_tcon(tlink);
+ server = tcon->ses->server;
+
+ rc = smb2_set_file_size_by_path(xid, tcon, full_path, attrs->ia_size,
+ cifs_sb, false);
+ cifs_dbg(FYI, "%s by path rc = %d on %s\n", __func__, rc, full_path);
+
+ cifs_put_tlink(tlink);
+
+ return rc;
+}
+
+static int
smb2_set_compression(const unsigned int xid, struct cifs_tcon *tcon,
struct cifsFileInfo *cfile)
{
@@ -1131,8 +1179,7 @@ struct smb_version_operations smb20_operations = {
.query_path_info = smb2_query_path_info,
.get_srv_inum = smb2_get_srv_inum,
.query_file_info = smb2_query_file_info,
- .set_file_size_by_path = smb2_set_file_size_by_path,
- .set_file_size_by_handle = smb2_set_file_size_by_handle,
+ .set_file_size = smb2_set_file_size,
.set_file_info = smb2_set_file_info,
.set_compression = smb2_set_compression,
.mkdir = smb2_mkdir,
@@ -1205,8 +1252,7 @@ struct smb_version_operations smb21_operations = {
.query_path_info = smb2_query_path_info,
.get_srv_inum = smb2_get_srv_inum,
.query_file_info = smb2_query_file_info,
- .set_file_size_by_path = smb2_set_file_size_by_path,
- .set_file_size_by_handle = smb2_set_file_size_by_handle,
+ .set_file_size = smb2_set_file_size,
.set_file_info = smb2_set_file_info,
.set_compression = smb2_set_compression,
.mkdir = smb2_mkdir,
@@ -1280,8 +1326,7 @@ struct smb_version_operations smb30_operations = {
.query_path_info = smb2_query_path_info,
.get_srv_inum = smb2_get_srv_inum,
.query_file_info = smb2_query_file_info,
- .set_file_size_by_path = smb2_set_file_size_by_path,
- .set_file_size_by_handle = smb2_set_file_size_by_handle,
+ .set_file_size = smb2_set_file_size,
.set_file_info = smb2_set_file_info,
.set_compression = smb2_set_compression,
.mkdir = smb2_mkdir,
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
