From patchwork Thu Sep 14 14:51:29 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Wilck X-Patchwork-Id: 13385538 X-Patchwork-Delegate: christophe.varoqui@free.fr Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id DA498EEAA4F for ; Thu, 14 Sep 2023 14:53:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1694703213; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=hBBhp1hOv5rxag6HRlrKOufVgndHaZHU4kG7wCNvdfM=; b=HHXFtG4IR4lq32D9AQRFAL195LAl+4g4TBtHoKtb0qJFH4J1FkmicutVjq/VQDL8b+4d4d 0fQrn3e6mk+Q+XuYIQ0cLnXOnjTD4WHjvCuIsOBdeAAhOmf2MfTOWWgShzIwzQw5wqAzII dw5AR/cKMlFhksruoWWqKCKbyZzKork= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-649-BOYsSMP9M1metmdC31PcKQ-1; Thu, 14 Sep 2023 10:53:29 -0400 X-MC-Unique: BOYsSMP9M1metmdC31PcKQ-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 70BCE81D88A; Thu, 14 Sep 2023 14:53:26 +0000 (UTC) Received: from mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com [10.30.29.100]) by smtp.corp.redhat.com (Postfix) with ESMTP id D61E61054FC0; Thu, 14 Sep 2023 14:53:22 +0000 (UTC) Received: from mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (localhost [IPv6:::1]) by mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (Postfix) with ESMTP id 5F2AE19465B8; Thu, 14 Sep 2023 14:53:17 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) by mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (Postfix) with ESMTP id 4DB0C19465B7 for ; Thu, 14 Sep 2023 14:53:11 +0000 (UTC) Received: by smtp.corp.redhat.com (Postfix) id 05FCF1054FC3; Thu, 14 Sep 2023 14:53:11 +0000 (UTC) Received: from mimecast-mx02.redhat.com (mimecast06.extmail.prod.ext.rdu2.redhat.com [10.11.55.22]) by smtp.corp.redhat.com (Postfix) with ESMTPS id F22601054FC1 for ; Thu, 14 Sep 2023 14:53:10 +0000 (UTC) Received: from us-smtp-inbound-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id CEF93185A79C for ; Thu, 14 Sep 2023 14:53:10 +0000 (UTC) Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-369-jtMbxX0uNLucP2B5yqTy7Q-1; Thu, 14 Sep 2023 10:53:08 -0400 X-MC-Unique: jtMbxX0uNLucP2B5yqTy7Q-1 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 952EF21847; Thu, 14 Sep 2023 14:53:07 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 60A96139DB; Thu, 14 Sep 2023 14:53:07 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id UIYYFlMeA2UHAgAAMHmgww (envelope-from ); Thu, 14 Sep 2023 14:53:07 +0000 From: mwilck@suse.com To: Christophe Varoqui , Benjamin Marzinski Date: Thu, 14 Sep 2023 16:51:29 +0200 Message-ID: <20230914145131.15165-2-mwilck@suse.com> In-Reply-To: <20230914145131.15165-1-mwilck@suse.com> References: <20230914145131.15165-1-mwilck@suse.com> MIME-Version: 1.0 X-Mimecast-Impersonation-Protect: Policy=CLT - Impersonation Protection Definition; Similar Internal Domain=false; Similar Monitored External Domain=false; Custom External Domain=false; Mimecast External Domain=false; Newly Observed Domain=false; Internal User Name=false; Custom Display Name List=false; Reply-to Address Mismatch=false; Targeted Threat Dictionary=false; Mimecast Threat Dictionary=false; Custom Threat Dictionary=false X-Scanned-By: MIMEDefang 3.1 on 10.11.54.3 Subject: [dm-devel] [PATCH v3 26/38] multipath-tools tests: add test for ordering of bindings X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: device-mapper development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: dm-devel@redhat.com, Martin Wilck Errors-To: dm-devel-bounces@redhat.com Sender: "dm-devel" X-Scanned-By: MIMEDefang 3.1 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: suse.com From: Martin Wilck As the assignment of free aliases now relies on the bindings being properly sorted, add some unit tests to make sure the sorting algorithm works. Signed-off-by: Martin Wilck Reviewed-by: Benjamin Marzinski --- tests/alias.c | 212 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 209 insertions(+), 3 deletions(-) diff --git a/tests/alias.c b/tests/alias.c index dff5f93..4d0adba 100644 --- a/tests/alias.c +++ b/tests/alias.c @@ -13,6 +13,9 @@ #include "globals.c" #include "../libmultipath/alias.c" +/* For verbose printing of all aliases in the ordering tests */ +#define ALIAS_DEBUG 0 + #if INT_MAX == 0x7fffffff /* user_friendly_name for map #INT_MAX */ #define MPATH_ID_INT_MAX "fxshrxw" @@ -439,11 +442,12 @@ static void mock_self_alias(const char *alias, const char *wwid) expect_condlog(3, USED_STR(alias, wwid)); \ } while(0) -static void __mock_bindings_file(const char *content) +static void __mock_bindings_file(const char *content, bool conflict_ok) { char *cnt __attribute__((cleanup(cleanup_charp))) = NULL; char *token, *savep = NULL; int i; + uintmax_t values[] = { BINDING_ADDED, BINDING_CONFLICT }; cnt = strdup(content); assert_ptr_not_equal(cnt, NULL); @@ -459,12 +463,12 @@ static void __mock_bindings_file(const char *content) continue; rc = add_binding(&global_bindings, alias, wwid); - assert_int_equal(rc, BINDING_ADDED); + assert_in_set(rc, values, conflict_ok ? 2 : 1); } } static void mock_bindings_file(const char *content) { - return __mock_bindings_file(content); + return __mock_bindings_file(content, false); } static int teardown_bindings(void **state) @@ -1744,6 +1748,207 @@ static int test_get_user_friendly_alias() return cmocka_run_group_tests(tests, NULL, NULL); } +/* Numbers 1-1000, randomly shuffled */ +static int random_numbers[1000] = { + 694, 977, 224, 178, 841, 818, 914, 549, 831, 942, 263, 834, 919, 800, + 111, 517, 719, 297, 988, 98, 332, 516, 754, 772, 495, 488, 331, 529, + 142, 747, 848, 618, 375, 624, 74, 753, 782, 944, 623, 468, 862, 997, + 417, 258, 298, 774, 673, 904, 883, 766, 867, 400, 11, 950, 14, 784, + 655, 155, 396, 9, 743, 93, 651, 245, 968, 306, 785, 581, 880, 486, + 168, 631, 203, 4, 663, 294, 702, 762, 619, 684, 48, 181, 21, 443, 643, + 863, 1000, 327, 26, 126, 382, 765, 586, 76, 49, 925, 319, 865, 797, + 876, 693, 334, 433, 243, 419, 901, 854, 326, 985, 347, 874, 527, 282, + 290, 380, 167, 95, 3, 257, 936, 60, 426, 227, 345, 577, 492, 467, 580, + 967, 422, 823, 718, 610, 64, 700, 412, 163, 288, 506, 828, 432, 51, + 356, 348, 539, 478, 17, 945, 602, 123, 450, 660, 429, 113, 310, 358, + 512, 758, 508, 19, 542, 304, 286, 446, 918, 723, 333, 603, 731, 978, + 230, 697, 109, 872, 175, 853, 947, 965, 121, 222, 101, 811, 117, 601, + 191, 752, 384, 415, 938, 278, 915, 715, 240, 552, 912, 838, 150, 840, + 627, 29, 636, 464, 861, 481, 992, 249, 934, 82, 368, 724, 807, 593, + 157, 147, 199, 637, 41, 62, 902, 505, 621, 342, 174, 260, 729, 961, + 219, 311, 629, 789, 81, 739, 860, 712, 223, 165, 741, 981, 485, 363, + 346, 709, 125, 369, 279, 634, 399, 162, 193, 769, 149, 314, 868, 612, + 524, 675, 341, 343, 476, 606, 388, 613, 850, 264, 903, 451, 908, 779, + 453, 148, 497, 46, 132, 43, 885, 955, 269, 395, 72, 128, 767, 989, + 929, 423, 742, 55, 13, 79, 924, 182, 295, 563, 668, 169, 974, 154, + 970, 54, 674, 52, 437, 570, 550, 531, 554, 793, 678, 218, 367, 105, + 197, 315, 958, 892, 86, 47, 284, 37, 561, 522, 198, 689, 817, 573, + 877, 201, 803, 501, 881, 546, 530, 523, 780, 579, 953, 135, 23, 620, + 84, 698, 303, 656, 357, 323, 494, 58, 131, 913, 995, 120, 70, 1, 195, + 365, 210, 25, 898, 173, 307, 239, 77, 418, 952, 963, 92, 455, 425, 12, + 536, 161, 328, 933, 401, 251, 735, 725, 362, 322, 557, 681, 302, 53, + 786, 801, 391, 946, 748, 133, 717, 851, 7, 372, 993, 387, 906, 373, + 667, 33, 670, 389, 209, 611, 896, 652, 69, 999, 344, 845, 633, 36, + 487, 192, 180, 45, 640, 427, 707, 805, 188, 152, 905, 217, 30, 252, + 386, 665, 299, 541, 410, 787, 5, 857, 751, 392, 44, 595, 146, 745, + 641, 957, 866, 773, 806, 815, 659, 102, 704, 430, 106, 296, 129, 847, + 130, 990, 669, 236, 225, 680, 159, 213, 438, 189, 447, 600, 232, 594, + 32, 56, 390, 647, 855, 428, 330, 714, 738, 706, 666, 461, 469, 482, + 558, 814, 559, 177, 575, 538, 309, 383, 261, 156, 420, 761, 630, 893, + 10, 116, 940, 844, 71, 377, 662, 312, 520, 244, 143, 759, 119, 186, + 592, 909, 864, 376, 768, 254, 265, 394, 511, 760, 574, 6, 436, 514, + 59, 226, 644, 956, 578, 825, 548, 145, 736, 597, 378, 821, 987, 897, + 354, 144, 722, 895, 589, 503, 826, 498, 543, 617, 763, 231, 808, 528, + 89, 479, 607, 737, 170, 404, 371, 65, 103, 340, 283, 141, 313, 858, + 289, 124, 971, 687, 954, 732, 39, 926, 176, 100, 267, 519, 890, 535, + 276, 448, 27, 457, 899, 385, 184, 275, 770, 544, 614, 449, 160, 658, + 259, 973, 108, 604, 24, 207, 562, 757, 744, 324, 444, 962, 591, 480, + 398, 409, 998, 253, 325, 445, 979, 8, 35, 118, 73, 683, 208, 85, 190, + 791, 408, 871, 657, 179, 18, 556, 496, 475, 20, 894, 484, 775, 889, + 463, 241, 730, 57, 907, 551, 859, 943, 185, 416, 870, 590, 435, 471, + 932, 268, 381, 626, 502, 565, 273, 534, 672, 778, 292, 473, 566, 104, + 172, 285, 832, 411, 329, 628, 397, 472, 271, 910, 711, 690, 969, 585, + 809, 941, 923, 555, 228, 685, 242, 94, 96, 211, 140, 61, 922, 795, + 869, 34, 255, 38, 984, 676, 15, 560, 632, 434, 921, 355, 582, 351, + 212, 200, 819, 960, 649, 852, 75, 771, 361, 996, 238, 316, 720, 671, + 462, 112, 569, 171, 664, 625, 588, 405, 553, 270, 533, 353, 842, 114, + 972, 83, 937, 63, 194, 237, 537, 980, 802, 916, 959, 688, 839, 350, + 917, 650, 545, 615, 151, 352, 686, 726, 266, 509, 439, 491, 935, 608, + 518, 653, 339, 609, 277, 635, 836, 88, 407, 440, 642, 927, 229, 727, + 360, 477, 846, 413, 454, 616, 28, 598, 567, 540, 790, 424, 247, 317, + 746, 911, 798, 321, 547, 248, 734, 829, 220, 138, 756, 500, 691, 196, + 740, 930, 843, 733, 221, 827, 50, 813, 949, 525, 349, 474, 134, 875, + 695, 513, 414, 515, 638, 99, 366, 490, 975, 246, 465, 206, 281, 583, + 256, 587, 749, 2, 951, 679, 215, 364, 458, 402, 646, 991, 335, 982, + 835, 300, 900, 703, 994, 983, 234, 888, 532, 804, 584, 305, 792, 442, + 291, 964, 158, 370, 452, 250, 521, 166, 948, 812, 794, 272, 699, 205, + 183, 507, 301, 920, 781, 233, 824, 137, 489, 833, 887, 966, 856, 78, + 830, 153, 359, 696, 526, 216, 66, 701, 403, 891, 849, 571, 308, 483, + 164, 293, 928, 677, 320, 837, 441, 639, 564, 510, 648, 274, 336, 661, + 878, 777, 816, 976, 493, 810, 67, 87, 91, 187, 882, 986, 80, 22, 499, + 90, 705, 139, 136, 122, 708, 716, 886, 572, 127, 40, 721, 764, 16, + 379, 692, 645, 456, 710, 460, 783, 97, 776, 713, 884, 115, 466, 596, + 374, 406, 110, 568, 68, 214, 622, 470, 107, 504, 682, 31, 421, 576, + 654, 605, 788, 799, 280, 338, 931, 873, 204, 287, 459, 755, 939, 599, + 431, 796, 235, 42, 750, 262, 318, 393, 202, 822, 879, 820, 728, 337, +}; + +static void fill_bindings_random(struct strbuf *buf, int start, int end, + const char *prefix) +{ + int i; + + for (i = start; i < end; i++) { + print_strbuf(buf, "%s", prefix); + format_devname(buf, random_numbers[i]); + print_strbuf(buf, " WWID%d\n", random_numbers[i]); + } +} + +struct random_aliases { + int start; + int end; + const char *prefix; +}; + +static void order_test(int n, struct random_aliases ra[], bool conflict_ok) +{ + STRBUF_ON_STACK(buf); + int i, j, prev, curr, tmp; + struct binding *bdg; + Bindings *bindings = &global_bindings; + + for (j = 0; j < n; j++) + fill_bindings_random(&buf, ra[j].start, ra[j].end, ra[j].prefix); + __mock_bindings_file(get_strbuf_str(&buf), conflict_ok); + + for (j = 0; j < n; j++) { + bdg = VECTOR_SLOT(bindings, 0); + if (ALIAS_DEBUG && j == 0) + printf("%d: %s\n", 0, bdg->alias); + prev = scan_devname(bdg->alias, ra[j].prefix); + i = 1; + vector_foreach_slot_after(bindings, bdg, i) { + if (ALIAS_DEBUG && j == 0) + printf("%d: %s\n", i, bdg->alias); + tmp = scan_devname(bdg->alias, ra[j].prefix); + if (tmp == -1) + continue; + curr = tmp; + if (prev > 0) { + if (curr <= prev) + printf("ERROR: %d (%s) %d >= %d\n", + i, bdg->alias, prev, curr); + assert_true(curr > prev); + } + prev = curr; + } + } +} + +static void order_01(void **state) +{ + struct random_aliases ra[] = { + { 0, 1000, "MPATH" }, + }; + + order_test(ARRAY_SIZE(ra), ra, false); +} + +static void order_02(void **state) +{ + struct random_aliases ra[] = { + { 0, 500, "MPATH" }, + { 200, 700, "mpath" }, + }; + order_test(ARRAY_SIZE(ra), ra, false); +} + +static void order_03(void **state) +{ + struct random_aliases ra[] = { + { 500, 1000, "MPTH" }, + { 0, 500, "MPATH" }, + }; + order_test(ARRAY_SIZE(ra), ra, false); +} + +static void order_04(void **state) +{ + struct random_aliases ra[] = { + { 0, 500, "mpa" }, + { 250, 750, "mp" }, + }; + order_test(ARRAY_SIZE(ra), ra, true); +} + +static void order_05(void **state) +{ + struct random_aliases ra[] = { + { 0, 100, "A" }, + { 0, 100, "B" }, + { 0, 100, "C" }, + { 0, 100, "D" }, + }; + order_test(ARRAY_SIZE(ra), ra, false); +} + +static void order_06(void **state) +{ + struct random_aliases ra[] = { + { 0, 100, "" }, + { 0, 100, "a" }, + { 0, 100, "aa" }, + { 0, 100, "ab" }, + { 0, 100, "aaa" }, + }; + order_test(ARRAY_SIZE(ra), ra, true); +} + +static int test_bindings_order() +{ + const struct CMUnitTest tests[] = { + cmocka_unit_test_teardown(order_01, teardown_bindings), + cmocka_unit_test_teardown(order_02, teardown_bindings), + cmocka_unit_test_teardown(order_03, teardown_bindings), + cmocka_unit_test_teardown(order_04, teardown_bindings), + cmocka_unit_test_teardown(order_05, teardown_bindings), + cmocka_unit_test_teardown(order_06, teardown_bindings), + }; + + return cmocka_run_group_tests(tests, NULL, NULL); +} + int main(void) { int ret = 0; @@ -1755,6 +1960,7 @@ int main(void) ret += test_rlookup_binding(); ret += test_allocate_binding(); ret += test_get_user_friendly_alias(); + ret += test_bindings_order(); return ret; } From patchwork Thu Sep 14 14:51:30 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Wilck X-Patchwork-Id: 13386388 X-Patchwork-Delegate: christophe.varoqui@free.fr Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 5E58CEE3F2F for ; Fri, 15 Sep 2023 06:45:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1694760333; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=qS/LqQSwtKITz1dz/xTw5Dw2KKOeexzy7FB/ey3S0VA=; b=MHWMkzs5JXVVsYT4qMxAXKTBx0wj09vGfQ0Qed4CfkMFgB7eZJpgVpAZVP4RMCZgsF7sEw +1JbHm2hhccP5eX+RkcJU2783j7HdeIv5uO4sLffs4X7nGOjirPQZqaGXyLkIEeacaheXW 79K8VJVkwWgGpaYOFo6cz5RRHDkprSE= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-271-Pi5g6ObhNdeao8yHiEHcRw-1; Fri, 15 Sep 2023 02:45:29 -0400 X-MC-Unique: Pi5g6ObhNdeao8yHiEHcRw-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 2348A857A9A; Fri, 15 Sep 2023 06:45:28 +0000 (UTC) Received: from mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com [10.30.29.100]) by smtp.corp.redhat.com (Postfix) with ESMTP id 710502026D4B; Fri, 15 Sep 2023 06:45:26 +0000 (UTC) Received: from mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (localhost [IPv6:::1]) by mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (Postfix) with ESMTP id B07EA19465BA; Fri, 15 Sep 2023 06:44:53 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) by mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (Postfix) with ESMTP id 6E7CC19466F9 for ; Thu, 14 Sep 2023 14:53:21 +0000 (UTC) Received: by smtp.corp.redhat.com (Postfix) id 5A3BE40C6EBF; Thu, 14 Sep 2023 14:53:16 +0000 (UTC) Received: from mimecast-mx02.redhat.com (mimecast02.extmail.prod.ext.rdu2.redhat.com [10.11.55.18]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 53BCA40C6EA8 for ; Thu, 14 Sep 2023 14:53:11 +0000 (UTC) Received: from us-smtp-inbound-delivery-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.81]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 1662980418B for ; Thu, 14 Sep 2023 14:53:11 +0000 (UTC) Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-457-pQ8o0T0qOAewAFJi4ovRTA-1; Thu, 14 Sep 2023 10:53:09 -0400 X-MC-Unique: pQ8o0T0qOAewAFJi4ovRTA-1 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id DBBF02185F; Thu, 14 Sep 2023 14:53:07 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id A25FA139DB; Thu, 14 Sep 2023 14:53:07 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id wOYUJlMeA2UHAgAAMHmgww (envelope-from ); Thu, 14 Sep 2023 14:53:07 +0000 From: mwilck@suse.com To: Christophe Varoqui , Benjamin Marzinski Date: Thu, 14 Sep 2023 16:51:30 +0200 Message-ID: <20230914145131.15165-3-mwilck@suse.com> In-Reply-To: <20230914145131.15165-1-mwilck@suse.com> References: <20230914145131.15165-1-mwilck@suse.com> MIME-Version: 1.0 X-Mimecast-Impersonation-Protect: Policy=CLT - Impersonation Protection Definition; Similar Internal Domain=false; Similar Monitored External Domain=false; Custom External Domain=false; Mimecast External Domain=false; Newly Observed Domain=false; Internal User Name=false; Custom Display Name List=false; Reply-to Address Mismatch=false; Targeted Threat Dictionary=false; Mimecast Threat Dictionary=false; Custom Threat Dictionary=false X-Scanned-By: MIMEDefang 3.1 on 10.11.54.2 X-Mailman-Approved-At: Fri, 15 Sep 2023 06:44:53 +0000 Subject: [dm-devel] [PATCH v3 27/38] multipathd: watch bindings file with inotify + timestamp X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: device-mapper development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: dm-devel@redhat.com, Martin Wilck Errors-To: dm-devel-bounces@redhat.com Sender: "dm-devel" X-Scanned-By: MIMEDefang 3.1 on 10.11.54.4 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: suse.com From: Martin Wilck Since "libmultipath: keep bindings in memory", we don't re-read the bindings file after every modification. Add a notification mechanism that makes multipathd aware of changes to the bindings file. Because multipathd itself will change the bindings file, it must compare timestamps in order to avoid reading the file repeatedly. Because select_alias() can be called from multiple thread contexts (uxlsnr, uevent handler), we need to add locking for the bindings file. The timestamp must also be protected by a lock, because it can't be read or written atomically. Note: The notification mechanism expects the bindings file to be atomically replaced by rename(2). Changes must be made in a temporary file and applied using rename(2), as in update_bindings_file(). The inotify mechanism deliberately does not listen to close-after-write events that would be generated by editing the bindings file directly. This Note also: new bindings will only be read from add_map_with_path(), i.e. either during reconfigure(), or when a new map is created during runtime. Existing maps will not be renamed if the binding file changes, unless the user runs "multipathd reconfigure". This is not a change wrt the previous code, but it should be mentioned anyway. Signed-off-by: Martin Wilck libmultipath: protect global_bindings with a mutex Signed-off-by: Martin Wilck libmultipath: check timestamp of bindings file before reading it Signed-off-by: Martin Wilck Reviewed-by: Benjamin Marzinski --- libmultipath/alias.c | 252 +++++++++++++++++++++++++----- libmultipath/alias.h | 3 +- libmultipath/libmultipath.version | 5 + multipathd/uxlsnr.c | 36 ++++- tests/alias.c | 3 + 5 files changed, 254 insertions(+), 45 deletions(-) diff --git a/libmultipath/alias.c b/libmultipath/alias.c index 66e34e3..964b8a7 100644 --- a/libmultipath/alias.c +++ b/libmultipath/alias.c @@ -10,6 +10,7 @@ #include #include #include +#include #include "debug.h" #include "util.h" @@ -22,6 +23,7 @@ #include "config.h" #include "devmapper.h" #include "strbuf.h" +#include "time-util.h" /* * significant parts of this file were taken from iscsi-bindings.c of the @@ -50,6 +52,12 @@ "# alias wwid\n" \ "#\n" +/* uatomic access only */ +static int bindings_file_changed = 1; + +static pthread_mutex_t timestamp_mutex = PTHREAD_MUTEX_INITIALIZER; +static struct timespec bindings_last_updated; + struct binding { char *alias; char *wwid; @@ -60,6 +68,9 @@ struct binding { * an abstract type. */ typedef struct _vector Bindings; + +/* Protect global_bindings */ +static pthread_mutex_t bindings_mutex = PTHREAD_MUTEX_INITIALIZER; static Bindings global_bindings = { .allocated = 0 }; enum { @@ -78,6 +89,27 @@ static void _free_binding(struct binding *bdg) free(bdg); } +static void free_bindings(Bindings *bindings) +{ + struct binding *bdg; + int i; + + vector_foreach_slot(bindings, bdg, i) + _free_binding(bdg); + vector_reset(bindings); +} + +static void set_global_bindings(Bindings *bindings) +{ + Bindings old_bindings; + + pthread_mutex_lock(&bindings_mutex); + old_bindings = global_bindings; + global_bindings = *bindings; + pthread_mutex_unlock(&bindings_mutex); + free_bindings(&old_bindings); +} + static const struct binding *get_binding_for_alias(const Bindings *bindings, const char *alias) { @@ -199,7 +231,8 @@ static int delete_binding(Bindings *bindings, const char *wwid) return BINDING_DELETED; } -static int write_bindings_file(const Bindings *bindings, int fd) +static int write_bindings_file(const Bindings *bindings, int fd, + struct timespec *ts) { struct binding *bnd; STRBUF_ON_STACK(content); @@ -227,9 +260,56 @@ static int write_bindings_file(const Bindings *bindings, int fd) } len -= n; } + fsync(fd); + if (ts) { + struct stat st; + + if (fstat(fd, &st) == 0) + *ts = st.st_mtim; + else + clock_gettime(CLOCK_REALTIME_COARSE, ts); + } return 0; } +void handle_bindings_file_inotify(const struct inotify_event *event) +{ + struct config *conf; + const char *base; + bool changed = false; + struct stat st; + struct timespec ts = { 0, 0 }; + int ret; + + if (!(event->mask & IN_MOVED_TO)) + return; + + conf = get_multipath_config(); + base = strrchr(conf->bindings_file, '/'); + changed = base && base > conf->bindings_file && + !strcmp(base + 1, event->name); + ret = stat(conf->bindings_file, &st); + put_multipath_config(conf); + + if (!changed) + return; + + pthread_mutex_lock(×tamp_mutex); + if (ret == 0) { + ts = st.st_mtim; + changed = timespeccmp(&ts, &bindings_last_updated) > 0; + } + pthread_mutex_unlock(×tamp_mutex); + + if (changed) { + uatomic_xchg(&bindings_file_changed, 1); + condlog(3, "%s: bindings file must be re-read, new timestamp: %ld.%06ld", + __func__, (long)ts.tv_sec, (long)ts.tv_nsec / 1000); + } else + condlog(3, "%s: bindings file is up-to-date, timestamp: %ld.%06ld", + __func__, (long)ts.tv_sec, (long)ts.tv_nsec / 1000); +} + static int update_bindings_file(const Bindings *bindings, const char *bindings_file) { @@ -237,6 +317,7 @@ static int update_bindings_file(const Bindings *bindings, int fd = -1; char tempname[PATH_MAX]; mode_t old_umask; + struct timespec ts; if (safe_sprintf(tempname, "%s.XXXXXX", bindings_file)) return -1; @@ -248,7 +329,7 @@ static int update_bindings_file(const Bindings *bindings, } umask(old_umask); pthread_cleanup_push(cleanup_fd_ptr, &fd); - rc = write_bindings_file(bindings, fd); + rc = write_bindings_file(bindings, fd, &ts); pthread_cleanup_pop(1); if (rc == -1) { condlog(1, "failed to write new bindings file"); @@ -257,8 +338,12 @@ static int update_bindings_file(const Bindings *bindings, } if ((rc = rename(tempname, bindings_file)) == -1) condlog(0, "%s: rename: %m", __func__); - else + else { + pthread_mutex_lock(×tamp_mutex); + bindings_last_updated = ts; + pthread_mutex_unlock(×tamp_mutex); condlog(1, "updated bindings file %s", bindings_file); + } return rc; } @@ -387,6 +472,7 @@ int get_free_id(const Bindings *bindings, const char *prefix, const char *map_ww return id; } +/* Called with binding_mutex held */ static char * allocate_binding(const char *filename, const char *wwid, int id, const char *prefix) { @@ -423,6 +509,30 @@ allocate_binding(const char *filename, const char *wwid, int id, const char *pre return alias; } +enum { + BINDINGS_FILE_UP2DATE, + BINDINGS_FILE_READ, + BINDINGS_FILE_ERROR, + BINDINGS_FILE_BAD, +}; + +static int _read_bindings_file(const struct config *conf, Bindings *bindings, + bool force); + +static void read_bindings_file(void) +{ + struct config *conf; + Bindings bindings = {.allocated = 0, }; + int rc; + + conf = get_multipath_config(); + pthread_cleanup_push(put_multipath_config, conf); + rc = _read_bindings_file(conf, &bindings, false); + pthread_cleanup_pop(1); + if (rc == BINDINGS_FILE_READ) + set_global_bindings(&bindings); +} + /* * get_user_friendly_alias() action table * @@ -463,6 +573,11 @@ char *get_user_friendly_alias(const char *wwid, const char *file, const char *al bool new_binding = false; const struct binding *bdg; + read_bindings_file(); + + pthread_mutex_lock(&bindings_mutex); + pthread_cleanup_push(cleanup_mutex, &bindings_mutex); + if (!*alias_old) goto new_alias; @@ -514,40 +629,40 @@ new_alias: alias, wwid); out: + /* unlock bindings_mutex */ + pthread_cleanup_pop(1); return alias; } int get_user_friendly_wwid(const char *alias, char *buff) { const struct binding *bdg; + int rc = -1; if (!alias || *alias == '\0') { condlog(3, "Cannot find binding for empty alias"); return -1; } + read_bindings_file(); + + pthread_mutex_lock(&bindings_mutex); + pthread_cleanup_push(cleanup_mutex, &bindings_mutex); bdg = get_binding_for_alias(&global_bindings, alias); - if (!bdg) { + if (bdg) { + strlcpy(buff, bdg->wwid, WWID_SIZE); + rc = 0; + } else *buff = '\0'; - return -1; - } - strlcpy(buff, bdg->wwid, WWID_SIZE); - return 0; -} - -static void free_bindings(Bindings *bindings) -{ - struct binding *bdg; - int i; - - vector_foreach_slot(bindings, bdg, i) - _free_binding(bdg); - vector_reset(bindings); + pthread_cleanup_pop(1); + return rc; } void cleanup_bindings(void) { + pthread_mutex_lock(&bindings_mutex); free_bindings(&global_bindings); + pthread_mutex_unlock(&bindings_mutex); } enum { @@ -595,7 +710,20 @@ static int _check_bindings_file(const struct config *conf, FILE *file, char *line = NULL; size_t line_len = 0; ssize_t n; + char header[sizeof(BINDINGS_FILE_HEADER)]; + header[sizeof(BINDINGS_FILE_HEADER) - 1] = '\0'; + if (fread(header, sizeof(BINDINGS_FILE_HEADER) - 1, 1, file) < 1) { + condlog(2, "%s: failed to read header from %s", __func__, + conf->bindings_file); + fseek(file, 0, SEEK_SET); + rc = -1; + } else if (strcmp(header, BINDINGS_FILE_HEADER)) { + condlog(2, "%s: invalid header in %s", __func__, + conf->bindings_file); + fseek(file, 0, SEEK_SET); + rc = -1; + } pthread_cleanup_push(cleanup_free_ptr, &line); while ((n = getline(&line, &line_len, file)) >= 0) { char *alias, *wwid; @@ -643,6 +771,68 @@ static int mp_alias_compar(const void *p1, const void *p2) &((*(struct mpentry * const *)p2)->alias)); } +static int _read_bindings_file(const struct config *conf, Bindings *bindings, + bool force) +{ + int can_write; + int rc = 0, ret, fd; + FILE *file; + struct stat st; + int has_changed = uatomic_xchg(&bindings_file_changed, 0); + + if (!force) { + if (!has_changed) { + condlog(4, "%s: bindings are unchanged", __func__); + return BINDINGS_FILE_UP2DATE; + } + } + + fd = open_file(conf->bindings_file, &can_write, BINDINGS_FILE_HEADER); + if (fd == -1) + return BINDINGS_FILE_ERROR; + + file = fdopen(fd, "r"); + if (file != NULL) { + condlog(3, "%s: reading %s", __func__, conf->bindings_file); + + pthread_cleanup_push(cleanup_fclose, file); + ret = _check_bindings_file(conf, file, bindings); + if (ret == 0) { + struct timespec ts; + + rc = BINDINGS_FILE_READ; + ret = fstat(fd, &st); + if (ret == 0) + ts = st.st_mtim; + else { + condlog(1, "%s: fstat failed (%m), using current time", __func__); + clock_gettime(CLOCK_REALTIME_COARSE, &ts); + } + pthread_mutex_lock(×tamp_mutex); + bindings_last_updated = ts; + pthread_mutex_unlock(×tamp_mutex); + } else if (ret == -1 && can_write && !conf->bindings_read_only) { + ret = update_bindings_file(bindings, conf->bindings_file); + if (ret == 0) + rc = BINDINGS_FILE_READ; + else + rc = BINDINGS_FILE_BAD; + } else { + condlog(0, "ERROR: bad settings in read-only bindings file %s", + conf->bindings_file); + rc = BINDINGS_FILE_BAD; + } + pthread_cleanup_pop(1); + } else { + condlog(1, "failed to fdopen %s: %m", + conf->bindings_file); + close(fd); + rc = BINDINGS_FILE_ERROR; + } + + return rc; +} + /* * check_alias_settings(): test for inconsistent alias configuration * @@ -661,8 +851,7 @@ static int mp_alias_compar(const void *p1, const void *p2) */ int check_alias_settings(const struct config *conf) { - int can_write; - int rc = 0, i, fd; + int i, rc; Bindings bindings = {.allocated = 0, }; vector mptable = NULL; struct mpentry *mpe; @@ -695,27 +884,12 @@ int check_alias_settings(const struct config *conf) pthread_cleanup_pop(1); pthread_cleanup_pop(1); - fd = open_file(conf->bindings_file, &can_write, BINDINGS_FILE_HEADER); - if (fd != -1) { - FILE *file = fdopen(fd, "r"); + rc = _read_bindings_file(conf, &bindings, true); - if (file != NULL) { - pthread_cleanup_push(cleanup_fclose, file); - rc = _check_bindings_file(conf, file, &bindings); - pthread_cleanup_pop(1); - if (rc == -1 && can_write && !conf->bindings_read_only) - rc = update_bindings_file(&bindings, conf->bindings_file); - else if (rc == -1) - condlog(0, "ERROR: bad settings in read-only bindings file %s", - conf->bindings_file); - } else { - condlog(1, "failed to fdopen %s: %m", - conf->bindings_file); - close(fd); - } + if (rc == BINDINGS_FILE_READ) { + set_global_bindings(&bindings); + rc = 0; } - cleanup_bindings(); - global_bindings = bindings; return rc; } diff --git a/libmultipath/alias.h b/libmultipath/alias.h index 5ef6720..ca8911f 100644 --- a/libmultipath/alias.h +++ b/libmultipath/alias.h @@ -10,5 +10,6 @@ char *get_user_friendly_alias(const char *wwid, const char *file, struct config; int check_alias_settings(const struct config *); void cleanup_bindings(void); - +struct inotify_event; +void handle_bindings_file_inotify(const struct inotify_event *event); #endif /* _ALIAS_H */ diff --git a/libmultipath/libmultipath.version b/libmultipath/libmultipath.version index ddd302f..57e50c1 100644 --- a/libmultipath/libmultipath.version +++ b/libmultipath/libmultipath.version @@ -238,3 +238,8 @@ global: local: *; }; + +LIBMULTIPATH_20.1.0 { +global: + handle_bindings_file_inotify; +}; diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c index 02e89fb..d1f8f23 100644 --- a/multipathd/uxlsnr.c +++ b/multipathd/uxlsnr.c @@ -41,6 +41,7 @@ #include "cli.h" #include "uxlsnr.h" #include "strbuf.h" +#include "alias.h" /* state of client connection */ enum { @@ -190,6 +191,7 @@ void wakeup_cleanup(void *arg) struct watch_descriptors { int conf_wd; int dir_wd; + int mp_wd; /* /etc/multipath; for bindings file */ }; /* failing to set the watch descriptor is o.k. we just miss a warning @@ -200,6 +202,8 @@ static void reset_watch(int notify_fd, struct watch_descriptors *wds, struct config *conf; int dir_reset = 0; int conf_reset = 0; + int mp_reset = 0; + char *bindings_file __attribute__((cleanup(cleanup_charp))) = NULL; if (notify_fd == -1) return; @@ -214,7 +218,10 @@ static void reset_watch(int notify_fd, struct watch_descriptors *wds, conf_reset = 1; if (wds->dir_wd == -1) dir_reset = 1; + if (wds->mp_wd == -1) + mp_reset = 1; } + bindings_file = strdup(conf->bindings_file); put_multipath_config(conf); if (dir_reset) { @@ -235,7 +242,18 @@ static void reset_watch(int notify_fd, struct watch_descriptors *wds, if (wds->conf_wd == -1) condlog(3, "didn't set up notifications on /etc/multipath.conf: %m"); } - return; + if (mp_reset && bindings_file) { + char *slash = strrchr(bindings_file, '/'); + + if (slash && slash > bindings_file) { + *slash = '\0'; + wds->mp_wd = inotify_add_watch(notify_fd, bindings_file, + IN_MOVED_TO|IN_ONLYDIR); + if (wds->mp_wd == -1) + condlog(3, "didn't set up notifications on %s: %m", + bindings_file); + } + } } static void handle_inotify(int fd, struct watch_descriptors *wds) @@ -256,12 +274,13 @@ static void handle_inotify(int fd, struct watch_descriptors *wds) inotify_rm_watch(fd, wds->conf_wd); if (wds->dir_wd != -1) inotify_rm_watch(fd, wds->dir_wd); - wds->conf_wd = wds->dir_wd = -1; + if (wds->mp_wd != -1) + inotify_rm_watch(fd, wds->mp_wd); + wds->conf_wd = wds->dir_wd = wds->mp_wd = -1; } break; } - got_notify = 1; for (ptr = buff; ptr < buff + len; ptr += sizeof(struct inotify_event) + event->len) { event = (const struct inotify_event *) ptr; @@ -273,7 +292,13 @@ static void handle_inotify(int fd, struct watch_descriptors *wds) wds->conf_wd = inotify_add_watch(notify_fd, DEFAULT_CONFIGFILE, IN_CLOSE_WRITE); else if (wds->dir_wd == event->wd) wds->dir_wd = -1; + else if (wds->mp_wd == event->wd) + wds->mp_wd = -1; } + if (wds->mp_wd != -1 && wds->mp_wd == event->wd) + handle_bindings_file_inotify(event); + else + got_notify = 1; } } if (got_notify) @@ -599,7 +624,7 @@ void *uxsock_listen(long ux_sock, void *trigger_data) int max_pfds = MIN_POLLS + POLLFDS_BASE; /* conf->sequence_nr will be 1 when uxsock_listen is first called */ unsigned int sequence_nr = 0; - struct watch_descriptors wds = { .conf_wd = -1, .dir_wd = -1 }; + struct watch_descriptors wds = { .conf_wd = -1, .dir_wd = -1, .mp_wd = -1, }; struct vectors *vecs = trigger_data; condlog(3, "uxsock: startup listener"); @@ -666,7 +691,8 @@ void *uxsock_listen(long ux_sock, void *trigger_data) reset_watch(notify_fd, &wds, &sequence_nr); polls[POLLFD_NOTIFY].fd = notify_fd; - if (notify_fd == -1 || (wds.conf_wd == -1 && wds.dir_wd == -1)) + if (notify_fd == -1 || (wds.conf_wd == -1 && wds.dir_wd == -1 + && wds.mp_wd == -1)) polls[POLLFD_NOTIFY].events = 0; else polls[POLLFD_NOTIFY].events = POLLIN; diff --git a/tests/alias.c b/tests/alias.c index 4d0adba..d41d396 100644 --- a/tests/alias.c +++ b/tests/alias.c @@ -1954,6 +1954,9 @@ int main(void) int ret = 0; init_test_verbosity(3); + /* avoid open_file() call in _read_bindings_file */ + bindings_file_changed = 0; + ret += test_format_devname(); ret += test_scan_devname(); ret += test_lookup_binding(); From patchwork Thu Sep 14 14:51:31 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Wilck X-Patchwork-Id: 13385540 X-Patchwork-Delegate: christophe.varoqui@free.fr Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id B95EAEEAA4E for ; Thu, 14 Sep 2023 14:53:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1694703232; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=yhsJu3k9rBZ4WFmqbUhG9moC0n1mhUyTkCJqcNWOOU4=; b=RXNBUlQz+kpgnFfMQAFsQ+DY97mSsxibUzuZNNM6MpwGTnYBmlKmmFwWReF/czPVftm1FX rBG9p1bHaZjZKugm7PbO60MZ4ArskZXxzSnpTXQdtBGALvRVLd7KcnqFD/J7t7nQ3dKp3b KluO0qnDtSsWQ11KowQqEzpSmXMtdL8= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-401-Z-_Rvn5eNOK4RUjyCoX0iQ-1; Thu, 14 Sep 2023 10:53:49 -0400 X-MC-Unique: Z-_Rvn5eNOK4RUjyCoX0iQ-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 3DA8B185A78E; Thu, 14 Sep 2023 14:53:40 +0000 (UTC) Received: from mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com [10.30.29.100]) by smtp.corp.redhat.com (Postfix) with ESMTP id ECA9E1678B; Thu, 14 Sep 2023 14:53:39 +0000 (UTC) Received: from mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (localhost [IPv6:::1]) by mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (Postfix) with ESMTP id 4D81A19466F1; Thu, 14 Sep 2023 14:53:19 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) by mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (Postfix) with ESMTP id 8FFEE19466E7 for ; Thu, 14 Sep 2023 14:53:18 +0000 (UTC) Received: by smtp.corp.redhat.com (Postfix) id 78ADF28AC; Thu, 14 Sep 2023 14:53:13 +0000 (UTC) Received: from mimecast-mx02.redhat.com (mimecast05.extmail.prod.ext.rdu2.redhat.com [10.11.55.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 711947B62 for ; Thu, 14 Sep 2023 14:53:13 +0000 (UTC) Received: from us-smtp-inbound-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 5308A81D88E for ; Thu, 14 Sep 2023 14:53:13 +0000 (UTC) Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-186-83iudY09OtGOqYO-xqH5dg-1; Thu, 14 Sep 2023 10:53:09 -0400 X-MC-Unique: 83iudY09OtGOqYO-xqH5dg-1 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 2B3C721862; Thu, 14 Sep 2023 14:53:08 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id E73F1139DB; Thu, 14 Sep 2023 14:53:07 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id gCa0NlMeA2UHAgAAMHmgww (envelope-from ); Thu, 14 Sep 2023 14:53:07 +0000 From: mwilck@suse.com To: Christophe Varoqui , Benjamin Marzinski Date: Thu, 14 Sep 2023 16:51:31 +0200 Message-ID: <20230914145131.15165-4-mwilck@suse.com> In-Reply-To: <20230914145131.15165-1-mwilck@suse.com> References: <20230914145131.15165-1-mwilck@suse.com> MIME-Version: 1.0 X-Mimecast-Impersonation-Protect: Policy=CLT - Impersonation Protection Definition; Similar Internal Domain=false; Similar Monitored External Domain=false; Custom External Domain=false; Mimecast External Domain=false; Newly Observed Domain=false; Internal User Name=false; Custom Display Name List=false; Reply-to Address Mismatch=false; Targeted Threat Dictionary=false; Mimecast Threat Dictionary=false; Custom Threat Dictionary=false X-Scanned-By: MIMEDefang 3.1 on 10.11.54.5 Subject: [dm-devel] [PATCH v3 38/38] libmultipath: avoid -Warray-bounds error in uatomic operations X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: device-mapper development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: dm-devel@redhat.com, Martin Wilck Errors-To: dm-devel-bounces@redhat.com Sender: "dm-devel" X-Scanned-By: MIMEDefang 3.1 on 10.11.54.5 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: suse.com From: Martin Wilck The use of uatomic_xchg() in alias.c causes a -Warray-bounds error on distributions using gcc 12, such as Fedora 37. This is a similar error to 2534c4f ("libmultipath: avoid -Warray-bounds error with gcc 12 and musl libc"). This happens only with liburcu 0.13 and earlier, and only with certain gcc versions. See liburcu commit 835b9ab ("Fix: x86 and s390 uatomic: __hp() macro warning with gcc 11"). Enhance the fix for 2534c4f by a adding a workaround for uatomic_xchg(), and introduce the macro URCU_VERSION (originally only used for multipathd) globally. Signed-off-by: Martin Wilck Reviewed-by: Benjamin Marzinski --- Makefile.inc | 2 +- create-config.mk | 5 +++++ libmultipath/alias.c | 5 +++-- libmultipath/lock.h | 23 ++++++++++++++--------- multipathd/Makefile | 2 -- 5 files changed, 23 insertions(+), 14 deletions(-) diff --git a/Makefile.inc b/Makefile.inc index 6e384e6..04bfa56 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -95,7 +95,7 @@ OPTFLAGS := -O2 -g $(STACKPROT) --param=ssp-buffer-size=4 WARNFLAGS := -Werror -Wall -Wextra -Wformat=2 $(WFORMATOVERFLOW) -Werror=implicit-int \ -Werror=implicit-function-declaration -Werror=format-security \ $(WNOCLOBBERED) -Werror=cast-qual $(ERROR_DISCARDED_QUALIFIERS) $(W_URCU_TYPE_LIMITS) -CPPFLAGS := $(FORTIFY_OPT) $(CPPFLAGS) \ +CPPFLAGS := $(FORTIFY_OPT) $(CPPFLAGS) $(D_URCU_VERSION) \ -DBIN_DIR=\"$(bindir)\" -DMULTIPATH_DIR=\"$(plugindir)\" \ -DRUNTIME_DIR=\"$(runtimedir)\" -DCONFIG_DIR=\"$(configdir)\" \ -DDEFAULT_CONFIGFILE=\"$(configfile)\" -DSTATE_DIR=\"$(statedir)\" \ diff --git a/create-config.mk b/create-config.mk index d125597..4d318b9 100644 --- a/create-config.mk +++ b/create-config.mk @@ -73,6 +73,10 @@ TEST_URCU_TYPE_LIMITS = $(shell \ $(CC) -c -Werror=type-limits -o /dev/null -xc - 2>/dev/null \ || echo -Wno-type-limits ) +URCU_VERSION = $(shell \ + $(PKG_CONFIG) --modversion liburcu 2>/dev/null | \ + awk -F. '{ printf("-DURCU_VERSION=0x%06x", 256 * ( 256 * $$1 + $$2) + $$3); }') + DEFINES := ifneq ($(call check_func,dm_task_no_flush,$(devmapper_incdir)/libdevmapper.h),0) @@ -168,6 +172,7 @@ $(TOPDIR)/config.mk: $(multipathdir)/autoconfig.h @echo creating $@ @echo "FPIN_SUPPORT := $(FPIN_SUPPORT)" >$@ @echo "FORTIFY_OPT := $(FORTIFY_OPT)" >>$@ + @echo "D_URCU_VERSION := $(call URCU_VERSION)" >>$@ @echo "SYSTEMD := $(SYSTEMD)" >>$@ @echo "ANA_SUPPORT := $(ANA_SUPPORT)" >>$@ @echo "STACKPROT := $(call TEST_CC_OPTION,-fstack-protector-strong,-fstack-protector)" >>$@ diff --git a/libmultipath/alias.c b/libmultipath/alias.c index e5d3f15..74431f3 100644 --- a/libmultipath/alias.c +++ b/libmultipath/alias.c @@ -24,6 +24,7 @@ #include "devmapper.h" #include "strbuf.h" #include "time-util.h" +#include "lock.h" /* * significant parts of this file were taken from iscsi-bindings.c of the @@ -300,7 +301,7 @@ void handle_bindings_file_inotify(const struct inotify_event *event) pthread_mutex_unlock(×tamp_mutex); if (changed) { - uatomic_xchg(&bindings_file_changed, 1); + uatomic_xchg_int(&bindings_file_changed, 1); condlog(3, "%s: bindings file must be re-read, new timestamp: %ld.%06ld", __func__, (long)ts.tv_sec, (long)ts.tv_nsec / 1000); } else @@ -775,7 +776,7 @@ static int _read_bindings_file(const struct config *conf, Bindings *bindings, int rc = 0, ret, fd; FILE *file; struct stat st; - int has_changed = uatomic_xchg(&bindings_file_changed, 0); + int has_changed = uatomic_xchg_int(&bindings_file_changed, 0); if (!force) { if (!has_changed) { diff --git a/libmultipath/lock.h b/libmultipath/lock.h index 9814be7..ac80d1d 100644 --- a/libmultipath/lock.h +++ b/libmultipath/lock.h @@ -13,17 +13,22 @@ struct mutex_lock { int waiters; /* uatomic access only */ }; -#if !defined(__GLIBC__) && defined(__GNUC__) && __GNUC__ == 12 -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Warray-bounds" -#endif - static inline void init_lock(struct mutex_lock *a) { pthread_mutex_init(&a->mutex, NULL); uatomic_set(&a->waiters, 0); } +#if defined(__GNUC__) && __GNUC__ == 12 && URCU_VERSION < 0xe00 +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Warray-bounds" +#endif + +static inline int uatomic_xchg_int(int *ptr, int val) +{ + return uatomic_xchg(ptr, val); +} + static inline void lock(struct mutex_lock *a) { uatomic_inc(&a->waiters); @@ -31,6 +36,10 @@ static inline void lock(struct mutex_lock *a) uatomic_dec(&a->waiters); } +#if defined(__GNUC__) && __GNUC__ == 12 && URCU_VERSION < 0xe00 +#pragma GCC diagnostic pop +#endif + static inline int trylock(struct mutex_lock *a) { return pthread_mutex_trylock(&a->mutex); @@ -51,10 +60,6 @@ static inline bool lock_has_waiters(struct mutex_lock *a) return (uatomic_read(&a->waiters) > 0); } -#if !defined(__GLIBC__) && defined(__GNUC__) && __GNUC__ == 12 -#pragma GCC diagnostic pop -#endif - #define lock_cleanup_pop(a) pthread_cleanup_pop(1) void cleanup_lock (void * data); diff --git a/multipathd/Makefile b/multipathd/Makefile index cdba3db..0ba6ecb 100644 --- a/multipathd/Makefile +++ b/multipathd/Makefile @@ -5,8 +5,6 @@ CLI := multipathc MANPAGES := multipathd.8 CPPFLAGS += -I$(multipathdir) -I$(mpathutildir) -I$(mpathpersistdir) -I$(mpathcmddir) -I$(thirdpartydir) \ - $(shell $(PKG_CONFIG) --modversion liburcu 2>/dev/null | \ - awk -F. '{ printf("-DURCU_VERSION=0x%06x", 256 * ( 256 * $$1 + $$2) + $$3); }') \ -DBINDIR='"$(bindir)"' $(SYSTEMD_CPPFLAGS) #