From: Venky Shankar Date: Fri, 25 Aug 2023 12:50:13 +0000 (+0530) Subject: Revert "mds: align quota.max_bytes to 4MB or 4KB" X-Git-Tag: v19.0.0~491^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=6d8cb29d1e96fe66ec8838bf875ab69d3a058503;p=ceph.git Revert "mds: align quota.max_bytes to 4MB or 4KB" This seems to be causing quota related test failures in the fs suoite. @lxbsz will push a separate PR addressing the issue in tracker #56397. Signed-off-by: Venky Shankar --- diff --git a/doc/cephfs/quota.rst b/doc/cephfs/quota.rst index 6ff984f3d9e..e78173bcc3e 100644 --- a/doc/cephfs/quota.rst +++ b/doc/cephfs/quota.rst @@ -15,13 +15,10 @@ If the extended attributes appear on a directory that means a quota is configured there. If they are not present then no quota is set on that directory (although one may still be configured on a parent directory). -The value of ``ceph.quota.max_bytes`` must be aligned to 4MB if greater -than or equal to 4MB, otherwise it must be aligned to 4KB. - To set a quota, set the extended attribute on a CephFS directory with a value:: - setfattr -n ceph.quota.max_bytes -v 104857600 /some/dir # 100 MB + setfattr -n ceph.quota.max_bytes -v 100000000 /some/dir # 100 MB setfattr -n ceph.quota.max_files -v 10000 /some/dir # 10,000 files ``ceph.quota.max_bytes`` can also be set using human-friendly units:: @@ -36,7 +33,7 @@ To view quota limit:: $ getfattr -n ceph.quota.max_bytes /some/dir # file: dir1/ - ceph.quota.max_bytes="104857600" + ceph.quota.max_bytes="100000000" $ $ getfattr -n ceph.quota.max_files /some/dir # file: dir1/ diff --git a/qa/tasks/cephfs/test_cephfs_shell.py b/qa/tasks/cephfs/test_cephfs_shell.py index 86d8342f732..9f743476270 100644 --- a/qa/tasks/cephfs/test_cephfs_shell.py +++ b/qa/tasks/cephfs/test_cephfs_shell.py @@ -864,18 +864,18 @@ class TestQuota(TestCephFSShell): def test_set(self): self.create_dir() - set_values = ('4096', '2') + set_values = ('6', '2') self.assertTupleEqual(self.set_and_get_quota_vals(set_values), set_values) def test_replace_values(self): self.test_set() - set_values = ('8192', '4') + set_values = ('20', '4') self.assertTupleEqual(self.set_and_get_quota_vals(set_values), set_values) def test_set_invalid_dir(self): - set_values = ('4096', '5') + set_values = ('5', '5') try: self.assertTupleEqual(self.set_and_get_quota_vals( set_values, False), set_values) @@ -920,8 +920,9 @@ class TestQuota(TestCephFSShell): filename = 'test_file' file_abspath = path.join(dir_abspath, filename) try: - # Write should fail as bytes quota is set to 4096 - self.mount_a.write_n_mb(file_abspath, 1) + # Write should fail as bytes quota is set to 6 + self.mount_a.client_remote.write_file(file_abspath, + 'Disk raise Exception') raise Exception("Write should have failed") except CommandFailedError: # Test should pass only when write command fails diff --git a/src/client/Client.cc b/src/client/Client.cc index 94ed9ca9bdb..b3c93a2e0fc 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -12152,7 +12152,9 @@ int Client::statfs(const char *path, struct statvfs *stbuf, * blocks. We use 4MB only because it is big enough, and because it * actually *is* the (ceph) default block size. */ - stbuf->f_frsize = CEPH_4M_BLOCK_SIZE; + const int CEPH_BLOCK_SHIFT = 22; + stbuf->f_frsize = 1 << CEPH_BLOCK_SHIFT; + stbuf->f_bsize = 1 << CEPH_BLOCK_SHIFT; stbuf->f_files = total_files_on_fs; stbuf->f_ffree = -1; stbuf->f_favail = -1; @@ -12192,19 +12194,11 @@ int Client::statfs(const char *path, struct statvfs *stbuf, // Special case: if there is a size quota set on the Inode acting // as the root for this client mount, then report the quota status // as the filesystem statistics. - fsblkcnt_t total = quota_root->quota.max_bytes >> CEPH_4M_BLOCK_SHIFT; - const fsblkcnt_t used = quota_root->rstat.rbytes >> CEPH_4M_BLOCK_SHIFT; + const fsblkcnt_t total = quota_root->quota.max_bytes >> CEPH_BLOCK_SHIFT; + const fsblkcnt_t used = quota_root->rstat.rbytes >> CEPH_BLOCK_SHIFT; // It is possible for a quota to be exceeded: arithmetic here must // handle case where used > total. - fsblkcnt_t free = total > used ? total - used : 0; - - // For quota size less than 4KB, report the total=used=4KB,free=0 - // when quota is full and total=free=4KB, used=0 otherwise. - if (!total) { - total = 1; - free = quota_root->quota.max_bytes > quota_root->rstat.rbytes ? 1 : 0; - stbuf->f_frsize = CEPH_4K_BLOCK_SIZE; - } + const fsblkcnt_t free = total > used ? total - used : 0; stbuf->f_blocks = total; stbuf->f_bfree = free; @@ -12213,11 +12207,10 @@ int Client::statfs(const char *path, struct statvfs *stbuf, // General case: report the cluster statistics returned from RADOS. Because // multiple pools may be used without one filesystem namespace via // layouts, this is the most correct thing we can do. - stbuf->f_blocks = stats.kb >> CEPH_4K_BLOCK_SHIFT; - stbuf->f_bfree = stats.kb_avail >> CEPH_4K_BLOCK_SHIFT; - stbuf->f_bavail = stats.kb_avail >> CEPH_4K_BLOCK_SHIFT; + stbuf->f_blocks = stats.kb >> (CEPH_BLOCK_SHIFT - 10); + stbuf->f_bfree = stats.kb_avail >> (CEPH_BLOCK_SHIFT - 10); + stbuf->f_bavail = stats.kb_avail >> (CEPH_BLOCK_SHIFT - 10); } - stbuf->f_bsize = stbuf->f_frsize; return rval; } diff --git a/src/include/cephfs/types.h b/src/include/cephfs/types.h index 049ef8e3fcf..cca0a619305 100644 --- a/src/include/cephfs/types.h +++ b/src/include/cephfs/types.h @@ -45,13 +45,6 @@ #define CEPH_FS_ONDISK_MAGIC "ceph fs volume v011" #define MAX_MDS 0x100 -#define CEPH_4M_BLOCK_SHIFT 22 -#define CEPH_4M_BLOCK_SIZE (1 << CEPH_4M_BLOCK_SHIFT) // 4MB -#define CEPH_4K_BLOCK_SHIFT 12 -#define CEPH_4K_BLOCK_SIZE (1 << CEPH_4K_BLOCK_SHIFT) // 4KB - -#define IS_ALIGNED(x, a) (((x) & (int64_t(a) - 1)) == 0) - BOOST_STRONG_TYPEDEF(uint64_t, mds_gid_t) extern const mds_gid_t MDS_GID_NONE; diff --git a/src/mds/Server.cc b/src/mds/Server.cc index 13b499192a8..02cfdb4ef68 100644 --- a/src/mds/Server.cc +++ b/src/mds/Server.cc @@ -5925,17 +5925,11 @@ int Server::parse_quota_vxattr(string name, string value, quota_info_t *quota) return r; } } else if (name == "quota.max_bytes") { - /* - * The "quota.max_bytes" must be aligned to 4MB if greater than or - * equal to 4MB, otherwise must be aligned to 4KB. - */ string cast_err; int64_t q = strict_iec_cast(value, &cast_err); - if(!cast_err.empty() || - (!IS_ALIGNED(q, CEPH_4M_BLOCK_SIZE) && - (q < CEPH_4M_BLOCK_SIZE && !IS_ALIGNED(q, CEPH_4K_BLOCK_SIZE)))) { + if(!cast_err.empty()) { dout(10) << __func__ << ": failed to parse quota.max_bytes: " - << cast_err << dendl; + << cast_err << dendl; return -CEPHFS_EINVAL; } quota->max_bytes = q; diff --git a/src/test/pybind/test_cephfs.py b/src/test/pybind/test_cephfs.py index 3c4c9d5709a..896565ceb94 100644 --- a/src/test/pybind/test_cephfs.py +++ b/src/test/pybind/test_cephfs.py @@ -825,11 +825,9 @@ def test_get_pool(testdir): def test_disk_quota_exceeeded_error(testdir): cephfs.mkdir("/dir-1", 0o755) - cephfs.setxattr("/dir-1", "ceph.quota.max_bytes", b"4096", 0) + cephfs.setxattr("/dir-1", "ceph.quota.max_bytes", b"5", 0) fd = cephfs.open(b'/dir-1/file-1', 'w', 0o755) - cephfs.ftruncate(fd, 4092) - cephfs.lseek(fd, 4090, os.SEEK_SET) - assert_raises(libcephfs.DiskQuotaExceeded, cephfs.write, fd, b"abcdeghiklmnopqrstuvwxyz1234567890qwertyuioddd", -1) + assert_raises(libcephfs.DiskQuotaExceeded, cephfs.write, fd, b"abcdeghiklmnopqrstuvwxyz", 0) cephfs.close(fd) cephfs.unlink(b"/dir-1/file-1")