Bug 1206791

Summary: drbd: fix build error against kernel v6.1.1
Product: [openSUSE] openSUSE Tumbleweed Reporter: heming zhao <heming.zhao>
Component: High AvailabilityAssignee: heming zhao <heming.zhao>
Status: RESOLVED FIXED QA Contact: E-mail List <qa-bugs>
Severity: Major    
Priority: P2 - High CC: zzhou
Version: Current   
Target Milestone: ---   
Hardware: x86-64   
OS: Other   
Whiteboard:
Found By: --- Services Priority:
Business Priority: Blocker: ---
Marketing QA Status: --- IT Deployment: ---

Description heming zhao 2023-01-02 23:51:17 UTC
kernel v6.1.2 block layer APIs changed and was blocking drbd building.
Comment 1 heming zhao 2023-01-03 00:16:27 UTC
(In reply to heming zhao from comment #0)
> kernel v6.1.2 block layer APIs changed and was blocking drbd building.

sorry for typo, kernel version should be v6.1.1
Comment 2 heming zhao 2023-01-05 09:44:15 UTC
from this comment I record my works.

---------------
modify bsc-1201335_06-bdi.patch commit log

in bsc-1201335_06-bdi.patch, add missing upstream patch commit log:

```
commit b807a2c5e0e2e81c96160682977c6f497cfcee96
Author: Christoph Hellwig <hch@lst.de>
Date:   Thu Sep 24 08:51:29 2020 +0200

    drbd: remove dead code in device_to_statistics

    Ever since the switch to blk-mq, a lower device not used for VM
    writeback will not be marked congested, so the check will never
    trigger.

    Signed-off-by: Christoph Hellwig <hch@lst.de>
    Reviewed-by: Jan Kara <jack@suse.cz>
    Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
    Signed-off-by: Jens Axboe <axboe@kernel.dk>
```

this patch needs drbd commit c89bb88cde9349b1 to fix another issue.

-----------------
replace old patch:
  bsc-1204596_02-drbd-stop-using-bdevname-in-drbd_report_io_error.patch
with new: bsc-1204596_02-drbd-remove-usage-of-bdevname.patch


From d9a0a8d2c02f7a8da542230bebf0045f24f0fc74 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christoph=20B=C3=B6hmwalder?=
 <christoph.boehmwalder@linbit.com>
Date: Thu, 13 Oct 2022 15:00:46 +0200


warning:
```
/usr/src/packages/BUILD/drbd-9.0.30~1+git.8e9c0812/default/drbd_req.c: In function ‘drbd_report_io_error’:
/usr/src/packages/BUILD/drbd-9.0.30~1+git.8e9c0812/default/drbd_req.c:743:14: warning: unused variable ‘b’ [-Wunused-variable]
  743 |         char b[BDEVNAME_SIZE];
                     ^
```
Comment 3 heming zhao 2023-01-05 09:45:24 UTC
===> bsc-1206791-01-drbd-fix-static-analysis-warnings.patch


From f465ef1a70c7b559fcf3924e450bc5e836e448de Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christoph=20B=C3=B6hmwalder?=
 <christoph.boehmwalder@linbit.com>
Date: Mon, 3 Oct 2022 15:56:44 +0200
Subject: [PATCH] drbd: fix static analysis warnings



```
/usr/src/packages/BUILD/drbd-9.0.30~1+git.8e9c0812/default/drbd_nl.c: In function ‘_check_net_options’:
/usr/src/packages/BUILD/drbd-9.0.30~1+git.8e9c0812/default/drbd_nl.c:3523:21: warning: the comparison will
 always evaluate as ‘true’ for the address of ‘integrity_alg’ will never be NULL [-Waddress]
 3523 |                 if (!new_net_conf->integrity_alg != !old_net_conf->integrity_alg)
      |                     ^
```

--------------------

=========>  bsc-1206791-02-drbd-add-comments-explaining-removal-of-bdi-congesti.patch

From c89bb88cde9349b11dbf255e762203c626e70d30 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christoph=20B=C3=B6hmwalder?=
 <christoph.boehmwalder@linbit.com>
Date: Mon, 3 Oct 2022 14:49:55 +0200
Subject: [PATCH] drbd: add comments explaining removal of bdi congestion

The bdi congestion tracking framework was removed, so this leaves a few
confusing places where we just blindly set a "congested" flag to false.
Add some comments, briefly explaining the history of why it is like
that.



explaination:
bsc-1201335_06-bdi.patch followed upstream patch b807a2c5e0e2, which
removed s->dev_lower_blocked from device_to_statistics(). But drbd main
branch latest code still keep s->dev_lower_blocked, I used this patch
to restore s->dev_lower_blocked.

--------------------

==========> bsc-1206791-03-drbd-fix-warning-about-initializing-multiple-struct-.patch


From 1b4b7a945d5191225965d8e79fc6705c1b394e1d Mon Sep 17 00:00:00 2001
From: Philipp Reisner <philipp.reisner@linbit.com>
Date: Mon, 10 Oct 2022 09:20:20 +0200
Subject: [PATCH] drbd: fix warning about initializing multiple struct members
 with one memset()


fix compiling warning:

```
In function ‘fortify_memset_chk’,
    inlined from ‘drbd_send_sync_param’ at /usr/src/packages/BUILD/drbd-9.0.30~1+git.8e9c0812/default/drbd_main.c:1196:2:
/usr/src/linux-6.1.1-1/include/linux/fortify-string.h:314:25: warning: call to ‘__write_overflow_field’ declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Wattribute-warning]
  314 |                         __write_overflow_field(p_size_field, size);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
Comment 4 heming zhao 2023-01-05 09:46:57 UTC
  GENPATCHNAMES   6.1.1-1-default
  APPLIED_COCCI_FILES
blk_queue_split__no_present            [1]
prandom_u32__no_present                [2]
genl_policy__yes_in_ops                [3]
write_zeroes__no_capable               [4]
write_same__no_capable                 [5]
wb_congested_enum__no_present          [6]
queue_discard_zeroes_data__no_present  [7]


[1] blk_queue_split__no_present:

modify drbd-kernel-compat/gen_patch_names.c, add bio_split_to_limits support.

create:
- drbd-kernel-compat/cocci/bio_split_to_limits__yes_present.cocci
- drbd-kernel-compat/tests/have_bio_split_to_limits.c

conclusion: create patch bsc-1206791-04-blk_queue_split__no_present.patch



[2] prandom_u32__no_present:
 prandom_u32 already introduce since f8be841a6f02453ecf3b69dda1ccfc5b75cefa01

.compat_test.6.1.1/have_prandom_u32.stderr
```
<command-line>: warning: "KBUILD_MODNAME" redefined
<command-line>: note: this is the location of the previous definition
/usr/src/packages/BUILD/drbd-9.0.30~1+git.8e9c0812/default/drbd-kernel-compat/tests/have_prandom_u32.c: In function ‘main’:
/usr/src/packages/BUILD/drbd-9.0.30~1+git.8e9c0812/default/drbd-kernel-compat/tests/have_prandom_u32.c:6:17: error: implicit declaration of function ‘prandom_u32’; did you mean ‘prandom_u32_max’? [-Werror=implicit-function-declaration]
    6 |         u32 r = prandom_u32();
      |                 ^~~~~~~~~~~
      |                 prandom_u32_max
cc1: some warnings being treated as errors
```

prandom_u32_max introduced by commit c0842fbc1b18c7a044e6ff3e8fa78bfa822c7d1a
linux upstream kernel commit 81895a65ec63ee1daec3255dc1a06675d2fbe915 change drbd using from
prandom_u32 to prandom_u32_max.
prandom_u32 removed by commit de492c83cae0af72de370b9404aacda93dafcad5.

conclusion: create patch: bsc-1206791-05-prandom_u32_max.patch


[3] genl_policy__yes_in_ops

```
    patch(1, "genl_policy", false, true,
          COMPAT_GENL_POLICY_IN_OPS, "in_ops");
```

generate define: #define COMPAT_GENL_POLICY_IN_OPS

conclusion: under expectation, no need to change.

[4] write_zeroes__no_capable

```
    patch(1, "write_zeroes", true, false,
          COMPAT_HAVE_REQ_OP_WRITE_ZEROES, "capable");
```

test file: drbd-kernel-compat/tests/have_req_op_write_zeroes.c

error msg:

```
/usr/src/packages/BUILD/drbd-9.0.30~1+git.8e9c0812/default/drbd-kernel-compat/tests/have_req_op_write_zeroes.c:3:6: error: variable ‘dummy’ has initializer but incomplete type
    3 | enum req_opf dummy = REQ_OP_WRITE_ZEROES;
      |      ^~~~~~~
/usr/src/packages/BUILD/drbd-9.0.30~1+git.8e9c0812/default/drbd-kernel-compat/tests/have_req_op_write_zeroes.c:3:14: error: storage size of ‘dummy’ isn’t known
    3 | enum req_opf dummy = REQ_OP_WRITE_ZEROES;
      |              ^~~~~
```

result: #undef COMPAT_HAVE_REQ_OP_WRITE_ZEROES

solution:
change have_req_op_write_zeroes.c from "enum req_opf" to "enum req_op". then test
code will think REQ_OP_WRITE_ZEROES is supported.

conclusion: create patch bsc-1206791-06-write_zeroes__no_capable.patch.

[5] write_same__no_capable

first, upstream linux kernel had already removed write_same option by commit:
a34592ff6b78e84e11b19183b60cd240737f76f9

second, upstream drbd main branch commit 6842600a876d6cc4ae48f1a6735561b8ba9c5920
remove all drbd write_same code.

current opensuse TW using drbd-9.0 branch, which following upstream handling style,
and using cocci script (write_same__no_capable.cocci).

conclusion: under expectation, no need to change.


[6] wb_congested_enum__no_present

by default, the drbd-9.0 is using WB_async_congested & WB_sync_congested. For this
define ("wb_congested_enum__no_present") will trigger cocci script
drbd-kernel-compat/cocci/wb_congested_enum__no_present.cocci to apply wrong patch.

But bsc-1201335_06-bdi.patch already removed all WB_[async|sync]_congested code.
so wb_congested_enum__no_present won't make any problem.

conclusion: under expectation, no need to change.


[7] queue_discard_zeroes_data__no_present


drbd-kernel-compat/gen_patch_names.c
```
    patch(1, "queue_discard_zeroes_data", true, false,
          COMPAT_QUEUE_LIMITS_HAS_DISCARD_ZEROES_DATA, "present");
```

test code: drbd-kernel-compat/tests/queue_limits_has_discard_zeroes_data.c
```
#include <linux/blkdev.h>

struct queue_limits *foo(void)
{
    struct queue_limits *lim = NULL;

    lim->discard_zeroes_data = 1;

    return lim;
}
```

.compat_test.6.1.1/queue_limits_has_discard_zeroes_data.result
/* #undef COMPAT_QUEUE_LIMITS_HAS_DISCARD_ZEROES_DATA */

.compat_test.6.1.1/queue_limits_has_discard_zeroes_data.stderr
```
/usr/src/packages/BUILD/drbd-9.0.30~1+git.8e9c0812/default/drbd-kernel-compat/tests/queue_limits_has_discard_zeroes_data.c: In function ‘foo’:
/usr/src/packages/BUILD/drbd-9.0.30~1+git.8e9c0812/default/drbd-kernel-compat/tests/queue_limits_has_discard_zeroes_data.c:7:12: error: ‘struct queue_limits’ has no member named ‘discard_zeroes_data’
    7 |         lim->discard_zeroes_data = 1;
      |            ^~
```

drbd-kernel-compat/cocci/queue_discard_zeroes_data__no_present.cocci does the
correctly job to comment out all discard_zeroes_data places.

another info is drbd main branch introduce commit 41544c3f4e6e1687050468bf9f1da30345bcb601
to handle COMPAT_QUEUE_LIMITS_HAS_DISCARD_ZEROES_DATA case. but this commit is not
on drbd-9.0 branch. we could happily use existed code.

```
commit 41544c3f4e6e1687050468bf9f1da30345bcb601
Author: Christoph Böhmwalder <christoph.boehmwalder@linbit.com>
Date:   Wed Oct 12 10:09:06 2022 +0200

    compat: invert queue_discard_zeroes_data patch

    COMPAT_QUEUE_LIMITS_HAS_DISCARD_ZEROES_DATA is defined only on kernels
    <=3.10. This means that we (appear to) use the "old" API in the code
    base, and just patch it out with the compat layer for modern kernels.

    This is exactly the opposite way of how we want to be doing things.

    Instead -- even if it makes things a lot more complicated in the compat
    layer -- remove the old API from the code base and patch it back *in*
    using a compat patch
```

conclusion:  under expectation, no need to change.
Comment 5 heming zhao 2023-01-05 09:47:06 UTC
apply two upstream patches for enhancement drbd code.
- 2573d91be862ed65781c4a37dcaaa051a905a48a
  respond to: bsc-1206791-07-drbd-fix-use-after-free-bugs-in-get_initial_state.patch
- c2a620949ed609d6e256d1ce4d671a6da6bfeac0
  respond to: bsc-1206791-08-lib-lru_cache-Fixed-array-overflow-caused-by-incorre.patch

detail patches info

```
From 2573d91be862ed65781c4a37dcaaa051a905a48a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christoph=20B=C3=B6hmwalder?=
 <christoph.boehmwalder@linbit.com>
Date: Wed, 5 Oct 2022 13:36:18 +0200
Subject: [PATCH] drbd: fix use-after-free bugs in get_initial_state

See upstream commit aadb22ba2f65 ("drbd: Fix five use after free bugs
in get_initial_state").
```

```
commit c2a620949ed609d6e256d1ce4d671a6da6bfeac0
Author: John Sanpe <sanpeqf@gmail.com>
Date:   Sat Jul 23 09:59:31 2022 +0200

    lib/lru_cache: Fixed array overflow caused by incorrect boundary handling.
```
Comment 6 heming zhao 2023-01-05 13:03:34 UTC
build error
```
/usr/src/packages/BUILD/drbd-9.0.30~1+git.8e9c0812/default/drbd_dax_pmem.c: In function ‘drbd_dax_open’:
/usr/src/packages/BUILD/drbd-9.0.30~1+git.8e9c0812/default/drbd_dax_pmem.c:66:19: error: implicit declaration of function ‘dax_get_by_host’; did you mean ‘dax_remove_host’? [-Werror=implicit-function-declaration]
   66 |         dax_dev = dax_get_by_host(disk_name);
      |                   ^~~~~~~~~~~~~~~
      |                   dax_remove_host
```

partly use drbd-9.1 branch commit a68200c121de9e17a73a53962697fa32239e99f6
and drbd-9.0 branch commit 998a1faccbbb7e7b6d1042e7fe841734671ee365

conclusion: create patch bsc-1206791-09-pmem-use-fs_dax_get_by_bdev-instead-of-dax_get_by_ho.patch
Comment 7 heming zhao 2023-01-05 13:03:40 UTC
the final cocci file content:

```
  GENPATCHNAMES   6.1.1-1-default
  APPLIED_COCCI_FILES

bio_split_to_limits__yes_present
prandom_u32__no_present
genl_policy__yes_in_ops
write_same__no_capable
wb_congested_enum__no_present
queue_discard_zeroes_data__no_present
```
Comment 8 heming zhao 2023-01-10 01:09:31 UTC
opensuse factory had received request. close this bug.