Merge ~mitchdz/ubuntu/+source/multipath-tools:merge-lp2040360-noble into ubuntu/+source/multipath-tools:debian/sid

https://github.com/opensvc/multipath-tools/issues/76

The socket activation feature is not working as expected. I outlined the reason in the issue linked above, and I'm also investigating a way to overcome the limitation described there.

I hid my previous comments to not clutter these logs and waste your time reviewing. They aren't important, but Sergio was mentioning changes to attempt to enable socket-based activation which fell through, so that is not present in this merge.

A lot to talk about here, but I want to be very thorough.

Overall this merge changed a ton of things on the packaging side, so there should be extra caution taken here.

A lot of changes were dropped. Most make sense, but there is 2 changes that I want to add extra attention to:
1)
+ + d/initramfs/hooks: do not copy kpartx rules to initramfs
+ [ Patch does not apply cleanly in debian version 0.9.7-4 and seems to be unnecessary ]
This change is dropped, simply because it does not apply easily and I couldn't dig as to _why_ exactly we want to drop kpartx rules from initramfs. It is bundled in a commit titled "multipath initramfs fixes for booting from multipathed devices" so I of course tested the multipath-tools-boot, which I will explain further below. Please let me know if you think this should require more investigation whether it is safe to drop. I think it should be fine as long as booting from a multipathd device works. I do have slight concern that is could be related to the kpartx-boot split commit, but since it's not part of that commit I will assume it's not.

2)
+ - d/rules: Move udev rules to priority 95
+ [ Dropped due to being not necessary, and request from debian maintainer ]
I don't see much issues with this, but would love some second opinions. See the bug report for the merge to see the request from the debian maintainer for this.

Now, 2 autopkgtests were failing when I did the initial merge. initramfs && tgtbasedmpaths.

initramfs) I fixed the new test "initramfs" easily and the changes make sense there.

tgtbasedmpaths)

The issue with tgtbasedmpaths is that the multipathd daemon would seg fault sometime during the setup phase, usually around the command `iscsiadm --mode session -r 1 --op new`. This was pretty easy to reproduce, but could not easily be reproduced _after_ the first failure. So I would boot a fresh VM, run the autopkgtest commands manually, hit the seg fault, attempt to trigger the seg fault again, but could not. This led me to experiment a lot of things - my first hunch was that the multipathd.service was not actually restarting properly, but the PID did change and it seemed to restart properly.

Nevertheless, I decided to attempt a change I've been wanting to make a while, which is to remove the restart service in the multipath-tools.postinst, and manually stop+start the socket+service in the correct order that they depend on. This made the tgtbasedmpaths test very consistent and I could not make it fail. Honestly I'm not sure why this fixed it, but I think it's something we should probably do anyways so I'd like to propose merging it in with noble. This also fixes LP: #2034471.

Now, onto multipath-tools-boot. I was very concerned with this functionality and wanted to ensure it works fine. I tested this manually the following way:

1) Download noble-live-server-amd64.iso from https://cdimage.ubuntu.com/ubuntu-server/daily-live/current/
2) f...

  - multipath-tools/0.9.7-4ubuntu1~noble3
    + ✅ multipath-tools on noble for amd64 @ 23.02.24 18:38:49
      • Log: https://autopkgtest.ubuntu.com/results/autopkgtest-noble-mitchdz-merge-lp2040360-noble-multipath-tools/noble/amd64/m/multipath-tools/20240223_183849_438be@/log.gz
    + ✅ multipath-tools on noble for arm64 @ 23.02.24 18:38:20
      • Log: https://autopkgtest.ubuntu.com/results/autopkgtest-noble-mitchdz-merge-lp2040360-noble-multipath-tools/noble/arm64/m/multipath-tools/20240223_183820_db73d@/log.gz
    + ✅ multipath-tools on noble for armhf @ 23.02.24 18:26:10
      • Log: https://autopkgtest.ubuntu.com/results/autopkgtest-noble-mitchdz-merge-lp2040360-noble-multipath-tools/noble/armhf/m/multipath-tools/20240223_182610_e6a6a@/log.gz
    + ✅ multipath-tools on noble for ppc64el @ 23.02.24 18:39:15
      • Log: https://autopkgtest.ubuntu.com/results/autopkgtest-noble-mitchdz-merge-lp2040360-noble-multipath-tools/noble/ppc64el/m/multipath-tools/20240223_183915_2192a@/log.gz
    + ✅ multipath-tools on noble for s390x @ 23.02.24 18:34:16
      • Log: https://autopkgtest.ubuntu.com/results/autopkgtest-noble-mitchdz-merge-lp2040360-noble-multipath-tools/noble/s390x/m/multipath-tools/20240223_183416_7aa80@/log.gz

Looking at this merge.

There is a d/changelog conflict, btw

Thanks for pointing out the conflict! I'm honestly not exactly sure why that conflict exists right now, but I'll dig into it.

This review is going to be in multiple passes.

a) You are dropping this:
https://bugs.launchpad.net/ubuntu/+source/initramfs-tools/+bug/1673350
With the comment:
"Debian changed to using more modern helpers which should pick this up"

That bug was marked as critical.

Can you verify, with the test case on that bug, that this is still the case with this dropped delta?

b) - d/multipath.conf: Install friendly names multipath.conf by default
      [ We said we would drop it a while back and forgot to, dropping here ]

I think it was said this would be revisited, not just dropped. Has there been any new discussion about this? I vaguely remember that it was discussed a lot back when we introduced it, and we should have good reasoning to drop it. What does this do? What's the impact? Are there upgrade concerns? Or am I wrong and we really did say we would drop it in the next LTS?

a) good point, I just did a light inspection, I will go ahead and perform the test case.
b) I asked Utkarsh about it in IM and he gave his +1 to drop it, I'm not sure about much more discussions on this, and wasn't around when it was introduced.

Utkarsh did mention that it would be dropped from Ubuntu in the MR[0] so I assumed that was the collective stance

[0] - https://salsa.debian.org/linux-blocks-team/multipath-tools/-/merge_requests/4

> + + d/initramfs/hooks: do not copy kpartx rules to initramfs

Regarding this bit, part of a bigger delta. The original diff for this, in the logical tag, is:
49bd6a96320f43e3f13114195b4ff9b0a977ace9
 add_udev_rules()
 {
- for rules in 60-multipath.rules 60-kpartx.rules; do
+ for rules in 60-multipath.rules 56-dm-mpath.rules; do

It's just removing 60-kpartx.rules from the list, right?

In the new hooks/multipath file, we have:

# udev: kpartx
copy_exec /usr/sbin/kpartx
copy_exec /usr/bin/partx
copy_exec /usr/lib/udev/kpartx_id
copy_file udev_rule /usr/lib/udev/rules.d/56-dm-parts.rules
copy_file udev_rule /usr/lib/udev/rules.d/60-kpartx.rules
copy_file udev_rule /usr/lib/udev/rules.d/68-del-part-nodes.rules

So transporting the delta to the new debian package, wouldn't that be "just" dropping the copy_file line that copies 60-kpartx.rules?

From what I could dig up, kpartx-boot was split off to handle a dmraid (fakeraid) boot scenario. We have even grown a dependency on bin:kpartx-boot in the src:dmraid package. How to test that? No clue. But bringing in the delta to the new debian package looks like the above bit, so I'm unsure why you said it didn't apply. Maybe I'm missing something. The other big kpartx delta (creation of the new package) you seem to have transported just fine.

I just meant the patch doesn't apply as-is. We could definitely transport the logical change of that delta by removing the copy_file line.

I suppose if that delta has been around this long it should be fine to keep, but I'd like to investigate why we do that since I wasn't able to find that history.

Perhaps I can make a commit to remove the copy_file for the kpartx udev rules for this noble merge, and then for the next merge cycle it'll be an activity to see if it's truly safe to drop or not?

Would also need to modify the new initramfs dep8 test to stop checking for the kpartx udev rule if I make the commit mentioned above.

We don't have super good test coverage for all the scenarios where multipath-tools is being used. I would keep the delta until we know better, yes. I would keep it part of the "multipath initramfs fixes for booting from multipathed devices:" chunk. We have so little information about these changes, I would prefer to not scatter it more. At least we know it was part of that chunk.

Sounds good to me. I'll make a new commit to drop the kpartx udev rules explaining everything that we know so far. That is, once I fix the fun issue with our base being wrong :)

Also while it's on my mind - I did find the delta I had to make to the new initramfs dep8 test interesting

-chroot initramfs /usr/sbin/multipath -h
+chroot initramfs/main /usr/sbin/multipath -h

I'm not sure why unmkinitramfs makes a different directory structure than the test in debian, and would love to update the test in debian to work on both debian/ubuntu so we don't need to carry a delta going forward. I'm sure it could be something simple, just haven't dug into it much yet.

Maybe file an LP bug to follow up

Made a simple bug to track it - https://bugs.launchpad.net/ubuntu/+source/multipath-tools/+bug/2055462

> I would prefer to not scatter it more.

Keeping it in the same "block" should also help git range-diff to make more sense of the changes

Sorry not familiar with that terminology. By block do you mean in the same commit order? e.g. add the delta where the old drop was, so it will end up before the "* Dropped: - debian/initramfs/local-bottom..." and right after the "* Dropped: - debian/initramfs/hooks..." commits

I'm thinking we should keep the friendly-names delta. It's what made our package ship /etc/multipath.conf, and if we are now not shipping that anymore (or is debian shipping a config file now?), then that might need some config file handling with maintainer scripts.

> By block do you mean in the same commit order?

I mean it was part of this delta:
    - multipath initramfs fixes for booting from multipathed devices:
      + d/initramfs/hooks: also copy wwids file on the installed
        system to ensure all paths come up on boot. (LP 1479929)
      + d/initramfs/hooks: install multipathd and required
        directories.
      + d/initramfs/hooks: copy multipath udev rules to initramfs
## + d/initramfs/hooks: do not copy kpartx rules to initramfs ##
      + d/initramfs/local-bottom: remember to stop multipathd.
      + d/initramfs/local-premount: wait for udev to settle before
        the call to resolve_device() in local_mount_root(), so the
        by-uuid/ symlinks have a chance to be updated by the
        multipath udev rules (LP 1503286).
      + d/initramfs/local-premount: Run multipath with -B so not to
        assign names nor change /etc/multipath/bindings during
        initramfs (LP 1561103)

(my highlight above)

But I'm seeing you dropped most (all?) of it, so your plan works too. I'm still trying to go over all the changes :)

Your hunch was right - I did start the merge before 0.9.4-7 was in debian, that's why my PPA is 0.9.4-6ubuntu1. I'll go ahead and fix that.

Regarding the friendly names - the new package actually does create the same exact /etc/multipath.conf.

I expected it to be similar, but not exactly the same, because I think it should be the contents of https://git.launchpad.net/ubuntu/+source/multipath-tools/tree/debian/d-i/multipath.conf?h=applied/debian/sid

But instead we have

$ lxc launch ubuntu-daily:noble n-vm --vm
$ lxc shell n-vm
# add-apt-repository ppa:mitchdz/multipath-tools-noble-merge
# apt update -y
# apt purge multipath-tools
# cat /etc/multipath.conf
cat: /etc/multipath.conf: No such file or directory
# apt install -y multipath-tools=0.9.4-6ubuntu1~noble2
# cat /etc/multipath.conf
defaults {
    user_friendly_names yes
}

So for some reason we lose "find_multipaths yes" it seems. I'd like to dig into this a little more.

> So for some reason we lose "find_multipaths yes" it seems. I'd like to dig into this a little more.

Oh, I thought that define would be a compile-time default, and not end up as a config option explicitly in the config file.

FWIW, I believe https://salsa.debian.org/linux-blocks-team/multipath-tools/-/merge_requests/13 should be picked up in this merge.

well I am still carrying debian/patches/enable-find-multipaths.patch to force FIND_MULTIPATHS_ON which is a compile-time default.

good point, I'll make sure to include that in d/NEWS

FF is looming, I'm wondering if it would be safer to request an FFe so we have more time on this merge... Assuming an FFe would even be granted for such changes.

Let's only drop the changes that debian adopted, and that udev prio the DM requested, and see what this looks like then.

Okay so I'll
1) add back in:
d/multipath.conf: Install friendly names multipath.conf by default
d/initramfs/hooks: do not copy kpartx rules to initramfs

2) add no socket based activation to d/NEWS

3) verify LP 1673350 still works (no idea how much effort this one will be)

4) rebuild PPA

5) retest basic functionality and multipath-tools-boot

That's a bit of work so it's safe to assume this will miss FF.

Thanks for adding back friendly names. With the installer hat on, I don't fully understand the implications yet but from Dimitri's involvement there is clearly an installer angle to this.

If you take an installed system with rootfs on multipath, does changing the status of friendly names affect that?

Switching to in-progress to do the aforementioned changes.