From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Benjamin Marzinski Date: Thu, 29 Mar 2018 12:07:11 -0500 Subject: [PATCH] multipath: add comments This commit simply adds a number of comments based on suggestions by Martin Wilck. Cc: Martin Wilck Signed-off-by: Benjamin Marzinski --- multipathd/dmevents.c | 4 ++- tests/dmevents.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++- tests/util.c | 2 ++ 3 files changed, 76 insertions(+), 2 deletions(-) diff --git a/multipathd/dmevents.c b/multipathd/dmevents.c index 2281a10..0b0d0ce 100644 --- a/multipathd/dmevents.c +++ b/multipathd/dmevents.c @@ -122,6 +122,8 @@ static int arm_dm_event_poll(int fd) dmi.version[0] = DM_VERSION_MAJOR; dmi.version[1] = DM_VERSION_MINOR; dmi.version[2] = DM_VERSION_PATCHLEVEL; + /* This flag currently does nothing. It simply exists to + * duplicate the behavior of libdevmapper */ dmi.flags = 0x4; dmi.data_start = offsetof(struct dm_ioctl, data); dmi.data_size = sizeof(dmi); @@ -189,7 +191,7 @@ fail: return -1; } -/* You must call update_multipath() after calling this function, to +/* You must call __setup_multipath() after calling this function, to * deal with any events that came in before the device was added */ int watch_dmevents(char *name) { diff --git a/tests/dmevents.c b/tests/dmevents.c index 4442fc2..bba51dc 100644 --- a/tests/dmevents.c +++ b/tests/dmevents.c @@ -33,10 +33,13 @@ /* I have to do this to get at the static variables */ #include "../multipathd/dmevents.c" +/* pretend dm device */ struct dm_device { char name[WWID_SIZE]; + /* is this a mpath device, or other dm device */ int is_mpath; uint32_t evt_nr; + /* tracks the event number when the multipath device was updated */ uint32_t update_nr; }; @@ -48,6 +51,9 @@ struct test_data { struct test_data data; +/* Add a pretend dm device, or update its event number. This is used to build + * up the dm devices that the dmevents code queries with dm_task_get_names, + * dm_geteventnr, and dm_is_mpath */ int add_dm_device_event(char *name, int is_mpath, uint32_t evt_nr) { struct dm_device *dev; @@ -77,6 +83,7 @@ int add_dm_device_event(char *name, int is_mpath, uint32_t evt_nr) return 0; } +/* helper function for pretend dm devices */ struct dm_device *find_dm_device(const char *name) { struct dm_device *dev; @@ -88,6 +95,7 @@ struct dm_device *find_dm_device(const char *name) return NULL; } +/* helper function for pretend dm devices */ int remove_dm_device_event(const char *name) { struct dm_device *dev; @@ -103,6 +111,7 @@ int remove_dm_device_event(const char *name) return -1; } +/* helper function for pretend dm devices */ void remove_all_dm_device_events(void) { struct dm_device *dev; @@ -122,7 +131,9 @@ static inline void *align_ptr(void *ptr) return (void *)align_val((size_t)ptr); } -/* copied off of list_devices in dm-ioctl.c */ +/* copied off of list_devices in dm-ioctl.c except that it uses + * the pretend dm devices, and saves the output to the test_data + * structure */ struct dm_names *build_dm_names(void) { struct dm_names *names, *np, *old_np = NULL; @@ -199,12 +210,16 @@ int __wrap_open(const char *pathname, int flags) return mock_type(int); } +/* We never check the result of the close(), so there's no need to + * to mock a return value */ int __wrap_close(int fd) { assert_int_equal(fd, waiter->fd); return 0; } +/* the pretend dm device code checks the input and supplies the + * return value, so there's no need to do that here */ int __wrap_dm_is_mpath(const char *name) { struct dm_device *dev; @@ -216,6 +231,8 @@ int __wrap_dm_is_mpath(const char *name) return 0; } +/* either get return info from the pretend dm device, or + * override it to test -1 return */ int __wrap_dm_geteventnr(const char *name) { struct dm_device *dev; @@ -257,6 +274,8 @@ int __wrap_dm_task_run(struct dm_task *dmt) return mock_type(int); } +/* either get return info from the pretend dm device, or + * override it to test NULL return */ struct dm_names * __wrap_dm_task_get_names(struct dm_task *dmt) { int good = mock_type(int); @@ -299,6 +318,9 @@ void __wrap_remove_map_by_alias(const char *alias, struct vectors * vecs, assert_int_equal(purge_vec, 1); } +/* pretend update the pretend dm devices. If fail is set, it + * simulates having the dm device removed. Otherwise it just sets + * update_nr to record when the update happened */ int __wrap_update_multipath(struct vectors *vecs, char *mapname, int reset) { int fail; @@ -325,6 +347,9 @@ int __wrap_update_multipath(struct vectors *vecs, char *mapname, int reset) return fail; } +/* helper function used to check if the dmevents list of devices + * includes a specific device. To make sure that dmevents is + * in the correct state after running a function */ struct dev_event *find_dmevents(const char *name) { struct dev_event *dev_evt; @@ -336,14 +361,19 @@ struct dev_event *find_dmevents(const char *name) return NULL; } +/* null vecs pointer when initialized dmevents */ static void test_init_waiter_bad0(void **state) { + /* this boilerplate code just skips the test if + * dmevents polling is not supported */ struct test_data *datap = (struct test_data *)(*state); if (datap == NULL) skip(); + assert_int_equal(init_dmevent_waiter(NULL), -1); } +/* fail to open /dev/mapper/control */ static void test_init_waiter_bad1(void **state) { struct test_data *datap = (struct test_data *)(*state); @@ -354,6 +384,7 @@ static void test_init_waiter_bad1(void **state) assert_ptr_equal(waiter, NULL); } +/* waiter remains initialized after this test */ static void test_init_waiter_good0(void **state) { struct test_data *datap = (struct test_data *)(*state); @@ -364,6 +395,7 @@ static void test_init_waiter_good0(void **state) assert_ptr_not_equal(waiter, NULL); } +/* No dm device named foo */ static void test_watch_dmevents_bad0(void **state) { struct test_data *datap = (struct test_data *)(*state); @@ -373,6 +405,7 @@ static void test_watch_dmevents_bad0(void **state) assert_ptr_equal(find_dmevents("foo"), NULL); } +/* foo is not a multipath device */ static void test_watch_dmevents_bad1(void **state) { struct test_data *datap = (struct test_data *)(*state); @@ -384,6 +417,7 @@ static void test_watch_dmevents_bad1(void **state) assert_ptr_equal(find_dmevents("foo"), NULL); } +/* failed getting the dmevent number */ static void test_watch_dmevents_bad2(void **state) { struct test_data *datap = (struct test_data *)(*state); @@ -396,6 +430,8 @@ static void test_watch_dmevents_bad2(void **state) assert_int_equal(watch_dmevents("foo"), -1); assert_ptr_equal(find_dmevents("foo"), NULL); } + +/* verify that you can watch and unwatch dm multipath device "foo" */ static void test_watch_dmevents_good0(void **state) { struct dev_event *dev_evt; @@ -407,16 +443,20 @@ static void test_watch_dmevents_good0(void **state) assert_int_equal(add_dm_device_event("foo", 1, 5), 0); will_return(__wrap_dm_geteventnr, 0); assert_int_equal(watch_dmevents("foo"), 0); + /* verify foo is being watched */ dev_evt = find_dmevents("foo"); assert_ptr_not_equal(dev_evt, NULL); assert_int_equal(dev_evt->evt_nr, 5); assert_int_equal(dev_evt->action, EVENT_NOTHING); assert_int_equal(VECTOR_SIZE(waiter->events), 1); unwatch_dmevents("foo"); + /* verify foo is no longer being watched */ assert_int_equal(VECTOR_SIZE(waiter->events), 0); assert_ptr_equal(find_dmevents("foo"), NULL); } +/* verify that if you try to watch foo multiple times, it only + * is placed on the waiter list once */ static void test_watch_dmevents_good1(void **state) { struct dev_event *dev_evt; @@ -445,6 +485,7 @@ static void test_watch_dmevents_good1(void **state) assert_ptr_equal(find_dmevents("foo"), NULL); } +/* watch and then unwatch multiple devices */ static void test_watch_dmevents_good2(void **state) { struct dev_event *dev_evt; @@ -480,6 +521,7 @@ static void test_watch_dmevents_good2(void **state) assert_ptr_equal(find_dmevents("bar"), NULL); } +/* dm_task_create fails */ static void test_get_events_bad0(void **state) { struct test_data *datap = (struct test_data *)(*state); @@ -493,6 +535,7 @@ static void test_get_events_bad0(void **state) assert_int_equal(dm_get_events(), -1); } +/* dm_task_run fails */ static void test_get_events_bad1(void **state) { struct test_data *datap = (struct test_data *)(*state); @@ -505,6 +548,7 @@ static void test_get_events_bad1(void **state) assert_int_equal(dm_get_events(), -1); } +/* dm_task_get_names fails */ static void test_get_events_bad2(void **state) { struct test_data *datap = (struct test_data *)(*state); @@ -518,6 +562,7 @@ static void test_get_events_bad2(void **state) assert_int_equal(dm_get_events(), -1); } +/* If the device isn't being watched, dm_get_events returns NULL */ static void test_get_events_good0(void **state) { struct test_data *datap = (struct test_data *)(*state); @@ -534,6 +579,11 @@ static void test_get_events_good0(void **state) assert_int_equal(VECTOR_SIZE(waiter->events), 0); } +/* There are 5 dm devices. 4 of them are multipath devices. + * Only 3 of them are being watched. "foo" has a new event + * "xyzzy" gets removed. Nothing happens to bar. Verify + * that all the events are properly set, and that nothing + * happens with the two devices that aren't being watched */ static void test_get_events_good1(void **state) { struct dev_event *dev_evt; @@ -577,6 +627,8 @@ static void test_get_events_good1(void **state) assert_int_equal(VECTOR_SIZE(waiter->events), 3); } +/* poll does not return an event. nothing happens. The + * devices remain after this test */ static void test_dmevent_loop_bad0(void **state) { struct dm_device *dev; @@ -603,6 +655,7 @@ static void test_dmevent_loop_bad0(void **state) assert_int_equal(dev->update_nr, 5); } +/* arm_dm_event_poll's ioctl fails. Nothing happens */ static void test_dmevent_loop_bad1(void **state) { struct dm_device *dev; @@ -624,6 +677,7 @@ static void test_dmevent_loop_bad1(void **state) assert_int_equal(dev->update_nr, 5); } +/* dm_get_events fails. Nothing happens */ static void test_dmevent_loop_bad2(void **state) { struct dm_device *dev; @@ -646,6 +700,8 @@ static void test_dmevent_loop_bad2(void **state) assert_int_equal(dev->update_nr, 5); } +/* verify dmevent_loop runs successfully when no devices are being + * watched */ static void test_dmevent_loop_good0(void **state) { struct test_data *datap = (struct test_data *)(*state); @@ -663,6 +719,11 @@ static void test_dmevent_loop_good0(void **state) assert_int_equal(dmevent_loop(), 1); } +/* Watch 3 devices, where one device has an event (foo), one device is + * removed (xyzzy), and one device does nothing (bar). Verify that + * the device with the event gets updated, the device that is removed + * gets unwatched, and the device with no events stays the same. + * The devices remain after this test */ static void test_dmevent_loop_good1(void **state) { struct dm_device *dev; @@ -717,6 +778,10 @@ static void test_dmevent_loop_good1(void **state) assert_ptr_equal(find_dm_device("xyzzy"), NULL); } +/* watch another dm device and add events to two of them, so bar and + * baz have new events, and foo doesn't. Set update_multipath to + * fail for baz. Verify that baz is unwatched, bar is updated, and + * foo stays the same. */ static void test_dmevent_loop_good2(void **state) { struct dm_device *dev; @@ -762,6 +827,8 @@ static void test_dmevent_loop_good2(void **state) assert_ptr_equal(find_dm_device("baz"), NULL); } +/* remove dm device foo, and unwatch events on bar. Verify that + * foo is cleaned up and unwatched, and bar is no longer updated */ static void test_dmevent_loop_good3(void **state) { struct dm_device *dev; @@ -790,6 +857,8 @@ static void test_dmevent_loop_good3(void **state) assert_ptr_equal(find_dm_device("foo"), NULL); } + +/* verify that rearming the dmevents polling works */ static void test_arm_poll(void **state) { struct test_data *datap = (struct test_data *)(*state); @@ -799,6 +868,7 @@ static void test_arm_poll(void **state) assert_int_equal(arm_dm_event_poll(waiter->fd), 0); } +/* verify that the waiter is cleaned up */ static void test_cleanup_waiter(void **state) { struct test_data *datap = (struct test_data *)(*state); diff --git a/tests/util.c b/tests/util.c index e9a004f..113b134 100644 --- a/tests/util.c +++ b/tests/util.c @@ -74,6 +74,8 @@ static void test_basenamecpy_good5(void **state) assert_string_equal(dst, "bar"); } +/* multipath expects any trailing whitespace to be stripped off the basename, + * so that it will match pp->dev */ static void test_basenamecpy_good6(void **state) { char dst[6]; -- 2.7.4