diff mbox

[[PATCH,v2,CIFS] Allow setting per-file compression via SMB2 and SMB3

Message ID CAH2r5mubgNp_gY=0b_dWuQx2X2S1hESz5iKXwNhGZfXwCM7TsA@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Steve French Oct. 8, 2013, 8:41 p.m. UTC
Allow cifs/smb2/smb3 to return whether or not a file is compressed
via lsattr, and allow SMB2/SMB3 to set the per-file compression
flag ("chattr +c filename" on an smb2 or smb3 mount).

Windows users often set the compressed flag (it can be
done from the desktop and file manager).  David Disseldorp
has patches to Samba server to support this (at least on btrfs)
which are complementary to this

Signed-off-by: Steve French <smfrench@gmail.com>
---
 fs/cifs/cifsglob.h  |  2 ++
 fs/cifs/ioctl.c     | 55 +++++++++++++++++++++++++++++++++++------------------
 fs/cifs/smb2ops.c   | 11 +++++++++++
 fs/cifs/smb2pdu.c   | 33 ++++++++++++++++++++++++++++++--
 fs/cifs/smb2pdu.h   |  9 ++++++++-
 fs/cifs/smb2proto.h |  2 ++
 6 files changed, 91 insertions(+), 21 deletions(-)

      const __u8 oplock_level);

Comments

David Disseldorp Oct. 8, 2013, 10:25 p.m. UTC | #1
Hi Steve,

On Tue, 8 Oct 2013 15:41:17 -0500
Steve French <smfrench@gmail.com> wrote:

> +			/* Currently only flag we can set is compressed flag */
> +			if ((ExtAttrBits & FS_COMPR_FL) == 0)
> +				break;
> +
> +			/* Try to set compress flag */
> +			if (tcon->ses->server->ops->set_compression) {
> +				rc = tcon->ses->server->ops->set_compression(
> +							xid, tcon, pSMBFile);
> +				cifs_dbg(FYI, "set compress flag rc %d\n", rc);
>  			}

Is the FileFsAttributeInformation known at this stage? If so, you may
wish to check for the FILE_FILE_COMPRESSION capability, and avoid the
request dispatch if absent.

IIRC, Explorer hides the compression check-checkbox if the capability
is missing.
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steve French Oct. 8, 2013, 10:53 p.m. UTC | #2
On Tue, Oct 8, 2013 at 5:25 PM, David Disseldorp <ddiss@suse.de> wrote:
> Hi Steve,
>
> On Tue, 8 Oct 2013 15:41:17 -0500
> Steve French <smfrench@gmail.com> wrote:
>
>> +                     /* Currently only flag we can set is compressed flag */
>> +                     if ((ExtAttrBits & FS_COMPR_FL) == 0)
>> +                             break;
>> +
>> +                     /* Try to set compress flag */
>> +                     if (tcon->ses->server->ops->set_compression) {
>> +                             rc = tcon->ses->server->ops->set_compression(
>> +                                                     xid, tcon, pSMBFile);
>> +                             cifs_dbg(FYI, "set compress flag rc %d\n", rc);
>>                       }
>
> Is the FileFsAttributeInformation known at this stage? If so, you may

During mount we do:

        if (!tcon->ipc && server->ops->qfs_tcon)
                server->ops->qfs_tcon(xid, tcon);

(for informational/debug purposes) but qfs_tcon is only defined in
cifs, not for smb2/smb2.1/smb3.  We could add it easily enough (so you
could see this info in /proc/fs/cifs/ or perhaps /proc/mounts if that
were preferred - but in any case added SMB2 QFS Attribute Info would
probably be better as a distinct patch).


> wish to check for the FILE_FILE_COMPRESSION capability, and avoid the
> request dispatch if absent.

There doesn't seem to be much harm in sending the set compression
fsctl so we know the most uptodate info on whether the server can
support it or not - since this is an explicit command at cli (not in
the Linux GUIs AFAIK - although if there were a way to advertise
per-mount capabilities this would be a logical one to add)

> IIRC, Explorer hides the compression check-checkbox if the capability
> is missing.

I am a little worried about two cases:

1) what if the server reenabled compression on the underlying volume
after we mounted the share

2) what if the server exports multiple file systems under a single
share one which supports compression and one which doesn't - we would
only query on the root
Stefan Metzmacher Oct. 10, 2013, 6:25 a.m. UTC | #3
Hi Steve,

> Allow cifs/smb2/smb3 to return whether or not a file is compressed
> via lsattr, and allow SMB2/SMB3 to set the per-file compression
> flag ("chattr +c filename" on an smb2 or smb3 mount).

There're still unrelated generic fixes in SMB2_ioctl(), which would
a separate commit.

I also don't understand why the set operation should not be available
in all protocols, there's nothing SMB2 or SMB3 specific.

metze
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steve French Oct. 10, 2013, 1:50 p.m. UTC | #4
On Thu, Oct 10, 2013 at 1:25 AM, Stefan (metze) Metzmacher
<metze@samba.org> wrote:
> Hi Steve,
>
>> Allow cifs/smb2/smb3 to return whether or not a file is compressed
>> via lsattr, and allow SMB2/SMB3 to set the per-file compression
>> flag ("chattr +c filename" on an smb2 or smb3 mount).
>
> There're still unrelated generic fixes in SMB2_ioctl(), which would
> a separate commit.

Not totally unrelated because I noticed I had to fix the length
calculation when trying to get this particular ioctl to work, but it
is easy enough to break into another patch. Will do.

> I also don't understand why the set operation should not be available
> in all protocols, there's nothing SMB2 or SMB3 specific.

I could add the cifs ioctl as a followon patch but figured cifs was
lower priority because vast majority of current servers would have at
least smb2.   Was trying to focus on the remaining missing ioctls and
fs info calls that SMB2/SMB3 should be able to make (e.g. verify
negotiate, copy offload, query adapter info etc.)
diff mbox

Patch

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 52b6f6c..06e8947 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -278,6 +278,8 @@  struct smb_version_operations {
  /* set attributes */
  int (*set_file_info)(struct inode *, const char *, FILE_BASIC_INFO *,
      const unsigned int);
+ int (*set_compression)(const unsigned int, struct cifs_tcon *,
+       struct cifsFileInfo *);
  /* check if we can send an echo or nor */
  bool (*can_echo)(struct TCP_Server_Info *);
  /* send echo request */
diff --git a/fs/cifs/ioctl.c b/fs/cifs/ioctl.c
index 3e08455..0298670 100644
--- a/fs/cifs/ioctl.c
+++ b/fs/cifs/ioctl.c
@@ -3,7 +3,7 @@ 
  *
  *   vfs operations that deal with io control
  *
- *   Copyright (C) International Business Machines  Corp., 2005,2007
+ *   Copyright (C) International Business Machines  Corp., 2005,2013
  *   Author(s): Steve French (sfrench@us.ibm.com)
  *
  *   This library is free software; you can redistribute it and/or modify
@@ -34,13 +34,11 @@  long cifs_ioctl(struct file *filep, unsigned int
command, unsigned long arg)
  int rc = -ENOTTY; /* strange error - but the precedent */
  unsigned int xid;
  struct cifs_sb_info *cifs_sb;
-#ifdef CONFIG_CIFS_POSIX
  struct cifsFileInfo *pSMBFile = filep->private_data;
  struct cifs_tcon *tcon;
  __u64 ExtAttrBits = 0;
  __u64 ExtAttrMask = 0;
  __u64   caps;
-#endif /* CONFIG_CIFS_POSIX */

  xid = get_xid();

@@ -49,12 +47,12 @@  long cifs_ioctl(struct file *filep, unsigned int
command, unsigned long arg)
  cifs_sb = CIFS_SB(inode->i_sb);

  switch (command) {
-#ifdef CONFIG_CIFS_POSIX
  case FS_IOC_GETFLAGS:
  if (pSMBFile == NULL)
  break;
  tcon = tlink_tcon(pSMBFile->tlink);
  caps = le64_to_cpu(tcon->fsUnixInfo.Capability);
+#ifdef CONFIG_CIFS_POSIX
  if (CIFS_UNIX_EXTATTR_CAP & caps) {
  rc = CIFSGetExtAttr(xid, tcon,
     pSMBFile->fid.netfid,
@@ -63,29 +61,50 @@  long cifs_ioctl(struct file *filep, unsigned int
command, unsigned long arg)
  rc = put_user(ExtAttrBits &
  FS_FL_USER_VISIBLE,
  (int __user *)arg);
+ if (rc != EOPNOTSUPP)
+ break;
+ }
+#endif /* CONFIG_CIFS_POSIX */
+ rc = 0;
+ if (CIFS_I(inode)->cifsAttrs & ATTR_COMPRESSED) {
+ /* add in the compressed bit */
+ ExtAttrBits = FS_COMPR_FL;
+ rc = put_user(ExtAttrBits & FS_FL_USER_VISIBLE,
+      (int __user *)arg);
  }
  break;
-
  case FS_IOC_SETFLAGS:
  if (pSMBFile == NULL)
  break;
  tcon = tlink_tcon(pSMBFile->tlink);
  caps = le64_to_cpu(tcon->fsUnixInfo.Capability);
- if (CIFS_UNIX_EXTATTR_CAP & caps) {
- if (get_user(ExtAttrBits, (int __user *)arg)) {
- rc = -EFAULT;
- break;
- }
- /*
- * rc = CIFSGetExtAttr(xid, tcon,
- *       pSMBFile->fid.netfid,
- *       extAttrBits,
- *       &ExtAttrMask);
- */
+
+ if (get_user(ExtAttrBits, (int __user *)arg)) {
+ rc = -EFAULT;
+ break;
+ }
+
+ /*
+ * if (CIFS_UNIX_EXTATTR_CAP & caps)
+ * rc = CIFSSetExtAttr(xid, tcon,
+ *       pSMBFile->fid.netfid,
+ *       extAttrBits,
+ *       &ExtAttrMask);
+ * if (rc != EOPNOTSUPP)
+ * break;
+ */
+
+ /* Currently only flag we can set is compressed flag */
+ if ((ExtAttrBits & FS_COMPR_FL) == 0)
+ break;
+
+ /* Try to set compress flag */
+ if (tcon->ses->server->ops->set_compression) {
+ rc = tcon->ses->server->ops->set_compression(
+ xid, tcon, pSMBFile);
+ cifs_dbg(FYI, "set compress flag rc %d\n", rc);
  }
- cifs_dbg(FYI, "set flags not implemented yet\n");
  break;
-#endif /* CONFIG_CIFS_POSIX */
  default:
  cifs_dbg(FYI, "unsupported ioctl\n");
  break;
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 861b332..b96bacc 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -446,6 +446,14 @@  smb2_set_file_size(const unsigned int xid, struct
cifs_tcon *tcon,
 }

 static int
+smb2_set_compression(const unsigned int xid, struct cifs_tcon *tcon,
+   struct cifsFileInfo *cfile)
+{
+ return SMB2_set_compression(xid, tcon, cfile->fid.persistent_fid,
+    cfile->fid.volatile_fid);
+}
+
+static int
 smb2_query_dir_first(const unsigned int xid, struct cifs_tcon *tcon,
      const char *path, struct cifs_sb_info *cifs_sb,
      struct cifs_fid *fid, __u16 search_flags,
@@ -874,6 +882,7 @@  struct smb_version_operations smb20_operations = {
  .set_path_size = smb2_set_path_size,
  .set_file_size = smb2_set_file_size,
  .set_file_info = smb2_set_file_info,
+ .set_compression = smb2_set_compression,
  .mkdir = smb2_mkdir,
  .mkdir_setinfo = smb2_mkdir_setinfo,
  .rmdir = smb2_rmdir,
@@ -945,6 +954,7 @@  struct smb_version_operations smb21_operations = {
  .set_path_size = smb2_set_path_size,
  .set_file_size = smb2_set_file_size,
  .set_file_info = smb2_set_file_info,
+ .set_compression = smb2_set_compression,
  .mkdir = smb2_mkdir,
  .mkdir_setinfo = smb2_mkdir_setinfo,
  .rmdir = smb2_rmdir,
@@ -1017,6 +1027,7 @@  struct smb_version_operations smb30_operations = {
  .set_path_size = smb2_set_path_size,
  .set_file_size = smb2_set_file_size,
  .set_file_info = smb2_set_file_info,
+ .set_compression = smb2_set_compression,
  .mkdir = smb2_mkdir,
  .mkdir_setinfo = smb2_mkdir_setinfo,
  .rmdir = smb2_rmdir,
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index edccb52..fb19ad8 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -1137,6 +1137,7 @@  SMB2_ioctl(const unsigned int xid, struct
cifs_tcon *tcon, u64 persistent_fid,

  cifs_dbg(FYI, "SMB2 IOCTL\n");

+ *out_data = NULL;
  /* zero out returned data len, in case of error */
  if (plen)
  *plen = 0;
@@ -1183,10 +1184,12 @@  SMB2_ioctl(const unsigned int xid, struct
cifs_tcon *tcon, u64 persistent_fid,

  iov[0].iov_base = (char *)req;
  /* 4 for rfc1002 length field */
- iov[0].iov_len = get_rfc1002_length(req) + 4;
+ /* -1 since last byte is buf[0] which is sent in iov[1] or not at all */
+ iov[0].iov_len = get_rfc1002_length(req) + 4 - 1;

+ /* -1 since last byte is buf[0] which was counted in smb2_buf_len */
  if (indatalen)
- inc_rfc1001_len(req, indatalen);
+ inc_rfc1001_len(req, indatalen - 1);

  rc = SendReceive2(xid, ses, iov, num_iovecs, &resp_buftype, 0);
  rsp = (struct smb2_ioctl_rsp *)iov[0].iov_base;
@@ -1234,6 +1237,32 @@  ioctl_exit:
  return rc;
 }

+/*
+ *   Individual callers to ioctl worker function follow
+ */
+
+int
+SMB2_set_compression(const unsigned int xid, struct cifs_tcon *tcon,
+     u64 persistent_fid, u64 volatile_fid)
+{
+ int rc;
+ char *res_key = NULL;
+ struct  compress_ioctl fsctl_input;
+ char *ret_data = NULL;
+
+ fsctl_input.CompressionState = cpu_to_le16(COMPRESSION_FORMAT_DEFAULT);
+
+ rc = SMB2_ioctl(xid, tcon, persistent_fid, volatile_fid,
+ FSCTL_SET_COMPRESSION, true /* is_fsctl */,
+ (char *)&fsctl_input /* data input */,
+ 2 /* in data len */, &ret_data /* out data */, NULL);
+
+ cifs_dbg(VFS, "set compression rc %d\n", rc); /* FIXME */
+ kfree(res_key);
+
+ return rc;
+}
+
 int
 SMB2_close(const unsigned int xid, struct cifs_tcon *tcon,
    u64 persistent_fid, u64 volatile_fid)
diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
index b83d011..c7c3c82 100644
--- a/fs/cifs/smb2pdu.h
+++ b/fs/cifs/smb2pdu.h
@@ -569,6 +569,13 @@  struct network_interface_info_ioctl_rsp {

 #define NO_FILE_ID 0xFFFFFFFFFFFFFFFFULL /* general ioctls to srv not
to file */

+struct compress_ioctl {
+ __le16 CompressionState;
+} __packed;
+
+#define COMPRESSION_FORMAT_NONE 0x0000
+#define COMPRESSION_FORMAT_DEFAULT 0x0001
+#define COMPRESSION_FORMAT_LZNT1 0x0002
 struct smb2_ioctl_req {
  struct smb2_hdr hdr;
  __le16 StructureSize; /* Must be 57 */
@@ -584,7 +591,7 @@  struct smb2_ioctl_req {
  __le32 MaxOutputResponse;
  __le32 Flags;
  __u32  Reserved2;
- char   Buffer[0];
+ __u8   Buffer[0];
 } __packed;

 struct smb2_ioctl_rsp {
diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
index e3fb480..3cf22e3 100644
--- a/fs/cifs/smb2proto.h
+++ b/fs/cifs/smb2proto.h
@@ -142,6 +142,8 @@  extern int SMB2_set_eof(const unsigned int xid,
struct cifs_tcon *tcon,
 extern int SMB2_set_info(const unsigned int xid, struct cifs_tcon *tcon,
  u64 persistent_fid, u64 volatile_fid,
  FILE_BASIC_INFO *buf);
+extern int SMB2_set_compression(const unsigned int xid, struct cifs_tcon *tcon,
+ u64 persistent_fid, u64 volatile_fid);
 extern int SMB2_oplock_break(const unsigned int xid, struct cifs_tcon *tcon,
      const u64 persistent_fid, const u64 volatile_fid,