[PATCHES] s3-smbd: do not clamp free disk reporting with quota if quota is not enforced

Uri Simchoni uri at samba.org
Thu May 26 21:16:48 UTC 2016


Hi,

The attached patch set fixes free-space-reporting on an XFS file system
where quota accounting is enabled but quota enforcement is disabled.

Out of the supported file systems and OS's, it seems like only XFS has a
separate notion of quota enforcement and accounting. An alternative fix
could have been to just fix the XFS driver. However, this would break
getting the user's quota, which should not be affected by whether the
quota is actually being enforced (I verified this on a Windows 2008R2
server).

Because code common to all file systems is being changed, I verified
that it doesn't break ext4 on Linux, ufs on FreeBSD (the 4B driver), and
NFS. Haven't tested the 4A driver though.

Review appreciated.
Thanks,
Uri.
-------------- next part --------------
From 65ee7348c7b47b9effdc9af09f0fd475666836ac Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Thu, 26 May 2016 21:59:38 +0300
Subject: [PATCH 1/3] vfs_fake_dfq: add more mocking options

Add support for mocking FS user/group quotas (default quota and
quota flags).

Make the default block size 4096 instead of 0. This
turns the default into "no quota" instead of "punt to
lower VFS module" (that is, if the mock module is asked
to retrieve quota of a user/group/default for which there
is no config).

Add support for ENOSYS error

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11937

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/modules/vfs_fake_dfq.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/source3/modules/vfs_fake_dfq.c b/source3/modules/vfs_fake_dfq.c
index e476e16..bf49860 100644
--- a/source3/modules/vfs_fake_dfq.c
+++ b/source3/modules/vfs_fake_dfq.c
@@ -110,6 +110,12 @@ static int dfq_get_quota(struct vfs_handle_struct *handle, const char *path,
 		section = talloc_asprintf(talloc_tos(), "g%llu",
 					  (unsigned long long)id.gid);
 		break;
+	case SMB_USER_FS_QUOTA_TYPE:
+		section = talloc_strdup(talloc_tos(), "udflt");
+		break;
+	case SMB_GROUP_FS_QUOTA_TYPE:
+		section = talloc_strdup(talloc_tos(), "gdflt");
+		break;
 	default:
 		break;
 	}
@@ -118,7 +124,7 @@ static int dfq_get_quota(struct vfs_handle_struct *handle, const char *path,
 		goto dflt;
 	}
 
-	bsize = dfq_load_param(snum, rpath, section, "block size", 0);
+	bsize = dfq_load_param(snum, rpath, section, "block size", 4096);
 	if (bsize == 0) {
 		goto dflt;
 	}
@@ -129,6 +135,12 @@ static int dfq_get_quota(struct vfs_handle_struct *handle, const char *path,
 		goto out;
 	}
 
+	if (dfq_load_param(snum, rpath, section, "nosys", 0) != 0) {
+		errno = ENOSYS;
+		rc = -1;
+		goto out;
+	}
+
 	ZERO_STRUCTP(qt);
 
 	qt->bsize = bsize;
@@ -140,6 +152,7 @@ static int dfq_get_quota(struct vfs_handle_struct *handle, const char *path,
 	qt->isoftlimit =
 	    dfq_load_param(snum, rpath, section, "inode soft limit", 0);
 	qt->curinodes = dfq_load_param(snum, rpath, section, "cur inodes", 0);
+	qt->qflags = dfq_load_param(snum, rpath, section, "qflags", QUOTAS_DENY_DISK);
 
 	if (dfq_load_param(snum, rpath, section, "edquot", 0) != 0) {
 		errno = EDQUOT;
-- 
2.5.5


From 3f0ade8ae8bc324817fa3f7dcc7925c172f1dfe4 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Thu, 26 May 2016 22:52:09 +0300
Subject: [PATCH 2/3] selftest: add disk-free quota tests

Add a test for situation where quota accounting is enabled
but quota enforcement is disabled (disk-free should not take
quota into account)

Add a test for situation where overall quota status reporting
(whether or not it's enforcing) is not supported - as with NFS.
In that case it must be assumed that if quota is configured, then
it is also enforced (as with NFS).

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11937

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 selftest/knownfail                       |  2 ++
 source3/script/tests/test_dfree_quota.sh | 12 ++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/selftest/knownfail b/selftest/knownfail
index 2f2d6bf..fd86a9c 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -330,3 +330,5 @@
 # ad_dc requires signing
 #
 ^samba4.smb.signing.*disabled.*signing=off.*\(ad_dc\)
+#new disk-free tests fail the code
+^samba3.blackbox.dfree_quota \(fileserver\).Test dfree share root quota not enforced\(fileserver\)
diff --git a/source3/script/tests/test_dfree_quota.sh b/source3/script/tests/test_dfree_quota.sh
index 2579255..d36530d 100755
--- a/source3/script/tests/test_dfree_quota.sh
+++ b/source3/script/tests/test_dfree_quota.sh
@@ -66,6 +66,12 @@ trygrp2:u$uid:block size = 4096:hard limit = 0:soft limit = 0:cur blocks = 41
 trygrp2:g$gid:block size = 4096:hard limit = 60:soft limit = 60:cur blocks = 56
 blksize:df:block size = 512:disk free = 614400:disk size = 614400
 blksize:u$uid:block size = 1024:hard limit = 512000:soft limit = 0:cur blocks = 0
+notenforce:df:block size = 4096:disk free = 10:disk size = 80
+notenforce:u$uid:block size = 4096:hard limit = 40:soft limit = 40:cur blocks = 37
+notenforce:udflt:block size = 4096:qflags = 0
+nfs:df:block size = 4096:disk free = 10:disk size = 80
+nfs:u$uid:block size = 4096:hard limit = 40:soft limit = 40:cur blocks = 37
+nfs:udflt:nosys = 1
 ABC
 }
 
@@ -175,5 +181,11 @@ test_smbclient_dfree "Test quota->dfree no-quota try group" "subdir1" "trygrp2 s
 #block size different in quota and df systems
 test_smbclient_dfree "Test quota->dfree different block size" "subdir1" "blksize subdir1" "307200 1024. 307200" -U$USERNAME%$PASSWORD --option=clientmaxprotocol=SMB3 || failed=`expr $failed + 1`
 
+#quota configured but not enforced
+test_smbclient_dfree "Test dfree share root quota not enforced" "." "notenforce ." "320 1024. 40" -U$USERNAME%$PASSWORD --option=clientmaxprotocol=SMB3 || failed=`expr $failed + 1`
+
+#FS quota not implemented (NFS case)
+test_smbclient_dfree "Test dfree share root FS quota not implemented" "." "nfs ." "160 1024. 12" -U$USERNAME%$PASSWORD --option=clientmaxprotocol=SMB3 || failed=`expr $failed + 1`
+
 setup_conf
 exit $failed
-- 
2.5.5


From d2339c5d7a2a78a877f2a3360f540b95019eaffc Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Wed, 27 Apr 2016 23:22:25 +0300
Subject: [PATCH 3/3] smbd: dfree - ignore quota if not enforced

When calculating free disk space, do not take user quota
into account if quota is globally not enforced on the file
system.

This is meant to fix a specific problem with XFS. One might
say "why don't you fix the XFS-specific code instead?". The
reason for that is that getting and setting quota must not
be affected by whether quota is actually enforced. NTFS has
the same notion of separating quota accounting (and being
able to configure / retrieve configured quota), from quota
enforcement.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11937

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 selftest/knownfail    |  2 --
 source3/smbd/quotas.c | 32 +++++++++++++++++++++++++++++++-
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/selftest/knownfail b/selftest/knownfail
index fd86a9c..2f2d6bf 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -330,5 +330,3 @@
 # ad_dc requires signing
 #
 ^samba4.smb.signing.*disabled.*signing=off.*\(ad_dc\)
-#new disk-free tests fail the code
-^samba3.blackbox.dfree_quota \(fileserver\).Test dfree share root quota not enforced\(fileserver\)
diff --git a/source3/smbd/quotas.c b/source3/smbd/quotas.c
index 58d8460..20f74b4 100644
--- a/source3/smbd/quotas.c
+++ b/source3/smbd/quotas.c
@@ -489,9 +489,24 @@ bool disk_quotas(connection_struct *conn, const char *path, uint64_t *bsize,
 	SMB_DISK_QUOTA D;
 	unid_t id;
 
-	id.uid = geteuid();
+	/*
+	 * First of all, check whether user quota is
+	 * enforced. If the call fails, assume it is
+	 * not enforced.
+	 */
+	ZERO_STRUCT(D);
+	id.uid = -1;
+	r = SMB_VFS_GET_QUOTA(conn, path, SMB_USER_FS_QUOTA_TYPE, id, &D);
+	if (r == -1 && errno != ENOSYS) {
+		goto try_group_quota;
+	}
+	if (r == 0 && (D.qflags & QUOTAS_DENY_DISK) == 0) {
+		goto try_group_quota;
+	}
 
 	ZERO_STRUCT(D);
+	id.uid = geteuid();
+
 	r = SMB_VFS_GET_QUOTA(conn, path, SMB_USER_QUOTA_TYPE, id, &D);
 
 	/* Use softlimit to determine disk space, except when it has been exceeded */
@@ -528,6 +543,21 @@ bool disk_quotas(connection_struct *conn, const char *path, uint64_t *bsize,
 	return True;
 	
 try_group_quota:
+	/*
+	 * First of all, check whether group quota is
+	 * enforced. If the call fails, assume it is
+	 * not enforced.
+	 */
+	ZERO_STRUCT(D);
+	id.gid = -1;
+	r = SMB_VFS_GET_QUOTA(conn, path, SMB_GROUP_FS_QUOTA_TYPE, id, &D);
+	if (r == -1 && errno != ENOSYS) {
+		return false;
+	}
+	if (r == 0 && (D.qflags & QUOTAS_DENY_DISK) == 0) {
+		return false;
+	}
+
 	id.gid = getegid();
 
 	ZERO_STRUCT(D);
-- 
2.5.5



More information about the samba-technical mailing list