From 43ef161fd9ad1c3ffc4ccd98b27f154a000b298b Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Fri, 13 Sep 2024 09:09:19 +0200 Subject: [PATCH 01/32] GitHub workflows: use {upload,download}-artifact@v4 The @v1 releases are deprecated. https://github.blog/changelog/2024-02-13-deprecation-notice-v1-and-v2-of-the-artifact-actions/ Signed-off-by: Martin Wilck --- .github/workflows/abi.yaml | 6 ++++-- .github/workflows/foreign.yaml | 15 +++++++-------- .github/workflows/native.yaml | 5 +++-- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/.github/workflows/abi.yaml b/.github/workflows/abi.yaml index 393322e56..dce09f5c5 100644 --- a/.github/workflows/abi.yaml +++ b/.github/workflows/abi.yaml @@ -45,10 +45,11 @@ jobs: - name: create ABI run: make -Orecurse -j$(grep -c ^processor /proc/cpuinfo) abi.tar.gz - name: save ABI - uses: actions/upload-artifact@v1 + uses: actions/upload-artifact@v4 with: name: abi path: abi + overwrite: true - name: compare ABI against reference id: compare continue-on-error: true @@ -56,10 +57,11 @@ jobs: run: make abi-test - name: save differences if: ${{ steps.compare.outcome == 'failure' }} - uses: actions/upload-artifact@v1 + uses: actions/upload-artifact@v4 with: name: abi-test path: abi-test + overwrite: true - name: fail # MUST use >- here, otherwise the condition always evaluates to true diff --git a/.github/workflows/foreign.yaml b/.github/workflows/foreign.yaml index 9e4d35e0f..d68650df2 100644 --- a/.github/workflows/foreign.yaml +++ b/.github/workflows/foreign.yaml @@ -36,14 +36,13 @@ jobs: - name: checkout uses: actions/checkout@v1 - name: build - run: make -j8 -Orecurse test-progs - - name: create binary archive - run: make test-progs.tar + run: make -j -Orecurse test-progs.tar - name: upload binary archive - uses: actions/upload-artifact@v1 + uses: actions/upload-artifact@v4 with: name: cross-${{ matrix.os }}-${{ matrix.arch }} path: test-progs.tar + overwrite: true test: runs-on: ubuntu-22.04 @@ -61,11 +60,11 @@ jobs: run: echo CONTAINER_ARCH="arm/v7" >> $GITHUB_ENV if: ${{ matrix.arch == 'armhf' }} - name: download binary archive - uses: actions/download-artifact@v1 + uses: actions/download-artifact@v4 with: name: cross-${{ matrix.os }}-${{ matrix.arch }} - name: unpack binary archive - run: tar xfv cross-${{ matrix.os }}-${{ matrix.arch }}/test-progs.tar + run: tar xfv test-progs.tar - name: enable foreign arch uses: dbhi/qus/action@main - name: run tests @@ -100,11 +99,11 @@ jobs: run: echo CONTAINER_ARCH="arm/v7" >> $GITHUB_ENV if: ${{ matrix.arch == 'armhf' }} - name: download binary archive - uses: actions/download-artifact@v1 + uses: actions/download-artifact@v4 with: name: cross-${{ matrix.os }}-${{ matrix.arch }} - name: unpack binary archive - run: tar xfv cross-${{ matrix.os }}-${{ matrix.arch }}/test-progs.tar + run: tar xfv test-progs.tar - name: enable foreign arch uses: dbhi/qus/action@main - name: run tests diff --git a/.github/workflows/native.yaml b/.github/workflows/native.yaml index 80ff6df56..500becaae 100644 --- a/.github/workflows/native.yaml +++ b/.github/workflows/native.yaml @@ -57,10 +57,11 @@ jobs: - name: create binary archive run: make ${{ env.ARCHIVE_TGT }} - name: upload binary archive - uses: actions/upload-artifact@v1 + uses: actions/upload-artifact@v4 with: name: native-${{ matrix.os }} path: ${{ env.ARCHIVE_TGT }} + overwrite: true - name: clean run: make clean @@ -98,7 +99,7 @@ jobs: uses: actions/checkout@v1 - name: download binary archive - uses: actions/download-artifact@v1 + uses: actions/download-artifact@v4 with: name: native-${{ matrix.os }} - name: unpack binary archive From 2e552e37a345dbfae661c286c3b3aac86b7242cf Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Fri, 13 Sep 2024 09:10:38 +0200 Subject: [PATCH 02/32] GitHub workflows: update dawidd6/action-download-artifact Signed-off-by: Martin Wilck --- .github/workflows/abi.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/abi.yaml b/.github/workflows/abi.yaml index dce09f5c5..d5f8b477e 100644 --- a/.github/workflows/abi.yaml +++ b/.github/workflows/abi.yaml @@ -28,7 +28,7 @@ jobs: - name: get reference ABI id: reference continue-on-error: true - uses: dawidd6/action-download-artifact@v2 + uses: dawidd6/action-download-artifact@v6 with: workflow: abi.yaml branch: ${{ env.ABI_BRANCH }} From 978f2cbc5866aa85762caef0e1c5a56dbfa64db9 Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Fri, 13 Sep 2024 09:45:25 +0200 Subject: [PATCH 03/32] Github workflows: native.yml: use mosteo-actions/docker-run We can't use "container:" any more because upload-artifact@v4 doesn't work on Debian Jessie. Signed-off-by: Martin Wilck --- .github/workflows/native.yaml | 45 +++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/.github/workflows/native.yaml b/.github/workflows/native.yaml index 500becaae..70d01c55b 100644 --- a/.github/workflows/native.yaml +++ b/.github/workflows/native.yaml @@ -35,17 +35,9 @@ jobs: - debian-bookworm - fedora-40 - opensuse-leap - container: ghcr.io/mwilck/multipath-build-${{ matrix.os }} steps: - name: checkout uses: actions/checkout@v1 - - name: build and test - if: ${{ matrix.os != 'debian-jessie' }} - run: make -j -Orecurse test - - name: build and test (jessie) - # On jessie, we use libreadline 5 (no licensing issue) - if: ${{ matrix.os == 'debian-jessie' }} - run: make -j -Orecurse READLINE=libreadline test - name: set archive name # Leap containers have cpio but not tar @@ -54,8 +46,21 @@ jobs: - name: set archive name run: echo ARCHIVE_TGT=test-progs.tar >> $GITHUB_ENV if: ${{ matrix.os != 'opensuse-leap' }} - - name: create binary archive - run: make ${{ env.ARCHIVE_TGT }} + + - name: build and test + if: ${{ matrix.os != 'debian-jessie' }} + uses: mosteo-actions/docker-run@v1 + with: + image: ghcr.io/mwilck/multipath-build-${{ matrix.os }} + command: -j -Orecurse ${{ env.ARCHIVE_TGT }} test + - name: build and test (jessie) + # On jessie, we use libreadline 5 (no licensing issue) + if: ${{ matrix.os == 'debian-jessie' }} + uses: mosteo-actions/docker-run@v1 + with: + image: ghcr.io/mwilck/multipath-build-${{ matrix.os }} + command: -j -Orecurse READLINE=libreadline ${{ env.ARCHIVE_TGT }} test + - name: upload binary archive uses: actions/upload-artifact@v4 with: @@ -67,14 +72,18 @@ jobs: run: make clean - name: clang if: ${{ matrix.os != 'debian-jessie' }} - env: - CC: clang - run: make -j -Orecurse test + uses: mosteo-actions/docker-run@v1 + with: + image: ghcr.io/mwilck/multipath-build-${{ matrix.os }} + params: -e CC=clang + command: -j -Orecurse test - name: clang (jessie) if: ${{ matrix.os == 'debian-jessie' }} - env: - CC: clang - run: make READLINE=libreadline test + uses: mosteo-actions/docker-run@v1 + with: + image: ghcr.io/mwilck/multipath-build-${{ matrix.os }} + params: -e CC=clang + command: -j -Orecurse READLINE=libreadline test root-test: runs-on: ubuntu-22.04 @@ -103,10 +112,10 @@ jobs: with: name: native-${{ matrix.os }} - name: unpack binary archive - run: cpio -idv < native-${{ matrix.os }}/test-progs.cpio + run: cpio -idv < test-progs.cpio if: ${{ matrix.os == 'opensuse-leap' }} - name: unpack binary archive - run: tar xfmv native-${{ matrix.os }}/test-progs.tar + run: tar xfmv test-progs.tar if: ${{ matrix.os != 'opensuse-leap' }} - name: run root tests From 1d44836884e5f8892dee8db92b4cc6011375439f Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Fri, 13 Sep 2024 10:17:17 +0200 Subject: [PATCH 04/32] GitHub workflows: native.yml: use extra job for clang Running "make" in a container and "make clean" outside doesn't work (access right issues). So just use separate jobs. Signed-off-by: Martin Wilck --- .github/workflows/native.yaml | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/.github/workflows/native.yaml b/.github/workflows/native.yaml index 70d01c55b..95b53b87f 100644 --- a/.github/workflows/native.yaml +++ b/.github/workflows/native.yaml @@ -68,8 +68,22 @@ jobs: path: ${{ env.ARCHIVE_TGT }} overwrite: true - - name: clean - run: make clean + clang: + runs-on: ubuntu-22.04 + strategy: + fail-fast: false + matrix: + os: + - debian-jessie + - debian-buster + - debian-bullseye + - debian-bookworm + - fedora-40 + - opensuse-leap + steps: + - name: checkout + uses: actions/checkout@v1 + - name: clang if: ${{ matrix.os != 'debian-jessie' }} uses: mosteo-actions/docker-run@v1 From 01ced67e9b6c3583725afbfa190c1412d173ccfc Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Fri, 13 Sep 2024 10:30:47 +0200 Subject: [PATCH 05/32] GitHub workflows: native.yaml: make test and archive separately Avoid "text file busy" error on GitHub. dmevents-test: Text file busy Makefile:74: recipe for target 'dmevents.out' failed Signed-off-by: Martin Wilck --- .github/workflows/native.yaml | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/.github/workflows/native.yaml b/.github/workflows/native.yaml index 95b53b87f..f06b09df0 100644 --- a/.github/workflows/native.yaml +++ b/.github/workflows/native.yaml @@ -52,14 +52,27 @@ jobs: uses: mosteo-actions/docker-run@v1 with: image: ghcr.io/mwilck/multipath-build-${{ matrix.os }} - command: -j -Orecurse ${{ env.ARCHIVE_TGT }} test + command: -j -Orecurse test - name: build and test (jessie) # On jessie, we use libreadline 5 (no licensing issue) if: ${{ matrix.os == 'debian-jessie' }} uses: mosteo-actions/docker-run@v1 with: image: ghcr.io/mwilck/multipath-build-${{ matrix.os }} - command: -j -Orecurse READLINE=libreadline ${{ env.ARCHIVE_TGT }} test + command: -j -Orecurse READLINE=libreadline test + + - name: create ${{ env.ARCHIVE_TGT }} + if: ${{ matrix.os != 'debian-jessie' }} + uses: mosteo-actions/docker-run@v1 + with: + image: ghcr.io/mwilck/multipath-build-${{ matrix.os }} + command: ${{ env.ARCHIVE_TGT }} + - name: create ${{ env.ARCHIVE_TGT }} (jessie) + if: ${{ matrix.os == 'debian-jessie' }} + uses: mosteo-actions/docker-run@v1 + with: + image: ghcr.io/mwilck/multipath-build-${{ matrix.os }} + command: READLINE=libreadline ${{ env.ARCHIVE_TGT }} - name: upload binary archive uses: actions/upload-artifact@v4 From 6c6330d442667813bb3d581509602e5901d4da9b Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Thu, 14 Nov 2024 16:02:38 +0100 Subject: [PATCH 06/32] GitHub workflows: enable unit tests for stable branches Signed-off-by: Martin Wilck --- .github/workflows/build-and-unittest.yaml | 2 ++ .github/workflows/foreign.yaml | 2 ++ .github/workflows/multiarch-stable.yaml | 2 ++ .github/workflows/multiarch.yaml | 2 ++ .github/workflows/native.yaml | 2 ++ .github/workflows/rolling.yaml | 2 ++ 6 files changed, 12 insertions(+) diff --git a/.github/workflows/build-and-unittest.yaml b/.github/workflows/build-and-unittest.yaml index a838f9a2c..1a727e141 100644 --- a/.github/workflows/build-and-unittest.yaml +++ b/.github/workflows/build-and-unittest.yaml @@ -5,10 +5,12 @@ on: - master - queue - tip + - 'stable-*' pull_request: branches: - master - queue + - 'stable-*' jobs: jammy: runs-on: ubuntu-22.04 diff --git a/.github/workflows/foreign.yaml b/.github/workflows/foreign.yaml index d68650df2..0f80957b9 100644 --- a/.github/workflows/foreign.yaml +++ b/.github/workflows/foreign.yaml @@ -5,6 +5,7 @@ on: - master - queue - tip + - 'stable-*' paths: - '.github/workflows/foreign.yaml' - '**.h' @@ -15,6 +16,7 @@ on: branches: - master - queue + - 'stable-*' paths: - '.github/workflows/foreign.yaml' - '**.h' diff --git a/.github/workflows/multiarch-stable.yaml b/.github/workflows/multiarch-stable.yaml index e51d383ce..ffede53de 100644 --- a/.github/workflows/multiarch-stable.yaml +++ b/.github/workflows/multiarch-stable.yaml @@ -5,6 +5,7 @@ on: - master - queue - tip + - 'stable-*' paths: - '.github/workflows/multiarch-stable.yaml' - '**.h' @@ -15,6 +16,7 @@ on: branches: - master - queue + - 'stable-*' paths: - '.github/workflows/multiarch-stable.yaml' - '**.h' diff --git a/.github/workflows/multiarch.yaml b/.github/workflows/multiarch.yaml index df95a02fb..d2b833a4c 100644 --- a/.github/workflows/multiarch.yaml +++ b/.github/workflows/multiarch.yaml @@ -5,6 +5,7 @@ on: - master - queue - tip + - 'stable-*' paths: - '.github/workflows/multiarch.yaml' - '**.h' @@ -15,6 +16,7 @@ on: branches: - master - queue + - 'stable-*' paths: - '.github/workflows/multiarch.yaml' - '**.h' diff --git a/.github/workflows/native.yaml b/.github/workflows/native.yaml index f06b09df0..c9d9df9e6 100644 --- a/.github/workflows/native.yaml +++ b/.github/workflows/native.yaml @@ -5,6 +5,7 @@ on: - master - queue - tip + - 'stable-*' paths: - '.github/workflows/native.yaml' - '**.h' @@ -15,6 +16,7 @@ on: branches: - master - queue + - 'stable-*' paths: - '.github/workflows/native.yaml' - '**.h' diff --git a/.github/workflows/rolling.yaml b/.github/workflows/rolling.yaml index 3536b9446..66af7a447 100644 --- a/.github/workflows/rolling.yaml +++ b/.github/workflows/rolling.yaml @@ -5,6 +5,7 @@ on: - master - queue - tip + - 'stable-*' paths: - '.github/workflows/rolling.yaml' - '**.h' @@ -15,6 +16,7 @@ on: branches: - master - queue + - 'stable-*' paths: - '.github/workflows/rolling.yaml' - '**.h' From c90c971599d82b783c361afff3a1249296c7c4f0 Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Thu, 14 Nov 2024 16:51:05 +0100 Subject: [PATCH 07/32] GitHub Workflows: add abi check for stable branches The ABI should never change on a stable branch. This workflow asserts that. Signed-off-by: Martin Wilck --- .github/workflows/abi-stable.yaml | 89 +++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 .github/workflows/abi-stable.yaml diff --git a/.github/workflows/abi-stable.yaml b/.github/workflows/abi-stable.yaml new file mode 100644 index 000000000..a6746f233 --- /dev/null +++ b/.github/workflows/abi-stable.yaml @@ -0,0 +1,89 @@ +name: check-abi for stable branch +on: + push: + branches: + - 'stable-*' + paths: + - '.github/workflows/abi-stable.yaml' + - '**.h' + - '**.c' + - '**.version' + pull_request: + branches: + - 'stable-*' + workflow_dispatch: + +jobs: + reference-abi: + runs-on: ubuntu-20.04 + steps: + - name: get parent tag + run: > + echo ${{ github.ref }} | + sed -E 's,refs/heads/stable-([0-9]\.[0-9]*)\.y,PARENT_TAG=\1.0,' >> $GITHUB_ENV + - name: assert parent tag + run: /bin/false + if: ${{ env.PARENT_TAG == '' }} + - name: update + run: sudo apt-get update + - name: dependencies + run: > + sudo apt-get install --yes gcc + gcc make pkg-config abigail-tools + libdevmapper-dev libreadline-dev libaio-dev libsystemd-dev + libudev-dev libjson-c-dev liburcu-dev libcmocka-dev libedit-dev + - name: checkout ${{ env.PARENT_TAG }} + uses: actions/checkout@v4 + with: + ref: ${{ env.PARENT_TAG }} + - name: build ABI for ${{ env.PARENT_TAG }} + run: make -j$(grep -c ^processor /proc/cpuinfo) -Orecurse abi + - name: save ABI + uses: actions/upload-artifact@v4 + with: + name: multipath-abi-${{ env.PARENT_TAG }} + path: abi + + check-abi: + runs-on: ubuntu-20.04 + needs: reference-abi + steps: + - name: get parent tag + run: > + echo ${{ github.ref }} | + sed -E 's,refs/heads/stable-([0-9]\.[0-9]*)\.y,PARENT_TAG=\1.0,' >> $GITHUB_ENV + - name: assert parent tag + run: /bin/false + if: ${{ env.PARENT_TAG == '' }} + - name: checkout ${{ env.PARENT_TAG }} + uses: actions/checkout@v4 + with: + ref: ${{ env.PARENT_TAG }} + - name: download ABI for ${{ env.PARENT_TAG }} + id: download_abi + uses: actions/download-artifact@v4 + with: + name: multipath-abi-${{ env.PARENT_TAG }} + path: reference-abi + - name: update + run: sudo apt-get update + if: steps.download_abi.outcome != 'success' + - name: dependencies + run: > + sudo apt-get install --yes gcc + gcc make pkg-config abigail-tools + libdevmapper-dev libreadline-dev libaio-dev libsystemd-dev + libudev-dev libjson-c-dev liburcu-dev libcmocka-dev libedit-dev + - name: check ABI of ${{ github.ref }} against ${{ env.PARENT_TAG }} + id: check_abi + run: make -j$(grep -c ^processor /proc/cpuinfo) -Orecurse abi-test + continue-on-error: true + - name: save differences + if: ${{ steps.check_abi.outcome != 'success' }} + uses: actions/upload-artifact@v4 + with: + name: abi-test + path: abi-test + - name: fail + run: /bin/false + if: steps.check_abi.outcome != 'success' From e88648005381a9feba3abcdd1291efd6daf721a1 Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Tue, 12 Nov 2024 13:06:10 +0100 Subject: [PATCH 08/32] libmultipath: dm_get_maps(): don't bail out for single-map failures dm_get_maps() traverses the entire list of dm maps. We shouldn't give up just because probing a single map failed. Fixes: bf3a4ad ("libmultipath: simplify dm_get_maps()") Signed-off-by: Martin Wilck --- libmultipath/devmapper.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c index c497c2250..52bfe9cec 100644 --- a/libmultipath/devmapper.c +++ b/libmultipath/devmapper.c @@ -1262,10 +1262,8 @@ int dm_get_maps(vector mp) } vector_set_slot(mp, mpp); break; - case DMP_NO_MATCH: - break; default: - return 1; + break; } next = names->next; names = (void *) names + next; From 1a9ad294c97da0ae31153f5747bdbdb8f084df10 Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Thu, 31 Oct 2024 12:08:08 +0100 Subject: [PATCH 09/32] 11-dm-mpath.rules.in: import DM_COLDPLUG_SUSPENDED only once We import DM_COLDPLUG_SUSPENDED in all code flows below mpath_coldplug_end. Clarify this in the code. Signed-off-by: Martin Wilck Reviewed-by: Benjamin Marzinski --- multipath/11-dm-mpath.rules.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-mpath.rules.in index 30647b99a..67838261f 100644 --- a/multipath/11-dm-mpath.rules.in +++ b/multipath/11-dm-mpath.rules.in @@ -24,12 +24,13 @@ ENV{DM_UDEV_RULES_VSN}=="1|2", ENV{.DM_SUSPENDED}!="1", ENV{DISK_RO}!="1", \ ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="", GOTO="scan_import" LABEL="mpath_coldplug_end" +IMPORT{db}="DM_COLDPLUG_SUSPENDED" + # If this uevent didn't come from dm, don't try to update the # device state # Note that .MPATH_DEVICE_READY_OLD=="" here. Thus we won't activate the # device below at mpath_is_ready, which is correct. ENV{DM_COOKIE}!="?*", ENV{DM_ACTION}!="PATH_*", \ - IMPORT{db}="DM_COLDPLUG_SUSPENDED", \ GOTO="check_mpath_ready" ENV{.MPATH_DEVICE_READY_OLD}="$env{MPATH_DEVICE_READY}" @@ -67,7 +68,6 @@ LABEL="check_mpath_unchanged" # A previous coldplug event occurred while the device was suspended. # Activation might have been partially skipped. Activate the device now, # i.e. disable the MPATH_UNCHANGED logic and set DM_ACTIVATION=1. -IMPORT{db}="DM_COLDPLUG_SUSPENDED" ENV{DM_COLDPLUG_SUSPENDED}=="1", ENV{.DM_SUSPENDED}!="1", \ ENV{DM_ACTIVATION}="1", ENV{MPATH_UNCHANGED}="0", \ PROGRAM="@SYSDIR_BIN@/logger -t 11-dm-mpath.rules -p daemon.notice \"Forcing activation of previously suspended device\"", \ From ce38572451eb1b98a8ab72b2c421282a20b9d310 Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Thu, 31 Oct 2024 12:59:03 +0100 Subject: [PATCH 10/32] 11-dm-mpath.rules.in: handle inactive suspended devices correctly Since b22c273 ("11-dm-mpath.rules: Don't force activation while device is suspended"), we've handled the case where a device is suspended while an uevent is processed (e.g. because multipathd is reloading the map again at the same time). But we were missing the case where The device had never been initialized before. If .MPATH_DEVICE_READY_OLD was empty, we'd jump to scan_import without setting MPATH_DEVICE_READY to 0. This can cause a device not to be fully activated at boot time, because in follow-up uevents we'd assume that the device had already been set up. Treat the case in which an uevent is processed for a previously not fully set-up, suspended device like other situations where we set MPATH_DEVICE_READY to 0. Fixes: b22c273 ("11-dm-mpath.rules: Don't force activation while device is suspended") Reviewed-by: Benjamin Marzinski --- multipath/11-dm-mpath.rules.in | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-mpath.rules.in index 67838261f..20f8c6ac0 100644 --- a/multipath/11-dm-mpath.rules.in +++ b/multipath/11-dm-mpath.rules.in @@ -35,6 +35,13 @@ ENV{DM_COOKIE}!="?*", ENV{DM_ACTION}!="PATH_*", \ ENV{.MPATH_DEVICE_READY_OLD}="$env{MPATH_DEVICE_READY}" +# If the device wasn't ready previously and is currently suspended, +# we have to postpone the activation until the next event. +# In this case, we have to set MPATH_DEVICE_READY=0; otherwise, the +# MPATH_UNCHANGED logic will cause later rules to skipped in the next event. +ENV{.MPATH_DEVICE_READY_OLD}!="1", ENV{.DM_SUSPENDED}=="1", \ + ENV{MPATH_DEVICE_READY}="0", GOTO="check_mpath_unchanged" + # multipath sets DM_SUBSYSTEM_UDEV_FLAG2 when it reloads a # table with no active devices. If this happens, mark the # device not ready @@ -106,14 +113,10 @@ GOTO="scan_import" LABEL="mpath_is_ready" # If the device comes back online, set DM_ACTIVATION so that -# upper layers do a rescan. If the device is currently suspended, -# we have to postpone the activation until the next event. -# In this case, we have to set MPATH_DEVICE_READY=0; otherwise, the -# MPATH_UNCHANGED logic will cause later rules to skipped in the next event. -ENV{.MPATH_DEVICE_READY_OLD}!="0", GOTO="scan_import" -ENV{.DM_SUSPENDED}=="1", ENV{MPATH_DEVICE_READY}="0", GOTO="scan_import" - -ENV{DM_ACTIVATION}="1", ENV{MPATH_UNCHANGED}="0" +# upper layers will do a rescan. Don't do this if .MPATH_DEVICE_READY_OLD +# is just empty (see comment above the DM_COOKIE test above). +ENV{.MPATH_DEVICE_READY_OLD}=="0", \ + ENV{DM_ACTIVATION}="1", ENV{MPATH_UNCHANGED}="0" # The code to check multipath state ends here. We need to set # properties and symlinks regardless whether the map is usable or From 1183dc5beb89c8e519b25067ffb87223f1410df4 Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Thu, 31 Oct 2024 13:11:21 +0100 Subject: [PATCH 11/32] 11-dm-mpath.rules.in: clarify DM_ACTIVATION logic Our code is always setting MPATH_UNCHANGED and DM_ACTIVATION in pairs. While DM_ACTIVATION is a global DM property, MPATH_UNCHANGED is owned by us. Just set MPATH_UNCHANGED, and adapt DM_ACTIVATION when necessary just in one place. Signed-off-by: Martin Wilck Reviewed-by: Benjamin Marzinski --- multipath/11-dm-mpath.rules.in | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-mpath.rules.in index 20f8c6ac0..a2655cb2f 100644 --- a/multipath/11-dm-mpath.rules.in +++ b/multipath/11-dm-mpath.rules.in @@ -74,25 +74,25 @@ LABEL="check_mpath_unchanged" # A previous coldplug event occurred while the device was suspended. # Activation might have been partially skipped. Activate the device now, -# i.e. disable the MPATH_UNCHANGED logic and set DM_ACTIVATION=1. +# i.e. disable the MPATH_UNCHANGED logic. ENV{DM_COLDPLUG_SUSPENDED}=="1", ENV{.DM_SUSPENDED}!="1", \ - ENV{DM_ACTIVATION}="1", ENV{MPATH_UNCHANGED}="0", \ + ENV{MPATH_UNCHANGED}="0", \ PROGRAM="@SYSDIR_BIN@/logger -t 11-dm-mpath.rules -p daemon.notice \"Forcing activation of previously suspended device\"", \ GOTO="check_mpath_ready" # DM_SUBSYSTEM_UDEV_FLAG0 is the "RELOAD" flag for multipath subsystem. -# Drop the DM_ACTIVATION flag here as mpath reloads tables if any of its +# Set the MPATH_UNCHANGED flag here as mpath reloads tables if any of its # paths are lost/recovered. For any stack above the mpath device, this is not # something that should be reacted upon since it would be useless extra work. # It's exactly mpath's job to provide *seamless* device access to any of the # paths that are available underneath. ENV{DM_SUBSYSTEM_UDEV_FLAG0}=="1", \ - ENV{DM_ACTIVATION}="0", ENV{MPATH_UNCHANGED}="1" + ENV{MPATH_UNCHANGED}="1" -# For path failed or reinstated events, unset DM_ACTIVATION. +# For path failed or reinstated events, set MPATH_UNCHANGED. # This is similar to the DM_SUBSYSTEM_UDEV_FLAG0 case above. ENV{DM_ACTION}=="PATH_FAILED|PATH_REINSTATED", \ - ENV{DM_ACTIVATION}="0", ENV{MPATH_UNCHANGED}="1" + ENV{MPATH_UNCHANGED}="1" LABEL="check_mpath_ready" @@ -112,11 +112,10 @@ GOTO="scan_import" LABEL="mpath_is_ready" -# If the device comes back online, set DM_ACTIVATION so that +# If the device comes back online, clear MPATH_UNCHANGED so that # upper layers will do a rescan. Don't do this if .MPATH_DEVICE_READY_OLD # is just empty (see comment above the DM_COOKIE test above). -ENV{.MPATH_DEVICE_READY_OLD}=="0", \ - ENV{DM_ACTIVATION}="1", ENV{MPATH_UNCHANGED}="0" +ENV{.MPATH_DEVICE_READY_OLD}=="0", ENV{MPATH_UNCHANGED}="0" # The code to check multipath state ends here. We need to set # properties and symlinks regardless whether the map is usable or @@ -146,6 +145,10 @@ IMPORT{db}="ID_PART_GPT_AUTO_ROOT" LABEL="import_end" +# If MPATH_UNCHANGED is set, adapt DM_ACTIVATION. +ENV{MPATH_UNCHANGED}=="0", ENV{DM_ACTIVATION}="1" +ENV{MPATH_UNCHANGED}=="1", ENV{DM_ACTIVATION}="0" + # Reset previous DM_COLDPLUG_SUSPENDED if activation happens now ENV{.DM_SUSPENDED}!="1", ENV{DM_ACTIVATION}=="1", ENV{DM_COLDPLUG_SUSPENDED}="" From be50c2fe511301506f18ca249521cd1beea23881 Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Sun, 3 Nov 2024 23:04:08 +0100 Subject: [PATCH 12/32] 11-dm-mpath-rules.in: skip one .DM_NOSCAN check We set .DM_NOSCAN above where we set DM_UDEV_DISABLE_OTHER_RULES_FLAG, too. If the latter isn't set, .DM_NOSCAN can't be set. Skip the redundant test. Signed-off-by: Martin Wilck Reviewed-by: Benjamin Marzinski --- multipath/11-dm-mpath.rules.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-mpath.rules.in index a2655cb2f..79227bec1 100644 --- a/multipath/11-dm-mpath.rules.in +++ b/multipath/11-dm-mpath.rules.in @@ -132,7 +132,7 @@ ENV{DM_UDEV_PRIMARY_SOURCE_FLAG}!="1", GOTO="import_end" ENV{DM_UDEV_RULES_VSN}!="1|2", GOTO="import_end" # Don't import the properties from db if we will run blkid later. -ENV{.DM_NOSCAN}!="1", ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}!="1", GOTO="import_end" +ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}!="1", GOTO="import_end" IMPORT{db}="ID_FS_TYPE" IMPORT{db}="ID_FS_USAGE" From 1548380458932824909b4c1e02c24f68fa2789f2 Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Sun, 3 Nov 2024 23:07:23 +0100 Subject: [PATCH 13/32] 11-dm-mpath.rules.in: set .DM_NOSCAN if MPATH_UNCHANGED is set When multipath reloads a device or fails or restores a path, the udev rules disable LVM scanning, but since .DM_NOSCAN isn't set, blkid is still run on the device. When multipath devices that are set to queue_if_no_path lose all their paths at close to the same time, udev workers can hang trying to run blkid. The blkid results shouldn't change when multipathd is adding, removing, failing or reinstating paths, aside from avoiding hanging udev processes, we're skipping unnecessary work. Hence, set .DM_NOSCAN if MPATH_UNCHANGED is set, to avoid blkid from being called in 13-dm.rules. Suggested-by: Benjamin Marzinski Signed-off-by: Martin Wilck Reviewed-by: Benjamin Marzinski --- multipath/11-dm-mpath.rules.in | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-mpath.rules.in index 79227bec1..a816edbf4 100644 --- a/multipath/11-dm-mpath.rules.in +++ b/multipath/11-dm-mpath.rules.in @@ -145,9 +145,11 @@ IMPORT{db}="ID_PART_GPT_AUTO_ROOT" LABEL="import_end" -# If MPATH_UNCHANGED is set, adapt DM_ACTIVATION. +# If MPATH_UNCHANGED is set, adapt DM_ACTIVATION and DM_NOSCAN. +# .DM_NOSCAN controls whether blkid will be run in 13-dm-disk.rules; +# we don't want to do that if MPATH_UNCHANGED is 1. ENV{MPATH_UNCHANGED}=="0", ENV{DM_ACTIVATION}="1" -ENV{MPATH_UNCHANGED}=="1", ENV{DM_ACTIVATION}="0" +ENV{MPATH_UNCHANGED}=="1", ENV{DM_ACTIVATION}="0", ENV{.DM_NOSCAN}="1" # Reset previous DM_COLDPLUG_SUSPENDED if activation happens now ENV{.DM_SUSPENDED}!="1", ENV{DM_ACTIVATION}=="1", ENV{DM_COLDPLUG_SUSPENDED}="" From d40acc48b3717c71ba6a98aa3d9afbc63f74c750 Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Thu, 7 Nov 2024 22:07:03 +0100 Subject: [PATCH 14/32] libmultipath: don't set dev_loss_tmo to 0 for NO_PATH_RETRY_FAIL If pp->dev_loss is DEV_LOSS_TMO_UNSET and min_dev_loss is 0 (which is the case if no_path_retry is NO_PATH_RETRY_FAIL or NO_PATH_RETRY_UNDEF), we will set pp->dev_loss to 0, which is wrong. Fix it. Fixes: 058b5f5 ("libmultipath: fix dev_loss_tmo even if not set in configuration") Signed-off-by: Martin Wilck Reviewed-by: Benjamin Marzinski --- libmultipath/discovery.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c index e94705bf2..5043330cc 100644 --- a/libmultipath/discovery.c +++ b/libmultipath/discovery.c @@ -942,7 +942,7 @@ sysfs_set_scsi_tmo (struct config *conf, struct multipath *mpp) continue; } - if (pp->dev_loss == DEV_LOSS_TMO_UNSET) + if (pp->dev_loss == DEV_LOSS_TMO_UNSET && min_dev_loss != 0) pp->dev_loss = min_dev_loss; else if (pp->dev_loss < min_dev_loss) { pp->dev_loss = min_dev_loss; From 5ca938ad5f481a07a7bf8d8b394b878b7b4c854c Mon Sep 17 00:00:00 2001 From: Benjamin Marzinski Date: Mon, 14 Oct 2024 23:28:30 -0400 Subject: [PATCH 15/32] multipathd: fix deferred_failback_tick for reload removes If reload_and_sync_map() removes the multipath device, deferred_failback_tick() needs to decrement the counter so that it doesn't skip the following device. Reviewed-by: Martin Wilck Signed-off-by: Benjamin Marzinski --- multipathd/main.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/multipathd/main.c b/multipathd/main.c index 1b7fd04f1..e4ef9a1db 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -2080,9 +2080,12 @@ deferred_failback_tick (struct vectors *vecs) if (!mpp->failback_tick && need_switch_pathgroup(mpp, &need_reload)) { - if (need_reload) - reload_and_sync_map(mpp, vecs); - else + if (need_reload) { + if (reload_and_sync_map(mpp, vecs) == 2) { + /* multipath device removed */ + i--; + } + } else switch_pathgroup(mpp); } } From 444530dab166272591dbdd626cf64788f2e1b227 Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Wed, 13 Nov 2024 16:19:49 +0100 Subject: [PATCH 16/32] multipathd: fix an unsigned int ovwerflow Reported by coverity: "i--" may cause an underflow, which will again cause an overflow when the loop continues. Use a signed int for loops like this to make coverity happy. Signed-off-by: Martin Wilck --- multipathd/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/multipathd/main.c b/multipathd/main.c index e4ef9a1db..3fb623fdc 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -2068,7 +2068,7 @@ static void deferred_failback_tick (struct vectors *vecs) { struct multipath * mpp; - unsigned int i; + int i; bool need_reload; vector_foreach_slot (vecs->mpvec, mpp, i) { From 62f5fac443dda5df2ccef676128cf177d2ca37ce Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Wed, 6 Nov 2024 22:17:16 +0100 Subject: [PATCH 17/32] libmpathutil: avoid -Wcast-function-type-mismatch error with clang 19 Avoid the following error with clang 19: msort.c:268:27: error: cast from '__compar_fn_t' (aka 'int (*)(const void *, const void *)') to '__compar_d_fn_t' (aka 'int (*)(const void *, const void *, void *)') converts to incompatible function type [-Werror,-Wcast-function-type-mismatch] 268 | return msort_r (b, n, s, (__compar_d_fn_t)cmp, NULL); | ^~~~~~~~~~~~~~~~~~~~ Signed-off-by: Martin Wilck --- libmpathutil/msort.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/libmpathutil/msort.c b/libmpathutil/msort.c index 50f799d91..9df7b267d 100644 --- a/libmpathutil/msort.c +++ b/libmpathutil/msort.c @@ -259,9 +259,12 @@ msort_r (void *b, size_t n, size_t s, __compar_d_fn_t cmp, void *arg) * If this is safe for them, it should be for us, too. */ #pragma GCC diagnostic push -#if __GNUC__ >= 8 +#if __GNUC__ >= 8 || __clang_major__ >= 19 #pragma GCC diagnostic ignored "-Wcast-function-type" #endif +#if __clang_major__ >= 19 +#pragma GCC diagnostic ignored "-Wcast-function-type-mismatch" +#endif void msort (void *b, size_t n, size_t s, __compar_fn_t cmp) { From c994e8482d79ae1974d72968421aabfbf5a9b625 Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Thu, 14 Nov 2024 17:59:14 +0100 Subject: [PATCH 18/32] Update NEWS.md for 0.10.1 Signed-off-by: Martin Wilck --- NEWS.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/NEWS.md b/NEWS.md index 69acda47c..b740efb1e 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,28 @@ # multipath-tools Release Notes +## multipath-tools 0.10.1, 2024/11 + +This is the first bug fix release on the `stable-0.10.y` branch. It contains +bug fixes from 0.11.0, and some CI-related fixes. + +### Bug fixes + +* Fixed the problem that multipathd wouldn't start on systems with certain types + of device mapper devices, in particular devices with multiple DM targets. + The problem was introduced in 0.10.0. + Fixes [#102](https://github.com/opensvc/multipath-tools/issues/102). +* Fixed a corner case in the udev rules which could cause a device not to be + activated during boot if a cold plug uevent is processed for a previously + not configured multipath map while this map was suspended. This problem existed + since 0.9.8. +* Fixed the problem that devices with `no_path_retry fail` and no setting + for `dev_loss_tmo` might get the `dev_loss_tmo` set to 0, causing the + device to be deleted immediately in the event of a transport disruption. + This bug was introduced in 0.9.6. +* Fixed the problem that, if there were multiple maps with deferred failback + (`failback` value > 0 in `multipath.conf`), some maps might fail back later + than configured. The problem existed since 0.9.6. + ## multipath-tools 0.10.0, 2024/08 ### User-Visible Changes From 84a4dccb18929038ac351b6864bff20ed95776e9 Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Thu, 14 Nov 2024 15:20:24 +0100 Subject: [PATCH 19/32] libmultipath: don't print error message if WATCHDOG_USEC is 0 WATCHDOG_USEC may be set to 0, which means that the watchdog is disabled in systemd. Fixes: 9366cfb ("multipathd: Implement systemd watchdog integration") Signed-off-by: Martin Wilck Reviewed-by: Benjamin Marzinski --- libmultipath/config.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libmultipath/config.c b/libmultipath/config.c index 0e3a5cc18..226ddecbe 100644 --- a/libmultipath/config.c +++ b/libmultipath/config.c @@ -865,6 +865,9 @@ static void set_max_checkint_from_watchdog(struct config *conf) unsigned long checkint; if (envp && sscanf(envp, "%lu", &checkint) == 1) { + if (checkint == 0) + /* watchdog disabled */ + return; /* Value is in microseconds */ checkint /= 1000000; if (checkint < 1 || checkint > UINT_MAX) { From b2642d2846111a01ab485fac677a4f443e3bda3c Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Mon, 25 Nov 2024 18:01:32 +0100 Subject: [PATCH 20/32] libmultipath: reduce log level of "map X has multiple targets" On systems with LVM volumes, "multipath -ll" will spit out lots of messages like libmp_mapinfo: map vg-lv0 has multiple targets which is irritating. Reduce the log level of these messages to 3, as they are harmless most of the time. This is a backport of e8949c2 ("libmultipath: reduce log level of libmp_mapinfo() messages") from the master branch. We can't apply exactly the same fix because the stable branch is missing 8c772d3 ("libmultipath: check DM UUID earlier in libmp_mapinfo__"). Signed-off-by: Martin Wilck --- libmultipath/devmapper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c index 52bfe9cec..fe5637b3f 100644 --- a/libmultipath/devmapper.c +++ b/libmultipath/devmapper.c @@ -718,7 +718,7 @@ static int libmp_mapinfo__(int flags, mapid_t id, mapinfo_t info, const char *ma if (info.target || info.status || info.size || flags & MAPINFO_TGT_TYPE__) { if (dm_get_next_target(dmt, NULL, &start, &length, &target_type, ¶ms) != NULL) { - condlog(2, "%s: map %s has multiple targets", fname__, map_id); + condlog(3, "%s: map %s has multiple targets", fname__, map_id); return DMP_NOT_FOUND; } if (!params) { From 3dc6a89ce2bf026baca81cae81e18933fdc400f7 Mon Sep 17 00:00:00 2001 From: Benjamin Marzinski Date: Wed, 8 Jan 2025 19:06:53 -0500 Subject: [PATCH 21/32] libmultipath/foreign: fix memory leak in nvme foreign handler _find_controllers() needs to free the udev device if it doesn't get added to a path. Otherwise it can leak memory whenever check_foreign() is called, causing multipathd's memory usage to continually grow. Fixes: 7b47762 ("libmultipath: nvme: fix path detection for kernel 4.16") Signed-off-by: Benjamin Marzinski Reviewed-by: Martin Wilck (cherry picked from commit 5a3d334e416a4a35ee88d7b8f1433ff7f57923ad) --- libmultipath/foreign/nvme.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/libmultipath/foreign/nvme.c b/libmultipath/foreign/nvme.c index 0b7f4eabd..cde660ced 100644 --- a/libmultipath/foreign/nvme.c +++ b/libmultipath/foreign/nvme.c @@ -675,7 +675,8 @@ static void _find_controllers(struct context *ctx, struct nvme_map *map) pthread_cleanup_push_cast(free_scandir_result, &sr); for (i = 0; i < r; i++) { char *fn = di[i]->d_name; - struct udev_device *ctrl, *udev; + struct udev_device *ctrl; + struct udev_device *udev __attribute__((cleanup(cleanup_udev_device))) = NULL; if (safe_snprintf(pathbuf + n, sizeof(pathbuf) - n, "/%s", fn)) continue; @@ -719,11 +720,11 @@ static void _find_controllers(struct context *ctx, struct nvme_map *map) continue; path->gen.ops = &nvme_path_ops; - path->udev = udev; + path->udev = steal_ptr(udev); path->seen = true; path->map = map; path->ctl = udev_device_get_parent_with_subsystem_devtype - (udev, "nvme", NULL); + (path->udev, "nvme", NULL); if (path->ctl == NULL) { condlog(1, "%s: %s: failed to get controller for %s", __func__, THIS, fn); @@ -744,7 +745,7 @@ static void _find_controllers(struct context *ctx, struct nvme_map *map) } vector_set_slot(&map->pgvec, &path->pg); condlog(3, "%s: %s: new path %s added to %s", - __func__, THIS, udev_device_get_sysname(udev), + __func__, THIS, udev_device_get_sysname(path->udev), udev_device_get_sysname(map->udev)); } pthread_cleanup_pop(1); From b3d82d8a6f470c873371c9910d1897017a6c595a Mon Sep 17 00:00:00 2001 From: chenrenhui Date: Fri, 10 Jan 2025 14:38:16 +0800 Subject: [PATCH 22/32] libmultipath: add condition for enqueueing path to io error check In function io_err_stat_handle_pathfail(), path->io_err_dis_reinstate_time is set to 0 to enqueue path to io error check as soon as possible. But multipathd can not do it within marginal_path_err_recheck_gap_time seconds after power-on, because curr_time is less than marginal_path_err_recheck_gap_time. To handle the early marginal path, we can enqueue path when io_err_dis_reinstate_time is 0. Signed-off-by: chenrenhui Reviewed-by: Benjamin Marzinski Reviewed-by: Martin Wilck > (cherry picked from commit a1e3cf2d42cc4bab10753e466c8adb7efa83c99e) --- libmultipath/io_err_stat.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libmultipath/io_err_stat.c b/libmultipath/io_err_stat.c index 4996c0b04..879c310a1 100644 --- a/libmultipath/io_err_stat.c +++ b/libmultipath/io_err_stat.c @@ -367,7 +367,8 @@ int need_io_err_check(struct path *pp) return 1; get_monotonic_time(&curr_time); if ((curr_time.tv_sec - pp->io_err_dis_reinstate_time) > - pp->mpp->marginal_path_err_recheck_gap_time) { + pp->mpp->marginal_path_err_recheck_gap_time || + pp->io_err_dis_reinstate_time == 0) { io_err_stat_log(4, "%s: reschedule checking after %d seconds", pp->dev, pp->mpp->marginal_path_err_recheck_gap_time); From f793a9e63d5b39dc3e03644c0234050b8639aed4 Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Fri, 17 Jan 2025 22:38:26 +0100 Subject: [PATCH 23/32] Additional NEWS.md updates for 0.10.1 --- .github/actions/spelling/expect.txt | 4 ++-- NEWS.md | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt index 934f582ed..000d5ebb8 100644 --- a/.github/actions/spelling/expect.txt +++ b/.github/actions/spelling/expect.txt @@ -184,7 +184,6 @@ sas sbp scsi sda -sdc setmarginal setprkey setprstatus @@ -205,11 +204,11 @@ suse svg switchgroup sys +SYSDIR sysfs sysinit tcp terabytes -SYSDIR TESTDEPS testname tgill @@ -231,6 +230,7 @@ unsetprkey unsetprstatus unspec usb +USEC userdata userspace usr diff --git a/NEWS.md b/NEWS.md index b740efb1e..3c51553ef 100644 --- a/NEWS.md +++ b/NEWS.md @@ -22,6 +22,14 @@ bug fixes from 0.11.0, and some CI-related fixes. * Fixed the problem that, if there were multiple maps with deferred failback (`failback` value > 0 in `multipath.conf`), some maps might fail back later than configured. The problem existed since 0.9.6. +* Removed a warning message that multipathd would print if systemd's + `WATCHDOG_USEC` environment variable had the value "0", which means that the + watchdog is simply disabled. This (minor) problem existed since 0.4.9. +* Fixed a memory leak in the nvme foreign library. The bug existed since + 0.7.8. +* Fixed a problem in the marginal path detection algorithm that could cause + the io error check for a recently failed path to be delayed. This bug + existed since 0.7.4. ## multipath-tools 0.10.0, 2024/08 From e860ef5be48c5068a9719b687416c7d3217a4a70 Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Mon, 25 Nov 2024 13:11:07 +0100 Subject: [PATCH 24/32] libmultipath: fix handling of pp->pgindex pp->pgindex is set in disassemble_map() when a map is parsed. There are various possiblities for this index to become invalid. pp->pgindex is only used in enable_group() and followover_should_fallback(), and both callers take no action if it is 0, which is the right thing to do if we don't know the path's pathgroup. Make sure pp->pgindex is reset to 0 in various places: - when it's orphaned, - before (re)grouping paths, - when we detect a bad mpp assignment in update_pathvec_from_dm(). - when a pathgroup is deleted in update_pathvec_from_dm(). In this case, pgindex needs to be invalidated for all paths in all pathgroups after the one that was deleted. The hunk in group_paths is mostly redundant with the hunk in free_pgvec(), but because we're looping over pg->paths in the former and over pg->pgp in the latter, I think it's better too play safe. Fixes: 99db1bd ("[multipathd] re-enable disabled PG when at least one path is up") Fixes: https://github.com/opensvc/multipath-tools/issues/105 Signed-off-by: Martin Wilck Reviewed-by: Benjamin Marzinski (cherry picked from commit cd912cffa2797a18c47426c816afa8eb2eae5b22) (cherry picked from commit 714c20bebba6911255a527d937e7a62764ffe338) --- libmultipath/pgpolicies.c | 6 ++++++ libmultipath/structs.c | 12 +++++++++++- libmultipath/structs_vec.c | 15 +++++++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/libmultipath/pgpolicies.c b/libmultipath/pgpolicies.c index edc3c611f..23ef2bdc2 100644 --- a/libmultipath/pgpolicies.c +++ b/libmultipath/pgpolicies.c @@ -127,6 +127,8 @@ split_marginal_paths(vector paths, vector *normal_p, vector *marginal_p) int group_paths(struct multipath *mp, int marginal_pathgroups) { vector normal, marginal; + struct path *pp; + int i; if (!mp->pg) mp->pg = vector_alloc(); @@ -138,6 +140,10 @@ int group_paths(struct multipath *mp, int marginal_pathgroups) if (!mp->pgpolicyfn) goto fail; + /* Reset pgindex, we're going to invalidate it */ + vector_foreach_slot(mp->paths, pp, i) + pp->pgindex = 0; + if (!marginal_pathgroups || split_marginal_paths(mp->paths, &normal, &marginal) != 0) { if (mp->pgpolicyfn(mp, mp->paths) != 0) diff --git a/libmultipath/structs.c b/libmultipath/structs.c index 61c8f32c3..485172525 100644 --- a/libmultipath/structs.c +++ b/libmultipath/structs.c @@ -239,8 +239,18 @@ free_pgvec (vector pgvec, enum free_path_mode free_paths) if (!pgvec) return; - vector_foreach_slot(pgvec, pgp, i) + vector_foreach_slot(pgvec, pgp, i) { + + /* paths are going to be re-grouped, reset pgindex */ + if (free_paths != FREE_PATHS) { + struct path *pp; + int j; + + vector_foreach_slot(pgp->paths, pp, j) + pp->pgindex = 0; + } free_pathgroup(pgp, free_paths); + } vector_free(pgvec); } diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c index 5df495b3e..9dc5a5ca5 100644 --- a/libmultipath/structs_vec.c +++ b/libmultipath/structs_vec.c @@ -108,6 +108,7 @@ static bool update_pathvec_from_dm(vector pathvec, struct multipath *mpp, struct config *conf; bool mpp_has_wwid; bool must_reload = false; + bool pg_deleted = false; if (!mpp->pg) return false; @@ -125,6 +126,10 @@ static bool update_pathvec_from_dm(vector pathvec, struct multipath *mpp, vector_foreach_slot(pgp->paths, pp, j) { + /* A pathgroup has been deleted before. Invalidate pgindex */ + if (pg_deleted) + pp->pgindex = 0; + if (pp->mpp && pp->mpp != mpp) { condlog(0, "BUG: %s: found path %s which is already in %s", mpp->alias, pp->dev, pp->mpp->alias); @@ -139,6 +144,13 @@ static bool update_pathvec_from_dm(vector pathvec, struct multipath *mpp, must_reload = true; dm_fail_path(mpp->alias, pp->dev_t); vector_del_slot(pgp->paths, j--); + /* + * pp->pgindex has been set in disassemble_map(), + * which has probably been called just before for + * mpp. So he pgindex relates to mpp and may be + * wrong for pp->mpp. Invalidate it. + */ + pp->pgindex = 0; continue; } pp->mpp = mpp; @@ -237,6 +249,8 @@ static bool update_pathvec_from_dm(vector pathvec, struct multipath *mpp, vector_del_slot(mpp->pg, i--); free_pathgroup(pgp, KEEP_PATHS); must_reload = true; + /* Invalidate pgindex for all other pathgroups */ + pg_deleted = true; } mpp->need_reload = mpp->need_reload || must_reload; return must_reload; @@ -354,6 +368,7 @@ void orphan_path(struct path *pp, const char *reason) { condlog(3, "%s: orphan path, %s", pp->dev, reason); pp->mpp = NULL; + pp->pgindex = 0; uninitialize_path(pp); } From 8c62ec35d3bde07645d1751de806d7e8ba13afd3 Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Mon, 25 Nov 2024 15:18:15 +0100 Subject: [PATCH 25/32] libmultipath: make pgcmp detect if map is missing a path group The previous algorithm didn't detect the case case where cpgp contained a path that was not contained in pgp. Fix this. Cherry-picked from d4b35f61cb75c6e9b289e56c98457fc04ce4835e Fixes: 90773ba ("libmultipath: resolve hash collisions in pgcmp()") Signed-off-by: Martin Wilck --- libmultipath/configure.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/libmultipath/configure.c b/libmultipath/configure.c index a72579812..e86c1fd5e 100644 --- a/libmultipath/configure.c +++ b/libmultipath/configure.c @@ -426,6 +426,11 @@ compute_pgid(struct pathgroup * pgp) pgp->id ^= (long)pp; } +static void cleanup_bitfield(struct bitfield **p) +{ + free(*p); +} + static int pgcmp (struct multipath * mpp, struct multipath * cmpp) { @@ -433,16 +438,25 @@ pgcmp (struct multipath * mpp, struct multipath * cmpp) struct pathgroup * pgp; struct pathgroup * cpgp; int r = 0; + struct bitfield *bf __attribute__((cleanup(cleanup_bitfield))) = NULL; if (!mpp) return 0; + if (VECTOR_SIZE(mpp->pg) != VECTOR_SIZE(cmpp->pg)) + return 1; + + bf = alloc_bitfield(VECTOR_SIZE(cmpp->pg)); + if (!bf) + return 1; + vector_foreach_slot (mpp->pg, pgp, i) { compute_pgid(pgp); vector_foreach_slot (cmpp->pg, cpgp, j) { if (pgp->id == cpgp->id && !pathcmp(pgp, cpgp)) { + set_bit_in_bitfield(j, bf); r = 0; break; } @@ -451,6 +465,10 @@ pgcmp (struct multipath * mpp, struct multipath * cmpp) if (r) return r; } + vector_foreach_slot (cmpp->pg, cpgp, j) { + if (!is_bit_set_in_bitfield(j, bf)) + return 1; + } return r; } From 1eb0719f8028a472b00a26fe2adface090bc575d Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Tue, 26 Nov 2024 22:37:20 +0100 Subject: [PATCH 26/32] libmultipath: trigger uevents upon map creation in domap() If map creation succeeds, previously not multipathed devices are now multipathed. udev may not have noticed this yet, thus trigger path uevents to make it aware of the situation. Likewise, if creating a map fails, the paths in question were likely considered multipath members by udev, too. They will now be marked as failed, so trigger an event in this situation as well. Fixes: https://github.com/opensvc/multipath-tools/issues/103 Suggested-by: Benjamin Marzinski Signed-off-by: Martin Wilck Reviewed-by: Benjamin Marzinski (cherry picked from commit 98b3a7bd9ed2a89f068fe40c253843cf78261905) --- libmultipath/configure.c | 11 ++++------- libmultipath/devmapper.c | 9 +++------ 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/libmultipath/configure.c b/libmultipath/configure.c index e86c1fd5e..d9fac3848 100644 --- a/libmultipath/configure.c +++ b/libmultipath/configure.c @@ -581,8 +581,6 @@ trigger_paths_udev_change(struct multipath *mpp, bool is_mpath) vector_foreach_slot(pgp->paths, pp, j) trigger_path_udev_change(pp, is_mpath); } - - mpp->needs_paths_uevent = 0; } static int sysfs_set_max_sectors_kb(struct multipath *mpp) @@ -954,10 +952,10 @@ int domap(struct multipath *mpp, char *params, int is_daemon) * succeeded */ mpp->force_udev_reload = 0; - if (mpp->action == ACT_CREATE && - (remember_wwid(mpp->wwid) == 1 || - mpp->needs_paths_uevent)) + if (mpp->action == ACT_CREATE) { + remember_wwid(mpp->wwid); trigger_paths_udev_change(mpp, true); + } if (!is_daemon) { /* multipath client mode */ dm_switchgroup(mpp->alias, mpp->bestpg); @@ -982,8 +980,7 @@ int domap(struct multipath *mpp, char *params, int is_daemon) } dm_setgeometry(mpp); return DOMAP_OK; - } else if (r == DOMAP_FAIL && mpp->action == ACT_CREATE && - mpp->needs_paths_uevent) + } else if (r == DOMAP_FAIL && mpp->action == ACT_CREATE) trigger_paths_udev_change(mpp, false); return DOMAP_FAIL; diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c index fe5637b3f..a0070c566 100644 --- a/libmultipath/devmapper.c +++ b/libmultipath/devmapper.c @@ -536,7 +536,7 @@ static uint16_t build_udev_flags(const struct multipath *mpp, int reload) MPATH_UDEV_RELOAD_FLAG : 0); } -int dm_addmap_create (struct multipath *mpp, char * params) +int dm_addmap_create (struct multipath *mpp, char *params) { int ro; uint16_t udev_flags = build_udev_flags(mpp, 0); @@ -546,9 +546,7 @@ int dm_addmap_create (struct multipath *mpp, char * params) if (dm_addmap(DM_DEVICE_CREATE, TGT_MPATH, mpp, params, ro, udev_flags)) { - if (unmark_failed_wwid(mpp->wwid) == - WWID_FAILED_CHANGED) - mpp->needs_paths_uevent = 1; + unmark_failed_wwid(mpp->wwid); return 1; } /* @@ -566,8 +564,7 @@ int dm_addmap_create (struct multipath *mpp, char * params) break; } } - if (mark_failed_wwid(mpp->wwid) == WWID_FAILED_CHANGED) - mpp->needs_paths_uevent = 1; + mark_failed_wwid(mpp->wwid); return 0; } From 6de0c88f5fc8cb5a35c6478992fab46befd35d64 Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Wed, 27 Nov 2024 20:39:50 +0100 Subject: [PATCH 27/32] multipathd: trigger uevents upon map removal in coalesce_maps() ... if a map has been flushed. In this case, we know that the the paths haven't been multipathed by coalesce_paths() because of the current configuration (failure to create the map can't be the reason if the map exists in coalesce_maps()). Make sure udev sees the paths which have been released from the map as non-multipath. Note that this is the only case where maps are flushed where it is correct to trigger paths uevents. In other cases, e.g. after a "remove map" CLI command, the configuration is unchanged and if we triggered an uevent, the map would be re-created by multipathd when the uevent arrived. Signed-off-by: Martin Wilck Reviewed-by: Benjamin Marzinski (cherry picked from commit ad3ea472b587c0c20d2100331b5c66f8602f8414) --- multipathd/main.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/multipathd/main.c b/multipathd/main.c index 3fb623fdc..4b089c0e2 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -794,8 +794,10 @@ coalesce_maps(struct vectors *vecs, vector nmpv) vector_del_slot(ompv, i); i--; } - else + else { condlog(2, "%s devmap removed", ompp->alias); + trigger_paths_udev_change(ompp, false); + } } else if (reassign_maps) { condlog(3, "%s: Reassign existing device-mapper" " devices", ompp->alias); From 70d8a4069763ff1a93a025a81e522a0a4f6d5cc1 Mon Sep 17 00:00:00 2001 From: Benjamin Marzinski Date: Wed, 4 Dec 2024 22:56:36 -0500 Subject: [PATCH 28/32] libmultipath: Don't skip set_path_max_sectors_kb() If a multipath device already has need_reload set when a path is adopted, it won't call set_path_max_sectors_kb() because of short-circuit evaluation. This isn't what's intended. Fixes: e5e20c7b ("libmultipath: set max_sectors_kb in adopt_paths()") Signed-off-by: Benjamin Marzinski Reviewed-by: Martin Wilck (cherry picked from commit f5c0c4b25c5eaac87b78e6e0c8d52b0828c29893) --- libmultipath/structs_vec.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c index 9dc5a5ca5..683ab473a 100644 --- a/libmultipath/structs_vec.c +++ b/libmultipath/structs_vec.c @@ -349,8 +349,7 @@ int adopt_paths(vector pathvec, struct multipath *mpp, */ if (!current_mpp || !mp_find_path_by_devt(current_mpp, pp->dev_t)) - mpp->need_reload = mpp->need_reload || - set_path_max_sectors_kb(pp, mpp->max_sectors_kb); + mpp->need_reload = set_path_max_sectors_kb(pp, mpp->max_sectors_kb) || mpp->need_reload; } pp->mpp = mpp; From 9666a0fe83416a6833963aadbafd13c85486781e Mon Sep 17 00:00:00 2001 From: Benjamin Marzinski Date: Wed, 22 Jan 2025 22:16:42 -0500 Subject: [PATCH 29/32] libmultipath: stop static analyzer complaint in init_foreign This change doesn't actually fix anything. The code was already safe. Signed-off-by: Benjamin Marzinski Reviewed-by: Martin Wilck (cherry picked from commit 85ec51e7930b4cadfbc12718afa91ce3a4adf4b5) --- libmultipath/foreign.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libmultipath/foreign.c b/libmultipath/foreign.c index 28d1b1158..af5b0ed3b 100644 --- a/libmultipath/foreign.c +++ b/libmultipath/foreign.c @@ -129,7 +129,7 @@ static void free_pre(void *arg) static int _init_foreign(const char *enable) { char pathbuf[PATH_MAX]; - struct dirent **di; + struct dirent **di = NULL; struct scandir_result sr; int r, i; regex_t *enable_re = NULL; From 0514280519240da67d77d100903d0037c985257c Mon Sep 17 00:00:00 2001 From: Benjamin Marzinski Date: Wed, 22 Jan 2025 22:16:43 -0500 Subject: [PATCH 30/32] libmultipath: be lenient in allowing the alua-based pgpolicies multipath wouldn't autodetect the GROUP_BY_PRIO path grouping policy or allow the GROUP_BY_TPG policy if there was a path that didn't have its prioritizer selected (for instance because multipathd was reconfigured while it was offline). To avoid this, make verify_alua_prio() assume an alua multipath device if all the paths with a prioritizer selected (there must be at least one) use an alua-based prioritizer. Signed-off-by: Benjamin Marzinski Reviewed-by: Martin Wilck (cherry picked from commit b47a577998eaff203018a00b57ba5e3674645848) --- libmultipath/propsel.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c index a3fce2039..c09a06198 100644 --- a/libmultipath/propsel.c +++ b/libmultipath/propsel.c @@ -255,14 +255,18 @@ verify_alua_prio(struct multipath *mp) { int i; struct path *pp; + bool assume_alua = false; vector_foreach_slot(mp->paths, pp, i) { const char *name = prio_name(&pp->prio); + if (!prio_selected(&pp->prio)) + continue; if (strncmp(name, PRIO_ALUA, PRIO_NAME_LEN) && strncmp(name, PRIO_SYSFS, PRIO_NAME_LEN)) return false; + assume_alua = true; } - return true; + return assume_alua; } int select_detect_pgpolicy(struct config *conf, struct multipath *mp) From 02797bbde6417a318f892c238a655b4086b45438 Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Fri, 24 Jan 2025 20:03:15 +0100 Subject: [PATCH 31/32] Update NEWS.md for 0.10.2 --- .github/actions/spelling/expect.txt | 2 ++ NEWS.md | 23 ++++++++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt index 000d5ebb8..4ac57510d 100644 --- a/.github/actions/spelling/expect.txt +++ b/.github/actions/spelling/expect.txt @@ -11,6 +11,7 @@ ata autoconfig autodetected autoresize +backported barbie BINDIR blkid @@ -122,6 +123,7 @@ Lun lvm lvmteam Marzinski +misdetection mpath mpathb mpathpersist diff --git a/NEWS.md b/NEWS.md index 3c51553ef..1b239ffa7 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,27 @@ # multipath-tools Release Notes -## multipath-tools 0.10.1, 2024/11 +## multipath-tools 0.10.2, 2025/02 + +This release contains backported bug fixes from the stable-0.11.y branch. + +### Bug fixes + +* Fix multipathd crash because of invalid path group index value, for example + if an invalid path device was removed from a map. + Fixes [#105](https://github.com/opensvc/multipath-tools/issues/105). +* Make sure maps are reloaded in the path checker loop after detecting an + inconsistent or wrong kernel state (e.g. missing or falsely mapped path + device). Wrongly mapped paths will be unmapped and released to the system. + Fixes another issue reported in + [#105](https://github.com/opensvc/multipath-tools/issues/105). +* Fix the problem that `group_by_tpg` might be disabled if one or more + paths were offline during initial configuration. +* Fix possible misdetection of changed pathgroups in a map. +* Fix the problem that if a map was scheduled to be reloaded already, + `max_sectors_kb` might not be set on a path device that + was being added to a multipath map. This problem was introduced in 0.9.9. + +## multipath-tools 0.10.1, 2025/01 This is the first bug fix release on the `stable-0.10.y` branch. It contains bug fixes from 0.11.0, and some CI-related fixes. From ab323f504be2543f4baefa0b2f419583153f87e8 Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Sat, 25 Jan 2025 00:49:19 +0100 Subject: [PATCH 32/32] GitHub Workflows: fix abi-stable.yaml for pull request Signed-off-by: Martin Wilck --- .github/workflows/abi-stable.yaml | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/.github/workflows/abi-stable.yaml b/.github/workflows/abi-stable.yaml index a6746f233..472914ce3 100644 --- a/.github/workflows/abi-stable.yaml +++ b/.github/workflows/abi-stable.yaml @@ -15,12 +15,16 @@ on: jobs: reference-abi: - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 steps: - name: get parent tag run: > echo ${{ github.ref }} | sed -E 's,refs/heads/stable-([0-9]\.[0-9]*)\.y,PARENT_TAG=\1.0,' >> $GITHUB_ENV + if: ${{ github.event_name == 'push' }} + - name: get parent tag + run: echo PARENT_TAG=${{ github.base_ref }} >> $GITHUB_ENV + if: ${{ github.event_name == 'pull_request' }} - name: assert parent tag run: /bin/false if: ${{ env.PARENT_TAG == '' }} @@ -45,20 +49,24 @@ jobs: path: abi check-abi: - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 needs: reference-abi steps: - name: get parent tag run: > echo ${{ github.ref }} | sed -E 's,refs/heads/stable-([0-9]\.[0-9]*)\.y,PARENT_TAG=\1.0,' >> $GITHUB_ENV + if: ${{ github.event_name == 'push' }} + - name: get parent tag + run: echo PARENT_TAG=${{ github.base_ref }} >> $GITHUB_ENV + if: ${{ github.event_name == 'pull_request' }} - name: assert parent tag run: /bin/false if: ${{ env.PARENT_TAG == '' }} - - name: checkout ${{ env.PARENT_TAG }} + - name: checkout ${{ github.base_ref }} uses: actions/checkout@v4 with: - ref: ${{ env.PARENT_TAG }} + ref: ${{ github.base_ref }} - name: download ABI for ${{ env.PARENT_TAG }} id: download_abi uses: actions/download-artifact@v4