From 17c4fb2d04f84b5632983866e8bddfbd7d77a054 Mon Sep 17 00:00:00 2001 From: Krishnan Parthasarathi Date: Mon, 28 Oct 2013 19:14:49 +0530 Subject: quota: get directory size before enforcing quota on rename Change-Id: If18cab5992ddc91457782786942971deb1b51ead BUG: 1023974 Signed-off-by: Krishnan Parthasarathi Reviewed-on: http://review.gluster.org/6155 Reviewed-by: Raghavendra G Tested-by: Gluster Build System Reviewed-by: Vijay Bellur --- tests/bugs/bug-1023974.t | 33 ++++++++++++++++ xlators/features/quota/src/quota.c | 81 ++++++++++++++++++++++++++++++++------ xlators/features/quota/src/quota.h | 3 ++ 3 files changed, 105 insertions(+), 12 deletions(-) create mode 100644 tests/bugs/bug-1023974.t diff --git a/tests/bugs/bug-1023974.t b/tests/bugs/bug-1023974.t new file mode 100644 index 000000000..56766b979 --- /dev/null +++ b/tests/bugs/bug-1023974.t @@ -0,0 +1,33 @@ +#!/bin/bash + +# This regression test tries to ensure renaming a directory with content, and +# no limit set, is accounted properly, when moved into a directory with quota +# limit set. + +. $(dirname $0)/../include.rc + +cleanup; + +TEST glusterd +TEST pidof glusterd; +TEST $CLI volume info; + +TEST $CLI volume create $V0 replica 2 $H0:$B0/${V0}{1,2,3,4,5,6}; +TEST $CLI volume start $V0; + +TEST $CLI volume quota $V0 enable; + +TEST glusterfs --volfile-id=$V0 --volfile-server=$H0 $M0; + +TEST mkdir -p $M0/1/2; +TEST $CLI volume quota $V0 limit-usage /1/2 100MB 70%; + +#The corresponding write(3) should fail with EDQUOT ("Disk quota exceeded") +TEST ! dd if=/dev/urandom of=$M0/1/2/file bs=1M count=102; +TEST mkdir $M0/1/3 -p; +TEST dd if=/dev/urandom of=$M0/1/3/file bs=1M count=102; + +#The corresponding rename(3) should fail with EDQUOT ("Disk quota exceeded") +TEST ! mv $M0/1/3/ $M0/1/2/3_mvd; + +cleanup; diff --git a/xlators/features/quota/src/quota.c b/xlators/features/quota/src/quota.c index d46247241..5cbd9f02d 100644 --- a/xlators/features/quota/src/quota.c +++ b/xlators/features/quota/src/quota.c @@ -879,8 +879,7 @@ quota_fill_inodectx (xlator_t *this, inode_t *inode, dict_t *dict, ctx = (quota_inode_ctx_t *)(unsigned long)value; if ((((ctx == NULL) || (ctx->hard_lim == hard_lim)) - && (hard_lim < 0) && !((IA_ISREG (buf->ia_type)) - || (IA_ISLNK (buf->ia_type))))) { + && (hard_lim < 0) && !QUOTA_REG_OR_LNK_FILE (buf->ia_type))) { ret = 0; goto out; } @@ -902,7 +901,7 @@ quota_fill_inodectx (xlator_t *this, inode_t *inode, dict_t *dict, ctx->buf = *buf; - if (!(IA_ISREG (buf->ia_type) || IA_ISLNK (buf->ia_type))) { + if (!QUOTA_REG_OR_LNK_FILE (buf->ia_type)) { goto unlock; } @@ -1869,8 +1868,7 @@ quota_rename_cbk (call_frame_t *frame, void *cookie, xlator_t *this, goto out; } - if (IA_ISREG (local->oldloc.inode->ia_type) - || IA_ISLNK (local->oldloc.inode->ia_type)) { + if (QUOTA_REG_OR_LNK_FILE (local->oldloc.inode->ia_type)) { size = buf->ia_blocks * 512; } @@ -1881,8 +1879,7 @@ quota_rename_cbk (call_frame_t *frame, void *cookie, xlator_t *this, size); } - if (!(IA_ISREG (local->oldloc.inode->ia_type) - || IA_ISLNK (local->oldloc.inode->ia_type))) { + if (!QUOTA_REG_OR_LNK_FILE (local->oldloc.inode->ia_type)) { goto out; } @@ -1997,6 +1994,46 @@ unwind: } +static int32_t +quota_rename_get_size_cbk (call_frame_t *frame, void *cookie, xlator_t *this, + int32_t op_ret, int32_t op_errno, inode_t *inode, + struct iatt *buf, dict_t *xdata, + struct iatt *postparent) +{ + quota_local_t *local = NULL; + int32_t ret = 0; + int64_t *size = 0; + + GF_ASSERT (frame); + GF_VALIDATE_OR_GOTO_WITH_ERROR ("quota", this, out, op_errno, + EINVAL); + GF_VALIDATE_OR_GOTO_WITH_ERROR (this->name, xdata, out, op_errno, + EINVAL); + local = frame->local; + GF_ASSERT (local); + local->link_count = 1; + + if (op_ret < 0) + goto out; + + + ret = dict_get_bin (xdata, QUOTA_SIZE_KEY, (void **) &size); + if (ret < 0) { + gf_log (this->name, GF_LOG_WARNING, + "size key not present in dict"); + op_errno = EINVAL; + goto out; + } + local->delta = ntoh64 (*size); + quota_check_limit (frame, local->newloc.parent, this, + NULL, NULL); + return 0; + +out: + quota_handle_validate_error (local, -1, op_errno); + return 0; +} + int32_t quota_rename (call_frame_t *frame, xlator_t *this, loc_t *oldloc, loc_t *newloc, dict_t *xdata) @@ -2039,8 +2076,7 @@ quota_rename (call_frame_t *frame, xlator_t *this, loc_t *oldloc, local->link_count = 1; local->stub = stub; - if (IA_ISREG (oldloc->inode->ia_type) - || IA_ISLNK (oldloc->inode->ia_type)) { + if (QUOTA_REG_OR_LNK_FILE (oldloc->inode->ia_type)) { ret = quota_inode_ctx_get (oldloc->inode, this, &ctx, 0); if (ctx == NULL) { gf_log (this->name, GF_LOG_WARNING, @@ -2050,11 +2086,32 @@ quota_rename (call_frame_t *frame, xlator_t *this, loc_t *oldloc, oldloc->inode ? uuid_utoa (oldloc->inode->gfid) : "0"); local->delta = 0; + } else { - local->delta = ctx->buf.ia_blocks * 512; + + /* FIXME: We need to account for the size occupied by this + * inode on the target directory. To avoid double + * accounting, we need to modify enforcer to perform + * quota_check_limit only uptil the least common ancestor + * directory inode*/ + + /* FIXME: The following code assumes that regular files and + *linkfiles are present, in their entirety, in a single + brick. This *assumption is invalid in the case of + stripe.*/ + + local->delta = ctx->buf.ia_blocks * 512; } - } else { - local->delta = 0; + + } else if (IA_ISDIR (oldloc->inode->ia_type)) { + ret = quota_validate (frame, oldloc->inode, this, + quota_rename_get_size_cbk); + if (ret){ + op_errno = -ret; + goto err; + } + + return 0; } quota_check_limit (frame, newloc->parent, this, NULL, NULL); diff --git a/xlators/features/quota/src/quota.h b/xlators/features/quota/src/quota.h index 96c19e77e..02bc0d8b6 100644 --- a/xlators/features/quota/src/quota.h +++ b/xlators/features/quota/src/quota.h @@ -130,6 +130,9 @@ goto label; \ } while (0) +#define QUOTA_REG_OR_LNK_FILE(ia_type) \ + (IA_ISREG (ia_type) || IA_ISLNK (ia_type)) + struct quota_dentry { -- cgit