[PATCHES] s3-smbd: do not clamp free disk reporting with quota if quota is not enforced
Jeremy Allison
jra at samba.org
Fri May 27 18:10:44 UTC 2016
On Fri, May 27, 2016 at 12:16:48AM +0300, Uri Simchoni wrote:
> 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.
Nice work Uri - thanks ! Pushed.
> 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