Merge ~jefferyto/ubuntu/+source/initramfs-tools:ubuntu-core-dev-fix-resume-device-type-check into ~ubuntu-core-dev/ubuntu/+source/initramfs-tools:ubuntu/devel

[Jeffery To]

I had assumed the existing check worked; after some testing I found it does not.

[Benjamin Drung (bdrung)]

I fail to reproduce the misbehavior. `resolve_device` returns with an exit code in case no device was found and `blkid -p -n swap $device` returns with exit code 2 in case that device is not a swap partition.

Can you provide logs showing which command misbehaved?

review: Needs Information

[Jeffery To (jefferyto)]

The case I described in the commit message (blkid will exit with success if it can gather information about the device, even if the device does not match the specified type) applies when the device exists but is not a swap device.

More concretely, if in /etc/initramfs-tools/conf.d/resume you set RESUME= to a device that exists but is not a swap device then run update-initramfs, no error message will be shown and the non-swap device will be saved as the resume device.

[Benjamin Drung (bdrung)]

Okay, I can reproduce the bug. It works on device mapper volumes, but the filter fails on real devices:

```
$ sudo blkid -p -n swap /dev/mapper/system_crypt || echo $?
2
$ sudo blkid --probe --match-types ext4 /dev/nvme0n1p3 || echo $?
/dev/nvme0n1p3: PART_ENTRY_SCHEME="gpt" PART_ENTRY_UUID="01306287-1c29-4032-ac5d-62cefd2f3646" PART_ENTRY_TYPE="c12a7328-f81f-11d2-ba4b-00a0c93ec93b" PART_ENTRY_NUMBER="3" PART_ENTRY_OFFSET="3906054144" PART_ENTRY_SIZE="974848" PART_ENTRY_DISK="259:0"
```

Let's file a bug against util-linux (that ships /usr/sbin/blkid) first.

[Benjamin Drung (bdrung)]

I filed https://bugs.launchpad.net/ubuntu/+source/util-linux/+bug/2071807

[Benjamin Drung (bdrung)]

Response from util-linux upstream: we need to fix initramfs-tools.

review: Needs Fixing

[Jeffery To (jefferyto)]

Adding a comment here to save the diff comment below.

[Benjamin Drung (bdrung)]

Commented below. I would be fine with all variants. Which one do you prefer?

[Jeffery To (jefferyto)]

I prefer neither option because if statements should not have side effects, in this case setting a variable. (Sometimes this is unavoidable with shell scripts, but it should be avoided when possible.) Both options are also less readable than my original code.

Having examined the code for resolve_device, I am convinced it only prints output if it is successful. Is there any condition in resolve_device where it will print output as an error?

[Benjamin Drung (bdrung)]

Okay. I merged your variant, but I added `|| true` to the assignments just in case this script will be executed with `set -e` as one point.

resolve_device only prints on success on stdout. It might print errors on stderr (but I haven't seen that on my local testing).

May I ask you to verify the SRU on noble in bug #1769297 once it landed there?

review: Approve

diff --git a/hooks/resume b/hooks/resume
index 8ff84dd..a13e5a2 100755
--- a/hooks/resume
+++ b/hooks/resume
@@ -23,8 +23,9 @@ if [ -n "$RESUME" ] && [ "$RESUME" != auto ]; then
 	if [ "$RESUME" = none ]; then
 		exit 0
 	fi
-	if resume_dev_node="$(resolve_device "$RESUME")" && \
-	   blkid -p -n swap "$resume_dev_node" >/dev/null 2>&1; then
+	resume_dev_node=$(resolve_device "$RESUME")
+	resume_dev_type=$(blkid -p -s TYPE -o value "$resume_dev_node")
+	if [ "$resume_dev_type" = swap ]; then
 		exit 0
 	fi