From 249517492a9621d3441d9ee635fcc37fe23e859d Mon Sep 17 00:00:00 2001 From: Hadi Moshayedi Date: Thu, 25 Jan 2024 19:21:53 -0800 Subject: [PATCH] Add test coverage data & increase coverage. Adds test coverage data and generates coverage report using `lcov`. To generate and see a coverage report do: ``` export SPDK_PATH=/path/to/spdk make COVERAGE=true make check make coverage_report ruby -run -e httpd ./coverage_report/ -p 8000 ``` Furthermore, made some changes to `memcheck_ubi` and `test_ubi` test programs to test more combinations so we have higher coverage. Also, added coverage check to the github flow to detect regressions in test coverage. --- .github/scripts/check-coverage.sh | 10 ++ .github/workflows/release.yaml | 18 ++-- .gitignore | 2 + Makefile | 15 ++- test/memcheck_ubi.c | 46 +++++--- test/test_conf.json | 63 ++++++++++- test/test_ubi.c | 171 +++++++++++++++++++++++------- 7 files changed, 261 insertions(+), 64 deletions(-) create mode 100755 .github/scripts/check-coverage.sh diff --git a/.github/scripts/check-coverage.sh b/.github/scripts/check-coverage.sh new file mode 100755 index 0000000..47caa81 --- /dev/null +++ b/.github/scripts/check-coverage.sh @@ -0,0 +1,10 @@ +#!/bin/bash + +make coverage + +target=70.0 +p=$(lcov --summary coverage.info | grep "lines" | grep -Eo "[0-9]+\.[0-9]+") +if (( $(echo "$p < $target" | bc -l) )); then + echo "Error: Coverage $p% is less than $target%" + exit 1 +fi diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 659fb27..1fcea38 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -34,7 +34,7 @@ jobs: - name: Install dependencies run: | sudo apt-get update - sudo apt install liburing-dev + sudo apt install liburing-dev lcov sudo spdk/scripts/pkgdep.sh - name: Configure run: spdk/configure --with-crypto --with-vhost --target-arch=corei7 --prefix=$INSTALL_PREFIX --pydir=$INSTALL_PREFIX/python/lib --disable-unit-tests --disable-tests --disable-examples @@ -47,14 +47,20 @@ jobs: cp -R python/* $INSTALL_PREFIX/python/ mkdir $INSTALL_PREFIX/scripts cp scripts/rpc.py $INSTALL_PREFIX/scripts + - name: Run tests and check coverage + run: | + sudo spdk/scripts/setup.sh + export SPDK_PATH=$INSTALL_PREFIX + make clean + make COVERAGE=true + make check + .github/scripts/check-coverage.sh - name: Build bdev_ubi run: | - SPDK_PATH=$INSTALL_PREFIX make + export SPDK_PATH=$INSTALL_PREFIX + make clean + make cp build/bin/vhost_ubi $INSTALL_PREFIX/bin/ - - name: Run tests - run: | - sudo spdk/scripts/setup.sh - SPDK_PATH=$INSTALL_PREFIX make check - name: Package run: | cd $INSTALL_PREFIX/.. diff --git a/.gitignore b/.gitignore index 91366b1..b8d203e 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,5 @@ build/* .vscode/* valgrind.log +coverage.info +coverage_report/* diff --git a/Makefile b/Makefile index 12eb80c..91a5f0e 100644 --- a/Makefile +++ b/Makefile @@ -4,6 +4,7 @@ APP_DIR := app TEST_DIR := test OBJ_DIR := build/obj BIN_DIR := build/bin +TEST_BDEVS := --bdev ubi0 --bdev ubi_nosync --bdev ubi_directio --bdev ubi_copy_on_read ifneq ($(MAKECMDGOALS),format) PKG_CONFIG_PATH = $(SPDK_PATH)/lib/pkgconfig @@ -20,6 +21,10 @@ CFLAGS := -D_GNU_SOURCE -Iinclude -Wall -g -O3 -I$(SPDK_PATH)/include LDFLAGS := -Wl,--whole-archive,-Bstatic $(SPDK_DPDK_LIB) -Wl,--no-whole-archive -luring -Wl,-Bdynamic $(SYS_LIB) endif +ifeq ($(COVERAGE),true) + CFLAGS += -fprofile-arcs -ftest-coverage +endif + # Automatically generate a list of source files (.c) and object files (.o) SRCS := $(wildcard $(SRC_DIR)/*.c) OBJS := $(SRCS:$(SRC_DIR)/%.c=$(OBJ_DIR)/%.o) @@ -58,14 +63,18 @@ $(BIN_DIR)/test_disk.raw: $(BIN_DIR)/test_image.raw truncate --size 100M $@ check: - sudo ./build/bin/test_ubi --cpumask [0,1,2] --json test/test_conf.json + sudo ./build/bin/test_ubi --cpumask [0,1,2] --json test/test_conf.json $(TEST_BDEVS) valgrind: - sudo valgrind ./build/bin/memcheck_ubi --cpumask [0] --json test/test_conf.json + sudo valgrind ./build/bin/memcheck_ubi --cpumask [0] --json test/test_conf.json $(TEST_BDEVS) + +coverage: + lcov --capture --directory . --exclude=`pwd`/test/'*.c' --no-external --output-file coverage.info > /dev/null + genhtml coverage.info --output-directory coverage_report # Clean up build artifacts clean: - @rm -rf $(OBJ_DIR) $(BIN_DIR) + @rm -rf $(OBJ_DIR) $(BIN_DIR) coverage.info coverage_report # Automatically format source files format: diff --git a/test/memcheck_ubi.c b/test/memcheck_ubi.c index 586756e..737ab11 100644 --- a/test/memcheck_ubi.c +++ b/test/memcheck_ubi.c @@ -14,11 +14,14 @@ #define BLOCK_SIZE 512 #define MAX_OPS 5000 +#define MAX_BDEVS 10 struct { - char *bdev_name; + char *bdev_names[MAX_BDEVS]; + int n_bdevs; } g_opts; +size_t g_bdevs_tested = 0; struct test_state { struct spdk_bdev_desc *bdev_desc; struct spdk_io_channel *ch; @@ -43,6 +46,7 @@ static struct option g_cmdline_opts[] = {{ {.name = NULL}}; static void enqueue_io_ops(void *arg); +static void open_bdev(void *arg); #define continue_with_fn(fn) \ { \ @@ -70,8 +74,6 @@ static void fail_tests(void *arg) { } static void succeed_tests(void *arg) { - close_bdev(); - SPDK_NOTICELOG("Tests succeeded.\n"); spdk_app_stop(0); } @@ -86,9 +88,11 @@ static void io_completion_cb(struct spdk_bdev_io *bdev_io, bool success, void *a if (g_test_state.n_ops_finished != g_test_state.n_ops_queued) return; - // reached target op count - if (g_test_state.n_ops_finished >= g_test_state.n_ops_target) - continue_with_fn(succeed_tests); + // reached target op count. continue with testing the next bdev. + if (g_test_state.n_ops_finished >= g_test_state.n_ops_target) { + close_bdev(); + continue_with_fn(open_bdev); + } continue_with_fn(enqueue_io_ops); } @@ -144,7 +148,14 @@ static void enqueue_io_ops(void *arg) { } static void open_bdev(void *arg) { - char *name = g_opts.bdev_name ? g_opts.bdev_name : DEFAULT_BDEV_NAME; + if (g_bdevs_tested >= g_opts.n_bdevs) { + continue_with_fn(succeed_tests); + } + + char *name = g_opts.bdev_names[g_bdevs_tested++]; + memset(&g_test_state, 0, sizeof(g_test_state)); + g_test_state.n_ops_target = MAX_OPS; + int rc = spdk_bdev_open_ext(name, true, ubi_event_cb, NULL, &g_test_state.bdev_desc); if (rc < 0) { SPDK_ERRLOG("Could not open bdev %s: %s\n", name, strerror(-rc)); @@ -160,18 +171,18 @@ static void open_bdev(void *arg) { continue_with_fn(enqueue_io_ops); }; -static void start_tests(void *arg) { - memset(&g_test_state, 0, sizeof(g_test_state)); - g_test_state.n_ops_target = MAX_OPS; - continue_with_fn(open_bdev); -} +static void start_tests(void *arg) { continue_with_fn(open_bdev); } static void usage(void) { printf(" -bdev Block device to be used for testing.\n"); } static int parse_arg(int argc, char *argv) { switch (argc) { case MEMCHECK_OPTION_BDEV: - g_opts.bdev_name = strdup(argv); + if (g_opts.n_bdevs >= MAX_BDEVS) { + fprintf(stderr, "Too many bdevs.\n"); + exit(-1); + } + g_opts.bdev_names[g_opts.n_bdevs++] = strdup(argv); break; default: return -EINVAL; @@ -191,12 +202,19 @@ int main(int argc, char **argv) { exit(rc); } + if (g_opts.n_bdevs == 0) { + g_opts.n_bdevs = 1; + g_opts.bdev_names[0] = strdup(DEFAULT_BDEV_NAME); + } + rc = spdk_app_start(&opts, start_tests, NULL); if (rc) { SPDK_ERRLOG("Error occured while testing bdev_ubi.\n"); } - free(g_opts.bdev_name); + for (int i = 0; i < g_opts.n_bdevs; i++) + free(g_opts.bdev_names[i]); + spdk_app_fini(); return rc; diff --git a/test/test_conf.json b/test/test_conf.json index b946bcb..3ed1449 100644 --- a/test/test_conf.json +++ b/test/test_conf.json @@ -19,7 +19,68 @@ "image_path": "build/bin/test_image.raw", "stripe_size_kb": 1024, "copy_on_read": false, - "directio": true + "directio": false, + "no_sync": false + } + }, + { + "method": "bdev_malloc_create", + "params": { + "name": "malloc1", + "block_size": 512, + "num_blocks": 204800 + } + }, + { + "method": "bdev_ubi_create", + "params": { + "name": "ubi_copy_on_read", + "base_bdev": "malloc1", + "image_path": "build/bin/test_image.raw", + "stripe_size_kb": 1024, + "copy_on_read": true, + "directio": false, + "no_sync": false + } + }, + { + "method": "bdev_malloc_create", + "params": { + "name": "malloc2", + "block_size": 512, + "num_blocks": 204800 + } + }, + { + "method": "bdev_ubi_create", + "params": { + "name": "ubi_directio", + "base_bdev": "malloc2", + "image_path": "build/bin/test_image.raw", + "stripe_size_kb": 1024, + "copy_on_read": false, + "directio": true, + "no_sync": false + } + }, + { + "method": "bdev_malloc_create", + "params": { + "name": "malloc3", + "block_size": 512, + "num_blocks": 204800 + } + }, + { + "method": "bdev_ubi_create", + "params": { + "name": "ubi_nosync", + "base_bdev": "malloc3", + "image_path": "build/bin/test_image.raw", + "stripe_size_kb": 1024, + "copy_on_read": false, + "directio": false, + "no_sync": true } }, { diff --git a/test/test_ubi.c b/test/test_ubi.c index 377dffd..5f01479 100644 --- a/test/test_ubi.c +++ b/test/test_ubi.c @@ -9,17 +9,25 @@ #include "spdk/vmd.h" #include -#define BDEV_NAME "ubi0" +#define DEFAULT_BDEV_NAME "ubi0" #define IMAGE_PATH "build/bin/test_image.raw" #define MAX_BLOCK_SIZE 4096 -#define IMAGE_SIZE (40 * 1024 * 1024) +#define MAX_BDEVS 10 +struct { + char *bdev_names[MAX_BDEVS]; + int n_bdevs; +} g_opts; + +size_t g_bdevs_tested = 0; struct test_state { struct spdk_bdev_desc *bdev_desc; struct spdk_io_channel *ch; FILE *image_file; uint32_t blocklen; + uint64_t blockcnt; + uint64_t image_size; uint64_t n_image_blocks; uint32_t n_tests; @@ -34,21 +42,36 @@ struct ubi_io_request { bool success; }; +enum dd_cmdline_opts { + MEMCHECK_OPTION_BDEV = 0x1000, +}; + +static struct option g_cmdline_opts[] = {{ + .name = "bdev", + .has_arg = 1, + .flag = NULL, + .val = MEMCHECK_OPTION_BDEV, + }, + {.name = NULL}}; + /* forward declarations */ static void stop_init_thread(void *arg); -static void open_bdev(void *arg); -static void close_bdev(void *arg); +static void open_io_channel(void *arg); +static void close_io_channel(void *arg); static void exit_io_thread(void *arg); static void ubi_event_cb(enum spdk_bdev_event_type type, struct spdk_bdev *bdev, void *event_ctx); static void ubi_bdev_write(void *arg); static void ubi_bdev_read(void *arg); +static void ubi_bdev_flush(void *arg); static void io_completion_cb(struct spdk_bdev_io *bdev_io, bool success, void *arg); static void run_ut_thread(void *arg); +static void run_bdev_tests(struct test_state *state, const char *name); static bool test_read(struct test_state *state, uint32_t start, uint32_t count); static bool test_write(struct test_state *state, uint32_t start, uint32_t count); +static bool test_random_ops(struct test_state *state, uint32_t count); static bool verify_image_block(struct test_state *state, uint64_t block, char *buf); static bool file_size(FILE *f, uint64_t *out); @@ -81,34 +104,21 @@ static void exit_io_thread(void *arg) { wake_ut_thread(); } -static void open_bdev(void *arg) { +static void open_io_channel(void *arg) { struct test_state *state = arg; - int rc; - rc = spdk_bdev_open_ext(BDEV_NAME, true, ubi_event_cb, NULL, &state->bdev_desc); - if (rc < 0) { - SPDK_ERRLOG("Could not open bdev %s: %s\n", BDEV_NAME, strerror(-rc)); - goto done; - } state->ch = spdk_bdev_get_io_channel(state->bdev_desc); if (state->ch == NULL) { SPDK_ERRLOG("Could not get I/O channel: %s\n", strerror(ENOMEM)); - goto done; } - struct spdk_bdev *bdev = spdk_bdev_desc_get_bdev(state->bdev_desc); - state->blocklen = bdev->blocklen; - -done: wake_ut_thread(); } -static void close_bdev(void *arg) { +static void close_io_channel(void *arg) { struct test_state *state = arg; if (state->ch) spdk_put_io_channel(state->ch); - if (state->bdev_desc) - spdk_bdev_close(state->bdev_desc); wake_ut_thread(); } @@ -146,6 +156,20 @@ static void ubi_bdev_read(void *arg) { } } +static void ubi_bdev_flush(void *arg) { + struct ubi_io_request *req = arg; + + // Reset success. This will be set in the completion callback. + req->success = false; + + int rc = spdk_bdev_flush_blocks(req->state->bdev_desc, req->state->ch, req->block_idx, + 1, io_completion_cb, req); + + if (rc) { + wake_ut_thread(); + } +} + static void io_completion_cb(struct spdk_bdev_io *bdev_io, bool success, void *arg) { struct ubi_io_request *req = arg; req->success = success; @@ -162,28 +186,50 @@ static void run_ut_thread(void *arg) { memset(&g_state, 0, sizeof(g_state)); struct test_state *state = &g_state; - execute_spdk_function(open_bdev, state); - if (state->bdev_desc == NULL || state->ch == NULL) { - state->n_failures++; - goto done; - } - state->image_file = fopen(IMAGE_PATH, "r"); if (state->image_file == NULL) { rc = errno; SPDK_ERRLOG("Could not open %s: %s\n", IMAGE_PATH, strerror(-rc)); state->n_failures++; - goto done; + goto cleanup; } - uint64_t image_size = 0; - if (!file_size(state->image_file, &image_size)) { + if (!file_size(state->image_file, &state->image_size)) { state->n_failures++; - goto done; + goto cleanup; + } + + for (size_t i = 0; i < g_opts.n_bdevs; i++) { + SPDK_NOTICELOG("Testing %s\n", g_opts.bdev_names[i]); + run_bdev_tests(state, g_opts.bdev_names[i]); } - state->n_image_blocks = image_size / state->blocklen; - SPDK_NOTICELOG("Image block count: %lu.\n", state->n_image_blocks); +cleanup: + if (state->image_file) + fclose(state->image_file); + + spdk_thread_send_msg(g_init_thread, stop_init_thread, state); + spdk_thread_exit(g_ut_thread); +} + +static void run_bdev_tests(struct test_state *state, const char *name) { + int rc = spdk_bdev_open_ext(name, true, ubi_event_cb, NULL, &state->bdev_desc); + if (rc < 0) { + SPDK_ERRLOG("Could not open bdev %s: %s\n", name, strerror(-rc)); + return; + } + + struct spdk_bdev *bdev = spdk_bdev_desc_get_bdev(state->bdev_desc); + state->blocklen = bdev->blocklen; + state->blockcnt = bdev->blockcnt; + state->n_image_blocks = state->image_size / state->blocklen; + + execute_spdk_function(open_io_channel, state); + if (state->bdev_desc == NULL || state->ch == NULL) { + state->n_failures++; + spdk_bdev_close(state->bdev_desc); + return; + } #define RUN_TEST(x) \ if (!(x)) { \ @@ -198,15 +244,11 @@ static void run_ut_thread(void *arg) { RUN_TEST(test_write(state, 20, 100)); // write 100 blocks to the non-image addresses RUN_TEST(test_write(state, state->n_image_blocks + 2, 100)); + // Some random io + RUN_TEST(test_random_ops(state, 50)); - execute_spdk_function(close_bdev, state); - -done: - if (state->image_file) - fclose(state->image_file); - - spdk_thread_send_msg(g_init_thread, stop_init_thread, state); - spdk_thread_exit(g_ut_thread); + execute_spdk_function(close_io_channel, state); + spdk_bdev_close(state->bdev_desc); } static bool test_read(struct test_state *state, uint32_t start, uint32_t count) { @@ -265,6 +307,30 @@ static bool test_write(struct test_state *state, uint32_t start, uint32_t count) return true; } +static bool test_random_ops(struct test_state *state, uint32_t count) { + struct ubi_io_request req; + req.state = state; + for (size_t i = 0; i < count; i++) { + int type = rand() % 3; + req.block_idx = rand() % state->blockcnt; + switch (type) { + case 0: + execute_spdk_function(ubi_bdev_read, &req); + break; + case 1: + execute_spdk_function(ubi_bdev_write, &req); + break; + case 2: + execute_spdk_function(ubi_bdev_flush, &req); + break; + } + if (!req.success) + return false; + } + + return true; +} + static bool verify_image_block(struct test_state *state, uint64_t block, char *buf) { char image_buf[MAX_BLOCK_SIZE]; @@ -343,6 +409,23 @@ static void stop_init_thread(void *arg) { spdk_app_stop(state->n_failures); } +static void usage(void) { printf(" -bdev Block device to be used for testing.\n"); } + +static int parse_arg(int argc, char *argv) { + switch (argc) { + case MEMCHECK_OPTION_BDEV: + if (g_opts.n_bdevs >= MAX_BDEVS) { + fprintf(stderr, "Too many bdevs.\n"); + exit(-1); + } + g_opts.bdev_names[g_opts.n_bdevs++] = strdup(argv); + break; + default: + return -EINVAL; + } + return 0; +} + int main(int argc, char **argv) { int rc; struct spdk_app_opts opts = {}; @@ -350,16 +433,24 @@ int main(int argc, char **argv) { opts.name = "test_ubi"; opts.reactor_mask = "0x1"; - rc = spdk_app_parse_args(argc, argv, &opts, NULL, NULL, NULL, NULL); + rc = spdk_app_parse_args(argc, argv, &opts, NULL, g_cmdline_opts, parse_arg, usage); if (rc != SPDK_APP_PARSE_ARGS_SUCCESS) { exit(rc); } + if (g_opts.n_bdevs == 0) { + g_opts.n_bdevs = 1; + g_opts.bdev_names[0] = strdup(DEFAULT_BDEV_NAME); + } + rc = spdk_app_start(&opts, test_ubi_run, NULL); if (rc) { SPDK_ERRLOG("Error occured while testing bdev_ubi.\n"); } + for (int i = 0; i < g_opts.n_bdevs; i++) + free(g_opts.bdev_names[i]); + spdk_app_fini(); return rc;