From 2ae4a04710bb2d66a4b7c2d10e56750858f08f04 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Thu, 10 May 2018 13:01:26 +0100 Subject: [PATCH] vdo status: Unit tests + fix bugs --- device-mapper/vdo/status.c | 28 ++-- device-mapper/vdo/target.h | 7 +- test/unit/Makefile.in | 5 +- test/unit/dmstatus_t.c | 2 +- test/unit/units.h | 2 + test/unit/vdo_t.c | 325 +++++++++++++++++++++++++++++++++++++ 6 files changed, 353 insertions(+), 16 deletions(-) create mode 100644 test/unit/vdo_t.c diff --git a/device-mapper/vdo/status.c b/device-mapper/vdo/status.c index 895ebc429..157111fde 100644 --- a/device-mapper/vdo/status.c +++ b/device-mapper/vdo/status.c @@ -14,9 +14,11 @@ static char *_tok_cpy(const char *b, const char *e) char *new = malloc((e - b) + 1); char *ptr = new; - if (new) + if (new) { while (b != e) *ptr++ = *b++; + *ptr = '\0'; + } return new; } @@ -131,7 +133,7 @@ static bool _parse_uint64(const char *b, const char *e, void *context) if (!isdigit(*b)) return false; - n = (n << 1) + (*b - '0'); + n = (n * 10) + (*b - '0'); b++; } @@ -149,16 +151,17 @@ static const char *_eat_space(const char *b, const char *e) static const char *_next_tok(const char *b, const char *e) { - while (b != e && !isspace(*b)) - b++; + const char *te = b; + while (te != e && !isspace(*te)) + te++; - return b == e ? NULL : b; + return te == b ? NULL : te; } -static void _set_error(struct parse_result *result, const char *fmt, ...) +static void _set_error(struct vdo_status_parse_result *result, const char *fmt, ...) __attribute__ ((format(printf, 2, 3))); -static void _set_error(struct parse_result *result, const char *fmt, ...) +static void _set_error(struct vdo_status_parse_result *result, const char *fmt, ...) { va_list ap; @@ -170,7 +173,7 @@ static void _set_error(struct parse_result *result, const char *fmt, ...) static bool _parse_field(const char **b, const char *e, bool (*p_fn)(const char *, const char *, void *), void *field, const char *field_name, - struct parse_result *result) + struct vdo_status_parse_result *result) { const char *te; @@ -190,7 +193,7 @@ static bool _parse_field(const char **b, const char *e, } -bool parse_vdo_status(const char *input, struct parse_result *result) +bool vdo_status_parse(const char *input, struct vdo_status_parse_result *result) { const char *b = b = input; const char *e = input + strlen(input); @@ -228,7 +231,12 @@ bool parse_vdo_status(const char *input, struct parse_result *result) XX(_parse_uint64, &s->total_blocks, "total blocks"); #undef XX - result->result = s; + if (b != e) { + _set_error(result, "too many tokens"); + goto bad; + } + + result->status = s; return true; bad: diff --git a/device-mapper/vdo/target.h b/device-mapper/vdo/target.h index 118ec93f6..3137e2c07 100644 --- a/device-mapper/vdo/target.h +++ b/device-mapper/vdo/target.h @@ -55,14 +55,13 @@ void vdo_status_destroy(struct vdo_status *s); #define VDO_MAX_ERROR 256 -struct parse_result { +struct vdo_status_parse_result { char error[VDO_MAX_ERROR]; - struct vdo_status *result; + struct vdo_status *status; }; // Parses the status line from the kernel target. -bool parse_vdo_status(const char *input, struct parse_result *result); - +bool vdo_status_parse(const char *input, struct vdo_status_parse_result *result); //---------------------------------------------------------------- diff --git a/test/unit/Makefile.in b/test/unit/Makefile.in index 10ddbe2c5..0e1015ca4 100644 --- a/test/unit/Makefile.in +++ b/test/unit/Makefile.in @@ -11,6 +11,8 @@ # Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA UNIT_SOURCE=\ + device-mapper/vdo/status.c \ + \ test/unit/bcache_t.c \ test/unit/bcache_utils_t.c \ test/unit/bitset_t.c \ @@ -22,7 +24,8 @@ UNIT_SOURCE=\ test/unit/framework.c \ test/unit/percent_t.c \ test/unit/run.c \ - test/unit/string_t.c + test/unit/string_t.c \ + test/unit/vdo_t.c UNIT_DEPENDS=$(subst .c,.d,$(UNIT_SOURCE)) UNIT_OBJECTS=$(UNIT_SOURCE:%.c=%.o) diff --git a/test/unit/dmstatus_t.c b/test/unit/dmstatus_t.c index 4b57f29e4..ae185ca28 100644 --- a/test/unit/dmstatus_t.c +++ b/test/unit/dmstatus_t.c @@ -78,7 +78,7 @@ void dm_status_tests(struct dm_list *all_tests) exit(1); } - register_test(ts, "/dm/target/mirror/status", "parsing mirror status", _test_mirror_status); + register_test(ts, "/device-mapper/mirror/status", "parsing mirror status", _test_mirror_status); dm_list_add(all_tests, &ts->list); } diff --git a/test/unit/units.h b/test/unit/units.h index c3c0c9685..ec0cbbf96 100644 --- a/test/unit/units.h +++ b/test/unit/units.h @@ -30,6 +30,7 @@ void io_engine_tests(struct dm_list *suites); void regex_tests(struct dm_list *suites); void percent_tests(struct dm_list *suites); void string_tests(struct dm_list *suites); +void vdo_tests(struct dm_list *suites); // ... and call it in here. static inline void register_all_tests(struct dm_list *suites) @@ -44,6 +45,7 @@ static inline void register_all_tests(struct dm_list *suites) regex_tests(suites); percent_tests(suites); string_tests(suites); + vdo_tests(suites); } //----------------------------------------------------------------- diff --git a/test/unit/vdo_t.c b/test/unit/vdo_t.c new file mode 100644 index 000000000..aaa4d9cc5 --- /dev/null +++ b/test/unit/vdo_t.c @@ -0,0 +1,325 @@ +/* + * Copyright (C) 2018 Red Hat, Inc. All rights reserved. + * + * This file is part of LVM2. + * + * This copyrighted material is made available to anyone wishing to use, + * modify, copy, or redistribute it subject to the terms and conditions + * of the GNU General Public License v.2. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include "device-mapper/vdo/target.h" +#include "framework.h" +#include "units.h" + +//---------------------------------------------------------------- + +static bool _status_eq(struct vdo_status *lhs, struct vdo_status *rhs) +{ + return !strcmp(lhs->device, rhs->device) && + (lhs->operating_mode == rhs->operating_mode) && + (lhs->recovering == rhs->recovering) && + (lhs->index_state == rhs->index_state) && + (lhs->compression_state == rhs->compression_state) && + (lhs->used_blocks == rhs->used_blocks) && + (lhs->total_blocks == rhs->total_blocks); +} + +#if 0 +static const char *_op_mode(enum vdo_operating_mode m) +{ + switch (m) { + case VDO_MODE_RECOVERING: + return "recovering"; + case VDO_MODE_READ_ONLY: + return "read-only"; + case VDO_MODE_NORMAL: + return "normal"; + } + + return ""; +} + +static const char *_index_state(enum vdo_index_state is) +{ + switch (is) { + case VDO_INDEX_ERROR: + return "error"; + case VDO_INDEX_CLOSED: + return "closed"; + case VDO_INDEX_OPENING: + return "opening"; + case VDO_INDEX_CLOSING: + return "closing"; + case VDO_INDEX_OFFLINE: + return "offline"; + case VDO_INDEX_ONLINE: + return "online"; + case VDO_INDEX_UNKNOWN: + return "unknown"; + } + + return ""; +} + +static void _print_status(FILE *stream, struct vdo_status *s) +{ + fprintf(stream, "device); + fprintf(stream, "%s ", _op_mode(s->operating_mode)); + fprintf(stream, "%s ", s->recovering ? "recovering" : "-"); + fprintf(stream, "%s ", _index_state(s->index_state)); + fprintf(stream, "%s ", s->compression_state == VDO_COMPRESSION_ONLINE ? "online" : "offline"); + fprintf(stream, "%llu ", (unsigned long long) s->used_blocks); + fprintf(stream, "%llu", (unsigned long long) s->total_blocks); + fprintf(stream, ">"); +} +#endif + +struct example_good { + const char *input; + struct vdo_status status; +}; + +static void _check_good(struct example_good *es, unsigned count) +{ + unsigned i; + + for (i = 0; i < count; i++) { + struct example_good *e = es + i; + struct vdo_status_parse_result pr; + + T_ASSERT(vdo_status_parse(e->input, &pr)); +#if 0 + _print_status(stderr, pr.status); + fprintf(stderr, "\n"); + _print_status(stderr, &e->status); + fprintf(stderr, "\n"); +#endif + T_ASSERT(_status_eq(&e->status, pr.status)); + } +} + +struct example_bad { + const char *input; + const char *reason; +}; + +static void _check_bad(struct example_bad *es, unsigned count) +{ + unsigned i; + + for (i = 0; i < count; i++) { + struct example_bad *e = es + i; + struct vdo_status_parse_result pr; + + T_ASSERT(!vdo_status_parse(e->input, &pr)); + T_ASSERT(!strcmp(e->reason, pr.error)); + } +} + +static void _test_device_names_good(void *fixture) +{ + static struct example_good _es[] = { + {"foo1234 read-only - error online 0 1234", + {(char *) "foo1234", VDO_MODE_READ_ONLY, false, VDO_INDEX_ERROR, VDO_COMPRESSION_ONLINE, 0, 1234}}, + {"f read-only - error online 0 1234", + {(char *) "f", VDO_MODE_READ_ONLY, false, VDO_INDEX_ERROR, VDO_COMPRESSION_ONLINE, 0, 1234}}, + }; + + _check_good(_es, DM_ARRAY_SIZE(_es)); +} + +static void _test_operating_mode_good(void *fixture) +{ + static struct example_good _es[] = { + {"device-name recovering - error online 0 1234", + {(char *) "device-name", VDO_MODE_RECOVERING, false, VDO_INDEX_ERROR, VDO_COMPRESSION_ONLINE, 0, 1234}}, + {"device-name read-only - error online 0 1234", + {(char *) "device-name", VDO_MODE_READ_ONLY, false, VDO_INDEX_ERROR, VDO_COMPRESSION_ONLINE, 0, 1234}}, + {"device-name normal - error online 0 1234", + {(char *) "device-name", VDO_MODE_NORMAL, false, VDO_INDEX_ERROR, VDO_COMPRESSION_ONLINE, 0, 1234}}, + }; + + _check_good(_es, DM_ARRAY_SIZE(_es)); +} + +static void _test_operating_mode_bad(void *fixture) +{ + static struct example_bad _es[] = { + {"device-name investigating - error online 0 1234", + "couldn't parse 'operating mode'"}}; + + _check_bad(_es, DM_ARRAY_SIZE(_es)); +} + +static void _test_recovering_good(void *fixture) +{ + static struct example_good _es[] = { + {"device-name recovering - error online 0 1234", + {(char *) "device-name", VDO_MODE_RECOVERING, false, VDO_INDEX_ERROR, VDO_COMPRESSION_ONLINE, 0, 1234}}, + {"device-name read-only recovering error online 0 1234", + {(char *) "device-name", VDO_MODE_READ_ONLY, true, VDO_INDEX_ERROR, VDO_COMPRESSION_ONLINE, 0, 1234}}, + }; + + _check_good(_es, DM_ARRAY_SIZE(_es)); +} + +static void _test_recovering_bad(void *fixture) +{ + static struct example_bad _es[] = { + {"device-name normal fish error online 0 1234", + "couldn't parse 'recovering'"}}; + + _check_bad(_es, DM_ARRAY_SIZE(_es)); +} + +static void _test_index_state_good(void *fixture) +{ + static struct example_good _es[] = { + {"device-name recovering - error online 0 1234", + {(char *) "device-name", VDO_MODE_RECOVERING, false, VDO_INDEX_ERROR, VDO_COMPRESSION_ONLINE, 0, 1234}}, + {"device-name recovering - closed online 0 1234", + {(char *) "device-name", VDO_MODE_RECOVERING, false, VDO_INDEX_CLOSED, VDO_COMPRESSION_ONLINE, 0, 1234}}, + {"device-name recovering - opening online 0 1234", + {(char *) "device-name", VDO_MODE_RECOVERING, false, VDO_INDEX_OPENING, VDO_COMPRESSION_ONLINE, 0, 1234}}, + {"device-name recovering - closing online 0 1234", + {(char *) "device-name", VDO_MODE_RECOVERING, false, VDO_INDEX_CLOSING, VDO_COMPRESSION_ONLINE, 0, 1234}}, + {"device-name recovering - offline online 0 1234", + {(char *) "device-name", VDO_MODE_RECOVERING, false, VDO_INDEX_OFFLINE, VDO_COMPRESSION_ONLINE, 0, 1234}}, + {"device-name recovering - online online 0 1234", + {(char *) "device-name", VDO_MODE_RECOVERING, false, VDO_INDEX_ONLINE, VDO_COMPRESSION_ONLINE, 0, 1234}}, + {"device-name recovering - unknown online 0 1234", + {(char *) "device-name", VDO_MODE_RECOVERING, false, VDO_INDEX_UNKNOWN, VDO_COMPRESSION_ONLINE, 0, 1234}}, + }; + + _check_good(_es, DM_ARRAY_SIZE(_es)); +} + +static void _test_index_state_bad(void *fixture) +{ + static struct example_bad _es[] = { + {"device-name normal - fish online 0 1234", + "couldn't parse 'index state'"}}; + + _check_bad(_es, DM_ARRAY_SIZE(_es)); +} + +static void _test_compression_state_good(void *fixture) +{ + static struct example_good _es[] = { + {"device-name recovering - error online 0 1234", + {(char *) "device-name", VDO_MODE_RECOVERING, false, VDO_INDEX_ERROR, VDO_COMPRESSION_ONLINE, 0, 1234}}, + {"device-name read-only - error offline 0 1234", + {(char *) "device-name", VDO_MODE_READ_ONLY, false, VDO_INDEX_ERROR, VDO_COMPRESSION_OFFLINE, 0, 1234}}, + }; + + _check_good(_es, DM_ARRAY_SIZE(_es)); +} + +static void _test_compression_state_bad(void *fixture) +{ + static struct example_bad _es[] = { + {"device-name normal - error fish 0 1234", + "couldn't parse 'compression state'"}}; + + _check_bad(_es, DM_ARRAY_SIZE(_es)); +} + +static void _test_used_blocks_good(void *fixture) +{ + static struct example_good _es[] = { + {"device-name recovering - error online 0 1234", + {(char *) "device-name", VDO_MODE_RECOVERING, false, VDO_INDEX_ERROR, VDO_COMPRESSION_ONLINE, 0, 1234}}, + {"device-name read-only - error offline 1 1234", + {(char *) "device-name", VDO_MODE_READ_ONLY, false, VDO_INDEX_ERROR, VDO_COMPRESSION_OFFLINE, 1, 1234}}, + {"device-name read-only - error offline 12 1234", + {(char *) "device-name", VDO_MODE_READ_ONLY, false, VDO_INDEX_ERROR, VDO_COMPRESSION_OFFLINE, 12, 1234}}, + {"device-name read-only - error offline 3456 1234", + {(char *) "device-name", VDO_MODE_READ_ONLY, false, VDO_INDEX_ERROR, VDO_COMPRESSION_OFFLINE, 3456, 1234}}, + }; + + _check_good(_es, DM_ARRAY_SIZE(_es)); +} + +static void _test_used_blocks_bad(void *fixture) +{ + static struct example_bad _es[] = { + {"device-name normal - error online fish 1234", + "couldn't parse 'used blocks'"}}; + + _check_bad(_es, DM_ARRAY_SIZE(_es)); +} + +static void _test_total_blocks_good(void *fixture) +{ + static struct example_good _es[] = { + {"device-name recovering - error online 0 1234", + {(char *) "device-name", VDO_MODE_RECOVERING, false, VDO_INDEX_ERROR, VDO_COMPRESSION_ONLINE, 0, 1234}}, + {"device-name recovering - error online 0 1", + {(char *) "device-name", VDO_MODE_RECOVERING, false, VDO_INDEX_ERROR, VDO_COMPRESSION_ONLINE, 0, 1}}, + {"device-name recovering - error online 0 0", + {(char *) "device-name", VDO_MODE_RECOVERING, false, VDO_INDEX_ERROR, VDO_COMPRESSION_ONLINE, 0, 0}}, + }; + + _check_good(_es, DM_ARRAY_SIZE(_es)); +} + +static void _test_total_blocks_bad(void *fixture) +{ + static struct example_bad _es[] = { + {"device-name normal - error online 0 fish", + "couldn't parse 'total blocks'"}}; + + _check_bad(_es, DM_ARRAY_SIZE(_es)); +} + +static void _test_status_bad(void *fixture) +{ + struct example_bad _bad[] = { + {"", "couldn't get token for device"}, + {"device-name read-only - error online 0 1000 lksd", "too many tokens"} + }; + + _check_bad(_bad, DM_ARRAY_SIZE(_bad)); +} + +//---------------------------------------------------------------- + +#define T(path, desc, fn) register_test(ts, "/device-mapper/vdo/status/" path, desc, fn) + +static struct test_suite *_tests(void) +{ + struct test_suite *ts = test_suite_create(NULL, NULL); + if (!ts) { + fprintf(stderr, "out of memory\n"); + exit(1); + }; + + T("device-names", "parse various device names", _test_device_names_good); + T("operating-mode-good", "operating mode, good examples", _test_operating_mode_good); + T("operating-mode-bad", "operating mode, bad examples", _test_operating_mode_bad); + T("recovering-good", "recovering, good examples", _test_recovering_good); + T("recovering-bad", "recovering, bad examples", _test_recovering_bad); + T("index-state-good", "index state, good examples", _test_index_state_good); + T("index-state-bad", "index state, bad examples", _test_index_state_bad); + T("compression-state-good", "compression state, good examples", _test_compression_state_good); + T("compression-state-bad", "compression state, bad examples", _test_compression_state_bad); + T("used-blocks-good", "used blocks, good examples", _test_used_blocks_good); + T("used-blocks-bad", "used blocks, bad examples", _test_used_blocks_bad); + T("total-blocks-good", "total blocks, good examples", _test_total_blocks_good); + T("total-blocks-bad", "total blocks, bad examples", _test_total_blocks_bad); + T("bad", "parse various badly formed vdo status lines", _test_status_bad); + + return ts; +} + +void vdo_tests(struct dm_list *all_tests) +{ + dm_list_add(all_tests, &_tests()->list); +} + +//----------------------------------------------------------------