Hi Chengen, Below is my initial review of the patches. I think there is a lot that can be done here to simplify and reduce noise, so that only the critical logic changes are included.
> lp2100252-0001-sd-device-introduce-sd_device_new_from_devname.patch 1. The sd_device_new_from_path() function is never use. Please drop that change. 2. Let's not add sd_device_new_from_devname() to the public API here. While that is the upstream change, I would like to avoid adding symbols to libsystemd in this SRU. Instead, please move the function to src/libsystemd/sd-device/device-private.c, and call it device_new_from_devname() or so. > lp2100252-0002-core-device-store-the-original-path.patch I think this patch is fine as-is. It's a preparation commit, and is required for the real changes later. > lp2100252-0003-core-device-move-several-functions.patch This patch appears to just move code around. Is this needed at all? If it is needed to compile correctly, can this instead be accomplished with forward declarations? That would be a far less noisy patch. > lp2100252-0004-core-device-always-accept-syspath-change.patch This seems reasonable from a first glance, but I will take a closer look on a later review. Also, it's up to you, but note that we do manage the systemd packaging in git at https://git.launchpad.net/~ubuntu-core- dev/ubuntu/+source/systemd. It may be more convenient for review purposes to open PRs, rather than using debdiffs. ** Changed in: systemd (Ubuntu Focal) Status: In Progress => Incomplete ** Changed in: systemd (Ubuntu Jammy) Status: In Progress => Incomplete -- You received this bug notification because you are a member of Ubuntu Touch seeded packages, which is subscribed to systemd in Ubuntu. https://bugs.launchpad.net/bugs/2100252 Title: A reused mount point is removed after deactivating the original volume group Status in systemd package in Ubuntu: Fix Released Status in systemd source package in Focal: Incomplete Status in systemd source package in Jammy: Incomplete Status in systemd source package in Noble: Fix Released Status in systemd source package in Oracular: Fix Released Status in systemd source package in Plucky: Fix Released Bug description: [Impact] A mount point was originally used by the old volume group. After replacing it with a new volume group and deactivating the old one, the mount point, now belonging to the new volume group, gets unmounted accidentally. [Fix] A patch set has been introduced to fix this issue: https://github.com/systemd/systemd/pull/23508 The related patches are as follows: 367a2597c351 core/device: store the original path dce2d35ce53d core/device: move several functions 4a1a1caf2192 core/device: always accept syspath change [Test Plan] 1. Set up a virtual machine with two volume groups: vg1 and vg2. root@lvm-focal:~# lsblk NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT loop0 7:0 0 91.9M 1 loop /snap/lxd/24061 loop1 7:1 0 49.9M 1 loop /snap/snapd/18357 loop2 7:2 0 63.3M 1 loop /snap/core20/1828 loop3 7:3 0 44.4M 1 loop /snap/snapd/23545 loop4 7:4 0 63.8M 1 loop /snap/core20/2496 loop5 7:5 0 91.9M 1 loop /snap/lxd/29619 sr0 11:0 1 1024M 0 rom vda 252:0 0 500G 0 disk ├─vda1 252:1 0 1M 0 part ├─vda2 252:2 0 2G 0 part /boot └─vda3 252:3 0 498G 0 part └─ubuntu--vg-ubuntu--lv 253:2 0 100G 0 lvm / vdb 252:16 0 20G 0 disk ├─vdb1 252:17 0 9.5G 0 part │ └─vg1-lvol0 253:0 0 2G 0 lvm └─vdb2 252:18 0 10.5G 0 part └─vg2-lvol0 253:1 0 2G 0 lvmt 2. Add a mount point entry in /etc/fstab. /dev/mapper/vg1-lvol0 /mnt/xfs xfs defaults 0 0 3. Run `mount -av` to mount the specified mount point. 4. Unmount the mount point and rename the volume groups. umount /mnt/xfs vgrename vg1 vg1_old vgrename vg2 vg1; 5. Run `mount -av` again and verify that the mount point is successfully mounted. root@lvm-focal:~# lsblk NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT loop0 7:0 0 91.9M 1 loop /snap/lxd/24061 loop1 7:1 0 49.9M 1 loop /snap/snapd/18357 loop2 7:2 0 63.3M 1 loop /snap/core20/1828 loop3 7:3 0 44.4M 1 loop /snap/snapd/23545 loop4 7:4 0 63.8M 1 loop /snap/core20/2496 loop5 7:5 0 91.9M 1 loop /snap/lxd/29619 sr0 11:0 1 1024M 0 rom vda 252:0 0 500G 0 disk ├─vda1 252:1 0 1M 0 part ├─vda2 252:2 0 2G 0 part /boot └─vda3 252:3 0 498G 0 part └─ubuntu--vg-ubuntu--lv 253:2 0 100G 0 lvm / vdb 252:16 0 20G 0 disk ├─vdb1 252:17 0 9.5G 0 part └─vdb2 252:18 0 10.5G 0 part └─vg1-lvol0 253:1 0 2G 0 lvm /mnt/xfs 6. Deactivate the old volume group and ensure that the mount point remains available. vgchange -an vg1_old [Where problems could occur] This patch set modifies the handling of devlink. If any regressions occur, the device unit may become non-functional. To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/systemd/+bug/2100252/+subscriptions -- Mailing list: https://launchpad.net/~touch-packages Post to : touch-packages@lists.launchpad.net Unsubscribe : https://launchpad.net/~touch-packages More help : https://help.launchpad.net/ListHelp
