[Bug 2100252] Re: A reused mount point is removed after deactivating the original volume group

[Nick Rosbrook]

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
Foundations Bugs, 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 at 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 at 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