From 22926d82069caf452ac54137599f42394d82984e Mon Sep 17 00:00:00 2001 From: Vitaliy Filippov Date: Sun, 25 Feb 2024 02:27:10 +0300 Subject: [PATCH] Add basic NFS tests, fix bugs --- .gitea/workflows/test.yml | 18 ++++++++++++++++++ src/nfs_kv.cpp | 24 +++++++++++++++++++++--- src/nfs_kv.h | 4 +++- src/nfs_kv_create.cpp | 3 ++- src/nfs_kv_read.cpp | 9 +++++---- src/nfs_kv_readdir.cpp | 2 +- src/nfs_kv_setattr.cpp | 11 ++++++++--- src/nfs_kv_write.cpp | 17 ++++++++++------- src/nfs_proxy.cpp | 17 +++++++++++++++-- tests/run_tests.sh | 2 ++ tests/test_nfs.sh | 32 ++++++++++++++++++++++++++++++++ 11 files changed, 117 insertions(+), 22 deletions(-) create mode 100755 tests/test_nfs.sh diff --git a/.gitea/workflows/test.yml b/.gitea/workflows/test.yml index 683c1d58..0635c3bd 100644 --- a/.gitea/workflows/test.yml +++ b/.gitea/workflows/test.yml @@ -856,3 +856,21 @@ jobs: echo "" done + test_nfs: + runs-on: ubuntu-latest + needs: build + container: ${{env.TEST_IMAGE}}:${{github.sha}} + steps: + - name: Run test + id: test + timeout-minutes: 3 + run: /root/vitastor/tests/test_nfs.sh + - name: Print logs + if: always() && steps.test.outcome == 'failure' + run: | + for i in /root/vitastor/testdata/*.log /root/vitastor/testdata/*.txt; do + echo "-------- $i --------" + cat $i + echo "" + done + diff --git a/src/nfs_kv.cpp b/src/nfs_kv.cpp index 2dc28d04..46166d54 100644 --- a/src/nfs_kv.cpp +++ b/src/nfs_kv.cpp @@ -29,11 +29,29 @@ nfstime3 nfstime_from_str(const std::string & s) return t; } +static std::string timespec_to_str(timespec t) +{ + char buf[64]; + snprintf(buf, sizeof(buf), "%ju.%09ju", t.tv_sec, t.tv_nsec); + int l = strlen(buf); + while (l > 0 && buf[l-1] == '0') + l--; + if (l > 0 && buf[l-1] == '.') + l--; + buf[l] = 0; + return buf; +} + std::string nfstime_to_str(nfstime3 t) { - char buf[32]; - snprintf(buf, sizeof(buf), "%u.%09u", t.seconds, t.nseconds); - return buf; + return timespec_to_str((timespec){ .tv_sec = t.seconds, .tv_nsec = t.nseconds }); +} + +std::string nfstime_now_str() +{ + timespec t; + clock_gettime(CLOCK_REALTIME, &t); + return timespec_to_str(t); } int kv_map_type(const std::string & type) diff --git a/src/nfs_kv.h b/src/nfs_kv.h index 34c92e89..abfcacd5 100644 --- a/src/nfs_kv.h +++ b/src/nfs_kv.h @@ -46,7 +46,7 @@ struct kv_inode_extend_t struct kv_fs_state_t { std::map list_cookies; - uint64_t fs_next_id = 0, fs_allocated_id = 0; + uint64_t fs_next_id = 1, fs_allocated_id = 0; std::vector unallocated_ids; std::vector allocating_shared; uint64_t cur_shared_inode = 0, cur_shared_offset = 0; @@ -58,11 +58,13 @@ struct shared_file_header_t uint64_t magic = 0; uint64_t inode = 0; uint64_t size = 0; + uint64_t alloc = 0; }; nfsstat3 vitastor_nfs_map_err(int err); nfstime3 nfstime_from_str(const std::string & s); std::string nfstime_to_str(nfstime3 t); +std::string nfstime_now_str(); int kv_map_type(const std::string & type); fattr3 get_kv_attributes(nfs_client_t *self, uint64_t ino, json11::Json attrs); std::string kv_direntry_key(uint64_t dir_ino, const std::string & filename); diff --git a/src/nfs_kv_create.cpp b/src/nfs_kv_create.cpp index 1f6668f3..deac1c33 100644 --- a/src/nfs_kv_create.cpp +++ b/src/nfs_kv_create.cpp @@ -105,7 +105,8 @@ static void kv_do_create(kv_create_state *st) } st->new_id = new_id; auto direntry = json11::Json::object{ { "ino", st->new_id } }; - if (st->attrobj["type"].string_value() == "dir") + if (st->attrobj.find("type") != st->attrobj.end() && + st->attrobj["type"].string_value() == "dir") { direntry["type"] = "dir"; } diff --git a/src/nfs_kv_read.cpp b/src/nfs_kv_read.cpp index c5e6d2b3..aa36c887 100644 --- a/src/nfs_kv_read.cpp +++ b/src/nfs_kv_read.cpp @@ -85,8 +85,7 @@ resume_2: return; } auto hdr = ((shared_file_header_t*)st->aligned_buf); - if (hdr->magic != SHARED_FILE_MAGIC_V1 || hdr->inode != st->ino || - align_shared_size(st->self, hdr->size) > align_shared_size(st->self, st->ientry["size"].uint64_value())) + if (hdr->magic != SHARED_FILE_MAGIC_V1 || hdr->inode != st->ino) { // Got unrelated data - retry from the beginning free(st->aligned_buf); @@ -101,7 +100,7 @@ resume_2: } } st->aligned_offset = (st->offset & ~(st->self->parent->pool_alignment-1)); - st->aligned_size = ((st->offset + st->size + st->self->parent->pool_alignment) & + st->aligned_size = ((st->offset + st->size + st->self->parent->pool_alignment-1) & ~(st->self->parent->pool_alignment-1)) - st->aligned_offset; st->aligned_buf = (uint8_t*)malloc_or_die(st->aligned_size); st->buf = st->aligned_buf + st->offset - st->aligned_offset; @@ -121,7 +120,7 @@ resume_2: return; resume_3: auto cb = std::move(st->cb); - cb(st->res); + cb(st->res < 0 ? st->res : 0); return; } @@ -157,6 +156,8 @@ int kv_nfs3_read_proc(void *opaque, rpc_op_t *rop) rpc_queue_reply(st->rop); delete st; }; + if (st->self->parent->trace) + fprintf(stderr, "[%d] READ %ju %ju+%ju\n", st->self->nfs_fd, st->ino, st->offset, st->size); nfs_kv_continue_read(st, 0); return 1; } diff --git a/src/nfs_kv_readdir.cpp b/src/nfs_kv_readdir.cpp index b7e8c409..7a7ac616 100644 --- a/src/nfs_kv_readdir.cpp +++ b/src/nfs_kv_readdir.cpp @@ -235,7 +235,7 @@ resume_2: st->self->parent->db->list_next(st->list_handle, NULL); return; resume_3: - if (st->res == -ENOENT || st->cur_key.size() > st->prefix.size() || st->cur_key.substr(0, st->prefix.size()) != st->prefix) + if (st->res == -ENOENT || st->cur_key.size() < st->prefix.size() || st->cur_key.substr(0, st->prefix.size()) != st->prefix) { st->self->parent->db->list_close(st->list_handle); st->list_handle = NULL; diff --git a/src/nfs_kv_setattr.cpp b/src/nfs_kv_setattr.cpp index 35aa9e71..1a64bd07 100644 --- a/src/nfs_kv_setattr.cpp +++ b/src/nfs_kv_setattr.cpp @@ -147,11 +147,16 @@ int kv_nfs3_setattr_proc(void *opaque, rpc_op_t *rop) st->set_attrs["uid"] = (uint64_t)args->new_attributes.uid.uid; if (args->new_attributes.gid.set_it) st->set_attrs["gid"] = (uint64_t)args->new_attributes.gid.gid; - if (args->new_attributes.atime.set_it) + if (args->new_attributes.atime.set_it == SET_TO_SERVER_TIME) + st->set_attrs["atime"] = nfstime_now_str(); + else if (args->new_attributes.atime.set_it == SET_TO_CLIENT_TIME) st->set_attrs["atime"] = nfstime_to_str(args->new_attributes.atime.atime); - if (args->new_attributes.mtime.set_it) + if (args->new_attributes.mtime.set_it == SET_TO_SERVER_TIME) + st->set_attrs["mtime"] = nfstime_now_str(); + else if (args->new_attributes.mtime.set_it == SET_TO_CLIENT_TIME) st->set_attrs["mtime"] = nfstime_to_str(args->new_attributes.mtime.mtime); - fprintf(stderr, "SETATTR %ju ATTRS %s\n", st->ino, json11::Json(st->set_attrs).dump().c_str()); + if (st->self->parent->trace) + fprintf(stderr, "[%d] SETATTR %ju ATTRS %s\n", st->self->nfs_fd, st->ino, json11::Json(st->set_attrs).dump().c_str()); st->cb = [st](int res) { auto reply = (SETATTR3res*)st->rop->reply; diff --git a/src/nfs_kv_write.cpp b/src/nfs_kv_write.cpp index 053de596..063f0524 100644 --- a/src/nfs_kv_write.cpp +++ b/src/nfs_kv_write.cpp @@ -272,7 +272,7 @@ static bool nfs_do_shared_readmodify(nfs_kv_write_state *st, int base_state, int else if (state == base_state) goto resume_0; assert(!st->aligned_buf); st->aligned_size = unshare - ? sizeof(shared_file_header_t) + (st->new_size + st->self->parent->pool_alignment-1) & ~(st->self->parent->pool_alignment-1) + ? sizeof(shared_file_header_t) + ((st->new_size + st->self->parent->pool_alignment-1) & ~(st->self->parent->pool_alignment-1)) : align_shared_size(st->self, st->new_size); st->aligned_buf = (uint8_t*)malloc_or_die(st->aligned_size); memset(st->aligned_buf + sizeof(shared_file_header_t), 0, st->offset); @@ -286,14 +286,15 @@ resume_0: { auto cb = std::move(st->cb); cb(st->res); - return true; + return false; } auto hdr = ((shared_file_header_t*)st->aligned_buf); - if (hdr->magic != SHARED_FILE_MAGIC_V1 || hdr->inode != st->ino || - align_shared_size(st->self, hdr->size) > align_shared_size(st->self, st->ientry["size"].uint64_value())) + if (hdr->magic != SHARED_FILE_MAGIC_V1 || hdr->inode != st->ino) { // Got unrelated data - retry from the beginning st->allow_cache = false; + free(st->aligned_buf); + st->aligned_buf = NULL; nfs_kv_continue_write(st, 0); return false; } @@ -302,6 +303,7 @@ resume_0: .magic = SHARED_FILE_MAGIC_V1, .inode = st->ino, .size = st->new_size, + .alloc = st->aligned_size, }; memcpy(st->aligned_buf + sizeof(shared_file_header_t) + st->offset, st->buf, st->size); memset(st->aligned_buf + sizeof(shared_file_header_t) + st->offset + st->size, 0, @@ -432,6 +434,7 @@ static void nfs_kv_extend_inode(nfs_kv_write_state *st, int state) auto ientry = json11::Json::parse(old_value, err).object_items(); if (err != "") { + fprintf(stderr, "Invalid JSON in inode %lu = %s: %s\n", st->ino, old_value.c_str(), err.c_str()); st->res2 = -EINVAL; return false; } @@ -578,9 +581,7 @@ resume_0: }, st->allow_cache); return; resume_1: - if (st->res < 0 || - st->ientry["type"].uint64_value() != 0 && - st->ientry["type"].uint64_value() != NF3REG) + if (st->res < 0 || kv_map_type(st->ientry["type"].string_value()) != NF3REG) { auto cb = std::move(st->cb); cb(st->res == 0 ? -EINVAL : st->res); @@ -769,6 +770,8 @@ int kv_nfs3_write_proc(void *opaque, rpc_op_t *rop) st->ino = kv_fh_inode(args->file); st->offset = args->offset; st->size = (args->count > args->data.size ? args->data.size : args->count); + if (st->self->parent->trace) + fprintf(stderr, "[%d] WRITE %ju %ju+%ju\n", st->self->nfs_fd, st->ino, st->offset, st->size); if (!st->ino || st->size > MAX_REQUEST_SIZE) { *reply = (WRITE3res){ .status = NFS3ERR_INVAL }; diff --git a/src/nfs_proxy.cpp b/src/nfs_proxy.cpp index 11aa055a..3428a470 100644 --- a/src/nfs_proxy.cpp +++ b/src/nfs_proxy.cpp @@ -98,7 +98,7 @@ void nfs_proxy_t::run(json11::Json cfg) srand48(tv.tv_sec*1000000000 + tv.tv_nsec); server_id = (uint64_t)lrand48() | ((uint64_t)lrand48() << 31) | ((uint64_t)lrand48() << 62); // Parse options - trace = cfg["log_level"].uint64_value() > 5; + trace = cfg["log_level"].uint64_value() > 5 || cfg["trace"].uint64_value() > 0; bind_address = cfg["bind"].string_value(); if (bind_address == "") bind_address = "0.0.0.0"; @@ -156,7 +156,15 @@ void nfs_proxy_t::run(json11::Json cfg) check_default_pool(); // Check if we're using VitastorFS fs_kv_inode = cfg["fs"].uint64_value(); - if (!fs_kv_inode && cfg["fs"].is_string()) + if (fs_kv_inode) + { + if (!INODE_POOL(fs_kv_inode)) + { + fprintf(stderr, "FS metadata inode number must include pool\n"); + exit(1); + } + } + else if (cfg["fs"].is_string()) { for (auto & ic: cli->st_cli.inode_config) { @@ -166,6 +174,11 @@ void nfs_proxy_t::run(json11::Json cfg) break; } } + if (!fs_kv_inode) + { + fprintf(stderr, "FS metadata image \"%s\" does not exist\n", cfg["fs"].string_value().c_str()); + exit(1); + } } readdir_getattr_parallel = cfg["readdir_getattr_parallel"].uint64_value(); if (!readdir_getattr_parallel) diff --git a/tests/run_tests.sh b/tests/run_tests.sh index fcf079ec..5212792e 100755 --- a/tests/run_tests.sh +++ b/tests/run_tests.sh @@ -68,3 +68,5 @@ SCHEME=xor ./test_scrub.sh PG_SIZE=3 ./test_scrub.sh PG_SIZE=6 PG_MINSIZE=4 OSD_COUNT=6 SCHEME=ec ./test_scrub.sh SCHEME=ec ./test_scrub.sh + +./test_nfs.sh diff --git a/tests/test_nfs.sh b/tests/test_nfs.sh new file mode 100755 index 00000000..9058e42b --- /dev/null +++ b/tests/test_nfs.sh @@ -0,0 +1,32 @@ +#!/bin/bash -ex + +PG_COUNT=16 +. `dirname $0`/run_3osds.sh + +build/src/vitastor-cli --etcd_address $ETCD_URL create -s 10G fsmeta +build/src/vitastor-nfs --fs fsmeta --etcd_address $ETCD_URL --portmap 0 --port 2050 --foreground 1 --trace 1 >>./testdata/nfs.log 2>&1 & +NFS_PID=$! + +mkdir -p testdata/nfs +sudo mount localhost:/ ./testdata/nfs -o port=2050,mountport=2050,nfsvers=3,soft,nolock,tcp +MNT=$(pwd)/testdata/nfs +trap "sudo umount -f $MNT"' || true; kill -9 $(jobs -p)' EXIT + +# write small file +ls -l ./testdata/nfs +dd if=/dev/urandom of=./testdata/f1 bs=100k count=1 +cp testdata/f1 ./testdata/nfs/ +sudo umount ./testdata/nfs/ +sudo mount localhost:/ ./testdata/nfs -o port=2050,mountport=2050,nfsvers=3,soft,nolock,tcp +ls -l ./testdata/nfs | grep f1 +diff ./testdata/f1 ./testdata/nfs/f1 + +# extend it to large file +dd if=/dev/urandom of=./testdata/f1 bs=1M count=1 +cp testdata/f1 ./testdata/nfs/ +sudo umount ./testdata/nfs/ +sudo mount localhost:/ ./testdata/nfs -o port=2050,mountport=2050,nfsvers=3,soft,nolock,tcp +ls -l ./testdata/nfs | grep f1 +diff ./testdata/f1 ./testdata/nfs/f1 + +format_green OK