From patchwork Fri Mar 30 03:36:59 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Benjamin Marzinski X-Patchwork-Id: 10316815 X-Patchwork-Delegate: christophe.varoqui@free.fr Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id C550860467 for ; Fri, 30 Mar 2018 03:37:16 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B4FAA2A347 for ; Fri, 30 Mar 2018 03:37:16 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A9A0D2A39C; Fri, 30 Mar 2018 03:37:16 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id A395D2A347 for ; Fri, 30 Mar 2018 03:37:15 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id BF7CEC036757; Fri, 30 Mar 2018 03:37:14 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 8F5BA611AB; Fri, 30 Mar 2018 03:37:14 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id EE8784CA9C; Fri, 30 Mar 2018 03:37:13 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id w2U3bC7a022626 for ; Thu, 29 Mar 2018 23:37:12 -0400 Received: by smtp.corp.redhat.com (Postfix) id BD9E842226; Fri, 30 Mar 2018 03:37:12 +0000 (UTC) Delivered-To: dm-devel@redhat.com Received: from redhat.com (octiron.msp.redhat.com [10.15.80.209]) by smtp.corp.redhat.com (Postfix) with SMTP id 858E9D7DF3; Fri, 30 Mar 2018 03:37:09 +0000 (UTC) Received: by redhat.com (sSMTP sendmail emulation); Thu, 29 Mar 2018 22:37:08 -0500 From: "Benjamin Marzinski" To: device-mapper development Date: Thu, 29 Mar 2018 22:36:59 -0500 Message-Id: <1522381023-15262-3-git-send-email-bmarzins@redhat.com> In-Reply-To: <1522381023-15262-1-git-send-email-bmarzins@redhat.com> References: <1522381023-15262-1-git-send-email-bmarzins@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-loop: dm-devel@redhat.com Cc: Martin Wilck Subject: [dm-devel] [PATCH 2/6] multipath: add comments X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: device-mapper development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Fri, 30 Mar 2018 03:37:15 +0000 (UTC) X-Virus-Scanned: ClamAV using ClamSMTP This commit simply adds a number of comments based on suggestions by Martin Wilck. Cc: Martin Wilck Signed-off-by: Benjamin Marzinski Reviewed-by: Martin Wilck --- 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];