lvm2: update to 2.03.42 - #30335
Conversation
4bc6ba7 to
9566181
Compare
openwrt-ai
left a comment
openwrt-ai
left a comment
There was a problem hiding this comment.
Reviewed 1 new commit. The version/hash bump itself looks fine — PKG_RELEASE correctly stays at 1 for a version bump, and CI's download.pl accepted the new PKG_HASH, so the tarball checksum is good.
The one thing that would block a merge is the patch format: both patches are committed as git format-patch output rather than in the shape make package/lvm2/refresh produces, and that is what all ten Feeds Package Test Build jobs are failing on ("Dirty patches detected"). Details inline.
Generated by Claude Code
| diff --git a/lib/commands/toolcontext.c b/lib/commands/toolcontext.c | ||
| index 4c8a894..d0eaa10 100644 |
There was a problem hiding this comment.
These patches are no longer in the form the tree's own refresh produces, and that is what all ten build jobs are failing on:
Refreshed patch 002-const-stdio.patch
Refreshed patch 003-no-mallinfo.patch
...
Dirty patches detected, please refresh and review the diff
##[error]Process completed with exit code 1.
The git format-patch output committed here carries diff --git / index … lines and untruncated function context in the hunk headers, none of which survive a quilt refresh — OpenWrt refreshes with quilt refresh -p ab --no-index --no-timestamps in include/quilt.mk:173, so the index lines are dropped and GNU diff -p caps the @@ function context at 40 chars (struct cmd_context *create_toolcontext(c). The trailing -- / 2.55.0 signature at the end of both files goes away too.
Running make package/lvm2/refresh V=s and committing the result gets CI green. The From: / Date: / Subject: / Signed-off-by: header block you added is above quilt's first diff line, so it is preserved as the patch header and you keep the authorship attribution — only the diff body gets rewritten.
Same applies to 003-no-mallinfo.patch.
Generated by Claude Code
| From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
| From: Daniel Golle <[email protected]> | ||
| Date: Sat, 25 Apr 2015 23:34:05 +0200 | ||
| Subject: [PATCH] no-mallinfo: guard glibc mallinfo()/mlockall policy behind | ||
| __GLIBC__ | ||
|
|
||
| _allocate_memory()'s heap-growth probing uses glibc's MALLINFO/mallinfo(), | ||
| which musl doesn't provide; guard it behind #ifdef __GLIBC__ and always | ||
| grow by the full alloc_size on musl instead. Likewise _lock_mem() reads | ||
| activation/use_mlockall from lvm.conf on glibc but always uses mlockall | ||
| unconditionally on musl, where the glibc-specific heuristic this option | ||
| tunes doesn't apply. | ||
|
|
||
| Signed-off-by: Daniel Golle <[email protected]> | ||
| --- | ||
| lib/mm/memlock.c | 11 +++++++++++ | ||
| 1 file changed, 11 insertions(+) |
There was a problem hiding this comment.
nit: the PR body describes this as "Refreshed 002-const-stdio.patch and 003-no-mallinfo.patch (hunk offsets only)" and the formalities checkbox adds "No new patch content here, this is only a hunk-offset refresh of an existing local patch" — but both files also gain a full authored header block and a reformatted diff body. The commit message does describe the reformatting, so it is just the PR body that reads as understating it; worth aligning the two so a reviewer skimming the description knows to look at more than the @@ lines.
Generated by Claude Code
|
Both points fixed, thank you:
|
openwrt-ai
left a comment
openwrt-ai
left a comment
There was a problem hiding this comment.
Re-reviewed at ec54886 (1 amended commit since 9566181).
Both points from the previous round are addressed. The patch bodies are now in the shape make package/lvm2/refresh produces — diff --git / index lines gone, @@ function context truncated at 40 chars (struct cmd_context *create_toolcontext(c), trailing -- / 2.55.0 footer gone — matching the pre-existing format in the base revision of both files. The From: / Date: / Subject: / Signed-off-by: header block survived the refresh intact, and the diffstats in both headers are accurate (002: 4 + 7 = 11 insertions; 003: 11 insertions). The null-SHA From 0000…0000 Mon Sep 17 00:00:00 2001 line is fine — 34 patches in openwrt/openwrt use the same form. The PR body now describes the header-block addition, so the earlier nit is resolved too.
CI on this head: the three FormalityCheck jobs are green and the build jobs are still queued, so nothing to report there.
Commit checks
ec54886"lvm2: update to 2.03.42" — the commit body still says the patches were "regenerated viagit format-patchagainst a scratch commit of the pristine source". That was true of the previous revision; after the amend the committed files are quilt/refresh output instead. Only the header block came fromformat-patch. Worth correcting, sinceformat-patchoutput is exactly what the build jobs rejected last round and the message as written invites the next person to reproduce it.
Generated by Claude Code
| buffering directly via glibc-only internals; _check_standard_fds() has | ||
| a musl-compatible fallback path already available. Wrap the |
There was a problem hiding this comment.
nit: the description says _check_standard_fds() "has a musl-compatible fallback path already available", but this patch is what adds it — the #else branch at lines 55-59 of this file (if (!is_valid_fd(STDERR_FILENO) || ...) return 0;) is a + line, not pre-existing upstream code. Reads as if only the #ifdef wrapping were new.
| buffering directly via glibc-only internals; _check_standard_fds() has | |
| a musl-compatible fallback path already available. Wrap the | |
| buffering directly via glibc-only internals; _check_standard_fds() gets a | |
| musl-compatible fallback path added below. Wrap the |
Generated by Claude Code
LVM2 2.03.42: - add --lockopt retries=N / removeretry=SECONDS to lvmlockd - use lockd_lv lock probes for pvmove holder availability in shared VGs - auto-revert metadata when initial pvmove mirror activation fails - refuse activation of a pvmove-locked LV when pvmove runs on another node; allow pvmove --abort without args when using lvmlockd - add cluster lock lifecycle for pvmove LV in shared VGs, and support for pvmove in a shared VG - allow lvremove -f to remove pvmove-locked LVs after an interrupted pvmove; handle active pvmove during vgchange -an with force flags - fix pvmove completion to use suspend+resume for DM table reload; fix deactivation cascade during active pvmove - support inactive LVs in pvmove with metadata-only mirror insertion; kill orphaned polling process on pvmove --abort - support formatting vdo volumes with the kernel vdo target (>=9.2); fix VDO index_memory_size_mb quantisation and report the minimum required pool size - add --enable-asan/--enable-tsan build options device-mapper 1.02.216: - make libdm thread safe - add use_kernel_format to dm_vdo_target_params - introduce libdevmapper-san with --enable-asan/--enable-tsan - parent table force reload when child device table changes Refreshed 002-const-stdio.patch and 003-no-mallinfo.patch against the new upstream source (make package/lvm2/refresh; hunk offsets only, no content change) and gave both a proper git-am header block (From/Date/ Subject/Signed-off-by), crediting their original authorship (Daniel Golle, 2015) with an accurate subject/body; they previously had none, which the CI formality bot flagged. The header block was drafted via git format-patch against a scratch commit of the pristine source, then the whole patch re-refreshed through quilt so the diff body matches what make package/lvm2/refresh actually produces byte for byte - format-patch output alone does not survive OpenWrt's own refresh (diff --git/index lines and full @@ context get stripped, the trailing signature dropped), which is what the CI patch-format check was rejecting before this. Signed-off-by: Daniel Golle <[email protected]>
|
Right, thanks — the message still described the format-patch step in isolation, which is exactly the part CI rejected. Corrected the wording: the header block came from format-patch, but the whole thing was then re-refreshed through quilt so the diff body matches |
openwrt-ai
left a comment
openwrt-ai
left a comment
There was a problem hiding this comment.
Reviewed 1 new commit; no new issues found.
git diff ec54886..c182eb9 is empty — the amend touched only the commit message, and the correction from the last round is in: the body now says the header block was drafted with format-patch and the whole patch then re-refreshed through quilt, which matches what is actually committed. The rest of the message still checks out against the diff — PKG_VERSION/PKG_VERSION_DM/PKG_HASH bumped, PKG_RELEASE correctly left at 1, and "hunk offsets only, no content change" is accurate for both patch bodies (002: -1665 → -1666 etc., 003: -560 → -565; the 40-char-truncated @@ context was already truncated in the base revision, so nothing but line numbers moved).
CI on this head is fully green — all three FormalityCheck jobs plus all ten Feeds Package Test Build targets passed, so the "Dirty patches detected" failure from the first round is resolved.
Two things still outstanding, neither new:
- The inline comment on
002-const-stdio.patchline 9 is unaddressed — the header body says_check_standard_fds()"has a musl-compatible fallback path already available", but that fallback is added by this patch. A suggestion block is on that thread. - The PR is still marked as a draft.
Generated by Claude Code
|
You don't really need to reply to every bot comment. Watching two AIs talk to each other is cool and all, but it just clutters up the pull request and makes it way too long. |
| Subject: [PATCH] const-stdio: guard glibc-specific stream buffering behind | ||
| __GLIBC__ |
There was a problem hiding this comment.
Any plans to upstream this? This is here for so long.
| Subject: [PATCH] no-mallinfo: guard glibc mallinfo()/mlockall policy behind | ||
| __GLIBC__ |
There was a problem hiding this comment.
Can you send this to upstream, please?
There was a problem hiding this comment.
I'll try... (both patches have been sitting there for years)
3 participants
📦 Package Details
Maintainer: @dangowrt
(You can find this by checking the history of the package
Makefile.)Description:
From 2.03.41. Substantial pvmove-in-shared-VG work for lvmlockd (new --lockopt retries/removeretry, cluster lock lifecycle, metadata auto-revert, activation refusal across nodes, suspend+resume for DM table reload), VDO formatting with the kernel vdo target (>=9.2), and new --enable-asan/--enable-tsan build options.
The tarball also bundles device-mapper as a separately versioned component; bumped
PKG_VERSION_DMfrom 1.02.215 to 1.02.216 to match upstream'sVERSION_DMfile (libdm made thread safe, dm_vdo_target_params gained use_kernel_format, new libdevmapper-san with asan/tsan support).Refreshed 002-const-stdio.patch and 003-no-mallinfo.patch against the new upstream source: hunk offsets only, no diff content changed, but both patches also gained a proper
From <hash> Mon Sep 17 00:00:00 2001/From:/Date:/Subject:/Signed-off-by:header block (neither patch had one before), attributed to their original author/date viagit log --follow.🧪 Run Testing Details
✅ Formalities
If your PR contains a patch:
git am(e.g., subject line, commit description, etc.)
We must try to upstream patches to reduce maintenance burden.
(No new patch content here — this is a hunk-offset refresh against the new upstream source, plus adding the git-am header block both patches were missing.)