[PATCHES] ntquota - differentiate between "no quota" and error condition
Jeremy Allison
jra at samba.org
Thu Mar 31 18:25:47 UTC 2016
On Wed, Mar 30, 2016 at 02:40:05PM +0300, Uri Simchoni wrote:
> Hi,
>
> The attached patch set changes the way the VFS indicates "no quota" for
> a user or a group. It is supposed not to change behavior (except for a
> small bugfix in smbcquotas client tool).
>
> Before the patches, the sysquota drivers would report an error if no
> quota was assigned. After the patch they report zero quota. This is
> meant to be the convention for VFS modules to follow as well.
>
> The purpose of this change is to allow stacking of VFS modules - allow a
> module to say "get the quota from underlying module but do something
> special if there is no quota", but at the same time not cover-up failures.
>
> When differentiating between real errors and no-quota in xfs and nfs, I
> was inspired by the "quota (8)" utility which also has to cope with this.
>
> Review appreciated.
OK, took me a while to feel safe with the changes in
source3/lib/sysquotas_4A.c (and some of the other
code in there *really* needs tidying up :-) but I
finally got there.
LGTM. Pushed !
> From 38920bd0893d381bc3eee55363ac19fb6bece3cd Mon Sep 17 00:00:00 2001
> From: Uri Simchoni <uri at samba.org>
> Date: Tue, 29 Mar 2016 23:30:23 +0300
> Subject: [PATCH 1/8] nt-quotas: vfs_get_ntquota() return NTSTATUS
>
> Signed-off-by: Uri Simchoni <uri at samba.org>
> ---
> source3/smbd/ntquotas.c | 19 ++++++++++++-------
> source3/smbd/nttrans.c | 4 +++-
> source3/smbd/proto.h | 3 ++-
> source3/smbd/trans2.c | 6 ++++--
> 4 files changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/source3/smbd/ntquotas.c b/source3/smbd/ntquotas.c
> index aa2ec3b..a36196b 100644
> --- a/source3/smbd/ntquotas.c
> +++ b/source3/smbd/ntquotas.c
> @@ -76,7 +76,8 @@ static uint64_t limit_blk2inodes(uint64_t in)
> return ret;
> }
>
> -int vfs_get_ntquota(files_struct *fsp, enum SMB_QUOTA_TYPE qtype, struct dom_sid *psid, SMB_NTQUOTA_STRUCT *qt)
> +NTSTATUS vfs_get_ntquota(files_struct *fsp, enum SMB_QUOTA_TYPE qtype,
> + struct dom_sid *psid, SMB_NTQUOTA_STRUCT *qt)
> {
> int ret;
> SMB_DISK_QUOTA D;
> @@ -84,8 +85,9 @@ int vfs_get_ntquota(files_struct *fsp, enum SMB_QUOTA_TYPE qtype, struct dom_sid
>
> ZERO_STRUCT(D);
>
> - if (!fsp||!fsp->conn||!qt)
> - return (-1);
> + if (!fsp || !fsp->conn || !qt) {
> + return NT_STATUS_INTERNAL_ERROR;
> + }
>
> ZERO_STRUCT(*qt);
>
> @@ -94,6 +96,7 @@ int vfs_get_ntquota(files_struct *fsp, enum SMB_QUOTA_TYPE qtype, struct dom_sid
> if (psid && !sid_to_uid(psid, &id.uid)) {
> DEBUG(0,("sid_to_uid: failed, SID[%s]\n",
> sid_string_dbg(psid)));
> + return NT_STATUS_NO_SUCH_USER;
> }
>
> ret = SMB_VFS_GET_QUOTA(fsp->conn, ".", qtype, id, &D);
> @@ -102,7 +105,7 @@ int vfs_get_ntquota(files_struct *fsp, enum SMB_QUOTA_TYPE qtype, struct dom_sid
> qt->sid = *psid;
>
> if (ret!=0) {
> - return ret;
> + return map_nt_error_from_unix(errno);
> }
>
> qt->usedspace = (uint64_t)D.curblocks*D.bsize;
> @@ -110,8 +113,7 @@ int vfs_get_ntquota(files_struct *fsp, enum SMB_QUOTA_TYPE qtype, struct dom_sid
> qt->hardlim = limit_unix2nt(D.hardlimit, D.bsize);
> qt->qflags = D.qflags;
>
> -
> - return 0;
> + return NT_STATUS_OK;
> }
>
> int vfs_set_ntquota(files_struct *fsp, enum SMB_QUOTA_TYPE qtype, struct dom_sid *psid, SMB_NTQUOTA_STRUCT *qt)
> @@ -181,6 +183,7 @@ int vfs_get_user_ntquota_list(files_struct *fsp, SMB_NTQUOTA_LIST **qt_list)
> SMB_NTQUOTA_STRUCT tmp_qt;
> SMB_NTQUOTA_LIST *tmp_list_ent;
> struct dom_sid sid;
> + NTSTATUS status;
>
> ZERO_STRUCT(tmp_qt);
>
> @@ -191,7 +194,9 @@ int vfs_get_user_ntquota_list(files_struct *fsp, SMB_NTQUOTA_LIST **qt_list)
>
> uid_to_sid(&sid, usr->pw_uid);
>
> - if (vfs_get_ntquota(fsp, SMB_USER_QUOTA_TYPE, &sid, &tmp_qt)!=0) {
> + status =
> + vfs_get_ntquota(fsp, SMB_USER_QUOTA_TYPE, &sid, &tmp_qt);
> + if (!NT_STATUS_IS_OK(status)) {
> DEBUG(5,("no quota entry for sid[%s] path[%s]\n",
> sid_string_dbg(&sid),
> fsp->conn->connectpath));
> diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c
> index 3a2c35f..232cc68 100644
> --- a/source3/smbd/nttrans.c
> +++ b/source3/smbd/nttrans.c
> @@ -2509,7 +2509,9 @@ static void call_nt_transact_get_user_quota(connection_struct *conn,
> return;
> }
>
> - if (vfs_get_ntquota(fsp, SMB_USER_QUOTA_TYPE, &sid, &qt)!=0) {
> + nt_status = vfs_get_ntquota(fsp, SMB_USER_QUOTA_TYPE,
> + &sid, &qt);
> + if (!NT_STATUS_IS_OK(nt_status)) {
> ZERO_STRUCT(qt);
> /*
> * we have to return zero's in all fields
> diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
> index 8e8cbc3..3612034 100644
> --- a/source3/smbd/proto.h
> +++ b/source3/smbd/proto.h
> @@ -600,7 +600,8 @@ NTSTATUS notify_walk(struct notify_context *notify,
>
> /* The following definitions come from smbd/ntquotas.c */
>
> -int vfs_get_ntquota(files_struct *fsp, enum SMB_QUOTA_TYPE qtype, struct dom_sid *psid, SMB_NTQUOTA_STRUCT *qt);
> +NTSTATUS vfs_get_ntquota(files_struct *fsp, enum SMB_QUOTA_TYPE qtype,
> + struct dom_sid *psid, SMB_NTQUOTA_STRUCT *qt);
> int vfs_set_ntquota(files_struct *fsp, enum SMB_QUOTA_TYPE qtype, struct dom_sid *psid, SMB_NTQUOTA_STRUCT *qt);
> int vfs_get_user_ntquota_list(files_struct *fsp, SMB_NTQUOTA_LIST **qt_list);
> void *init_quota_handle(TALLOC_CTX *mem_ctx);
> diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
> index f351008..0e1c6d9 100644
> --- a/source3/smbd/trans2.c
> +++ b/source3/smbd/trans2.c
> @@ -3712,9 +3712,11 @@ cBytesSector=%u, cUnitTotal=%u, cUnitAvail=%d\n", (unsigned int)bsize, (unsigned
> return NT_STATUS_ACCESS_DENIED;
> }
>
> - if (vfs_get_ntquota(&fsp, SMB_USER_FS_QUOTA_TYPE, NULL, "as)!=0) {
> + status = vfs_get_ntquota(&fsp, SMB_USER_FS_QUOTA_TYPE,
> + NULL, "as);
> + if (!NT_STATUS_IS_OK(status)) {
> DEBUG(0,("vfs_get_ntquota() failed for service [%s]\n",lp_servicename(talloc_tos(), SNUM(conn))));
> - return map_nt_error_from_unix(errno);
> + return status;
> }
>
> data_len = 48;
> --
> 2.5.5
>
>
> From ba8bb05d6a45b892f4eda443d8076c8ac44888b0 Mon Sep 17 00:00:00 2001
> From: Uri Simchoni <uri at samba.org>
> Date: Tue, 29 Mar 2016 23:05:09 +0300
> Subject: [PATCH 2/8] nt-quotas: return 0 as indication of no quota
>
> When getting user quota, the correct value to indicate "no quota"
> is 0, not -1.
>
> In [MS-FSCC] section 2.4.33 it is written that -1 designates no-quota.
> However, careful read of that section shows that this designation is only
> true when setting the quota, and this section says nothing about getting
> the quota.
>
> In [MS-FSA] section 2.1.5.20, it is written that "If SidList includes a SID
> that does not map to an existing SID in the Open.File.Volume.QuotaInformation
> list, the object store MUST return a FILE_QUOTA_INFORMATION structure
> (as specified in [MS-FSCC] section 2.4.33) that is filled with zeros.
>
> This is also verified experimentally and cleared with dochelp.
>
> Signed-off-by: Uri Simchoni <uri at samba.org>
> ---
> source3/smbd/ntquotas.c | 8 --------
> 1 file changed, 8 deletions(-)
>
> diff --git a/source3/smbd/ntquotas.c b/source3/smbd/ntquotas.c
> index a36196b..b9fd2f1 100644
> --- a/source3/smbd/ntquotas.c
> +++ b/source3/smbd/ntquotas.c
> @@ -53,14 +53,6 @@ static uint64_t limit_unix2nt(uint64_t in, uint64_t bsize)
>
> ret = (uint64_t)(in*bsize);
>
> - if (ret < in) {
> - /* we overflow */
> - ret = SMB_NTQUOTAS_NO_LIMIT;
> - }
> -
> - if (in == SMB_QUOTAS_NO_LIMIT)
> - ret = SMB_NTQUOTAS_NO_LIMIT;
> -
> return ret;
> }
>
> --
> 2.5.5
>
>
> From 503a511c7a9119a38e0de3e9fbb8d04d8ac4b10c Mon Sep 17 00:00:00 2001
> From: Uri Simchoni <uri at samba.org>
> Date: Wed, 30 Mar 2016 13:59:39 +0300
> Subject: [PATCH 3/8] ntquotas - skip entry if the quota is zero
>
> When listing user quotas, do not list the user
> if the driver returned success with zero quota -
> this signals that no quota is assigned for that
> user.
>
> Signed-off-by: Uri Simchoni <uri at samba.org>
> ---
> source3/smbd/ntquotas.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/source3/smbd/ntquotas.c b/source3/smbd/ntquotas.c
> index b9fd2f1..9b2e39a 100644
> --- a/source3/smbd/ntquotas.c
> +++ b/source3/smbd/ntquotas.c
> @@ -189,6 +189,11 @@ int vfs_get_user_ntquota_list(files_struct *fsp, SMB_NTQUOTA_LIST **qt_list)
> status =
> vfs_get_ntquota(fsp, SMB_USER_QUOTA_TYPE, &sid, &tmp_qt);
> if (!NT_STATUS_IS_OK(status)) {
> + DEBUG(5, ("failed getting quota for uid[%ld] - %s\n",
> + (long)usr->pw_uid, nt_errstr(status)));
> + continue;
> + }
> + if (tmp_qt.softlim == 0 && tmp_qt.hardlim == 0) {
> DEBUG(5,("no quota entry for sid[%s] path[%s]\n",
> sid_string_dbg(&sid),
> fsp->conn->connectpath));
> --
> 2.5.5
>
>
> From 10ddf36a0df6a88807c0e705564699fdfee1c4b4 Mon Sep 17 00:00:00 2001
> From: Uri Simchoni <uri at samba.org>
> Date: Wed, 30 Mar 2016 14:05:49 +0300
> Subject: [PATCH 4/8] sys-quotas: do not fail if user has no quota
>
> If the user/group has no quota, do not treat that as
> error condition. Instead, return zero quota.
>
> Signed-off-by: Uri Simchoni <uri at samba.org>
> ---
> source3/lib/sysquotas_4A.c | 16 ++--------------
> source3/lib/sysquotas_linux.c | 9 +--------
> 2 files changed, 3 insertions(+), 22 deletions(-)
>
> diff --git a/source3/lib/sysquotas_4A.c b/source3/lib/sysquotas_4A.c
> index 244b612..674c4ee 100644
> --- a/source3/lib/sysquotas_4A.c
> +++ b/source3/lib/sysquotas_4A.c
> @@ -104,13 +104,7 @@ int sys_get_vfs_quota(const char *path, const char *bdev, enum SMB_QUOTA_TYPE qt
> return ret;
> }
>
> - if ((D.dqb_curblocks==0)&&
> - (D.dqb_bsoftlimit==0)&&
> - (D.dqb_bhardlimit==0)) {
> - /* the upper layer functions don't want empty quota records...*/
> - return -1;
> - }
> -
> + ret = 0;
> break;
> #ifdef HAVE_GROUP_QUOTA
> case SMB_GROUP_QUOTA_TYPE:
> @@ -121,13 +115,7 @@ int sys_get_vfs_quota(const char *path, const char *bdev, enum SMB_QUOTA_TYPE qt
> return ret;
> }
>
> - if ((D.dqb_curblocks==0)&&
> - (D.dqb_bsoftlimit==0)&&
> - (D.dqb_bhardlimit==0)) {
> - /* the upper layer functions don't want empty quota records...*/
> - return -1;
> - }
> -
> + ret = 0;
> break;
> #endif /* HAVE_GROUP_QUOTA */
> case SMB_USER_FS_QUOTA_TYPE:
> diff --git a/source3/lib/sysquotas_linux.c b/source3/lib/sysquotas_linux.c
> index bf3504a..5984626 100644
> --- a/source3/lib/sysquotas_linux.c
> +++ b/source3/lib/sysquotas_linux.c
> @@ -447,14 +447,7 @@ int sys_get_vfs_quota(const char *path, const char *bdev, enum SMB_QUOTA_TYPE qt
> }
> }
> }
> -
> - if ((dp->curblocks==0)&&
> - (dp->softlimit==0)&&
> - (dp->hardlimit==0)) {
> - /* the upper layer functions don't want empty quota records...*/
> - return -1;
> - }
> -
> + ret = 0;
> break;
> case SMB_USER_FS_QUOTA_TYPE:
> id.uid = getuid();
> --
> 2.5.5
>
>
> From 6784814c1a70c52375e6aaa64fbf8b98c70f0e83 Mon Sep 17 00:00:00 2001
> From: Uri Simchoni <uri at samba.org>
> Date: Wed, 30 Mar 2016 13:00:29 +0300
> Subject: [PATCH 5/8] xfs-quota: do not fail if user has no quota
>
> XFS fails quotactl(Q_XGETQUOTA) with ENOENT if the user
> or group has no quota assigned to it. This is not an error
> condition - simply report 0 quota in this case.
>
> Signed-off-by: Uri Simchoni <uri at samba.org>
> ---
> source3/lib/sysquotas_xfs.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/source3/lib/sysquotas_xfs.c b/source3/lib/sysquotas_xfs.c
> index ccc7fc0..bea86d5 100644
> --- a/source3/lib/sysquotas_xfs.c
> +++ b/source3/lib/sysquotas_xfs.c
> @@ -90,16 +90,28 @@ int sys_get_xfs_quota(const char *path, const char *bdev, enum SMB_QUOTA_TYPE qt
> DEBUG(10,("sys_get_xfs_quota: path[%s] bdev[%s] SMB_USER_QUOTA_TYPE uid[%u]\n",
> path, bdev, (unsigned)id.uid));
>
> - if ((ret=quotactl(QCMD(Q_XGETQUOTA,USRQUOTA), bdev, id.uid, (caddr_t)&D)))
> + ret=quotactl(QCMD(Q_XGETQUOTA,USRQUOTA), bdev, id.uid, (caddr_t)&D);
> + /* XFS fails with ENOENT if the user has no
> + * quota. Our protocol in that case is to
> + * succeed and return 0 as quota.
> + */
> + if (ret != 0 && errno != ENOENT) {
> return ret;
> + }
> break;
> #ifdef HAVE_GROUP_QUOTA
> case SMB_GROUP_QUOTA_TYPE:
> DEBUG(10,("sys_get_xfs_quota: path[%s] bdev[%s] SMB_GROUP_QUOTA_TYPE gid[%u]\n",
> path, bdev, (unsigned)id.gid));
>
> - if ((ret=quotactl(QCMD(Q_XGETQUOTA,GRPQUOTA), bdev, id.gid, (caddr_t)&D)))
> + ret=quotactl(QCMD(Q_XGETQUOTA,GRPQUOTA), bdev, id.gid, (caddr_t)&D);
> + /* XFS fails with ENOENT if the user has no
> + * quota. Our protocol in that case is to
> + * succeed and return 0 as quota.
> + */
> + if (ret != 0 && errno != ENOENT) {
> return ret;
> + }
> break;
> #endif /* HAVE_GROUP_QUOTA */
> case SMB_USER_FS_QUOTA_TYPE:
> --
> 2.5.5
>
>
> From 3c9439c3f6e6ab3a2dca5403b0d75b6ba7571a1c Mon Sep 17 00:00:00 2001
> From: Uri Simchoni <uri at samba.org>
> Date: Wed, 30 Mar 2016 13:21:58 +0300
> Subject: [PATCH 6/8] nfs-quota: do not fail on ECONNREFUSED
>
> Trying to differentiate between "no quota" and real
> error conditions - if the connection to rpc.quotad
> is refused it could simply mean that the remote host
> has no quota and therefore report this as success with
> no quota.
>
> Signed-off-by: Uri Simchoni <uri at samba.org>
> ---
> source3/lib/sysquotas_nfs.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/source3/lib/sysquotas_nfs.c b/source3/lib/sysquotas_nfs.c
> index 4b37e34..fe46d3f 100644
> --- a/source3/lib/sysquotas_nfs.c
> +++ b/source3/lib/sysquotas_nfs.c
> @@ -180,8 +180,20 @@ int sys_get_nfs_quota(const char *path, const char *bdev,
> timeout);
>
> if (clnt_stat != RPC_SUCCESS) {
> - DEBUG(3, ("sys_get_nfs_quotas: clnt_call failed\n"));
> - ret = -1;
> + if (errno == ECONNREFUSED) {
> + /* If we cannot connect with rpc.quotad, it may
> + * simply be because there's no quota on the remote
> + * system
> + */
> + DBG_INFO("clnt_call failed with ECONNREFUSED - "
> + "assuming no quotas on server\n");
> + ret = 0;
> + } else {
> + int save_errno = errno;
> + DBG_NOTICE("clnt_call failed - %s\n", strerror(errno));
> + errno = save_errno;
> + ret = -1;
> + }
> goto out;
> }
>
> --
> 2.5.5
>
>
> From e2ac257833811349983c1d2d2c1a902798738d77 Mon Sep 17 00:00:00 2001
> From: Uri Simchoni <uri at samba.org>
> Date: Wed, 30 Mar 2016 14:02:31 +0300
> Subject: [PATCH 7/8] smbd: do not cover up VFS failures to get quota
>
> Now that the VFS follows the convention that get-quota
> returns error only on error condition, and success
> with zero quota if there is no quota assigned,
> reply with an error if failing to obtain a user's
> quota.
>
> Signed-off-by: Uri Simchoni <uri at samba.org>
> ---
> source3/smbd/nttrans.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c
> index 232cc68..fa3f74c 100644
> --- a/source3/smbd/nttrans.c
> +++ b/source3/smbd/nttrans.c
> @@ -2512,12 +2512,7 @@ static void call_nt_transact_get_user_quota(connection_struct *conn,
> nt_status = vfs_get_ntquota(fsp, SMB_USER_QUOTA_TYPE,
> &sid, &qt);
> if (!NT_STATUS_IS_OK(nt_status)) {
> - ZERO_STRUCT(qt);
> - /*
> - * we have to return zero's in all fields
> - * instead of returning an error here
> - * --metze
> - */
> + reply_nterror(req, nt_status);
> }
>
> /* Realloc the size of parameters and data we will return */
> --
> 2.5.5
>
>
> From b22c988d0e5c2d08eb1f9098dc04a18f9ba80ae4 Mon Sep 17 00:00:00 2001
> From: Uri Simchoni <uri at samba.org>
> Date: Wed, 30 Mar 2016 14:20:44 +0300
> Subject: [PATCH 8/8] smbcquotas: print "NO LIMIT" only if returned quota value
> is 0.
>
> If the user being queried has no quota, the server returns 0 as
> its quota. This is the observed smbd and Windows behavior, which
> is also documented in [MS-FSA] 2.5.1.20.
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=11815
>
> Signed-off-by: Uri Simchoni <uri at samba.org>
> ---
> source3/utils/smbcquotas.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/source3/utils/smbcquotas.c b/source3/utils/smbcquotas.c
> index 9e64319..e6f1dfb 100644
> --- a/source3/utils/smbcquotas.c
> +++ b/source3/utils/smbcquotas.c
> @@ -236,7 +236,7 @@ static const char *quota_str_static(uint64_t val, bool special, bool _numeric)
> {
> const char *result;
>
> - if (!_numeric&&special&&(val == SMB_NTQUOTAS_NO_LIMIT)) {
> + if (!_numeric && special && val == 0) {
> return "NO LIMIT";
> }
> result = talloc_asprintf(talloc_tos(), "%"PRIu64, val);
> --
> 2.5.5
>
More information about the samba-technical
mailing list