Code review comment for lp:~jderose/ecryptfs/fix-1597154

Revision history for this message
Tyler Hicks (tyhicks) wrote :

Hi Jason - the code changes look good. I made a small change to replace the sed regexes special '/' characters with ':' characters to make things a little more readable (the other call to sed in ecryptfs-setup-swap does that, as well).

That said, manual testing is not working as expected. I am testing on a freshly installed amd64 VM that was installed using GPT partitioning on an nvme drive. QEMU is being started like so:

$ qemu-system-x86_64 -enable-kvm -m 1G -vga qxl \
  -drive if=pflash,format=raw,readonly,file=/usr/share/OVMF/OVMF_CODE.fd \
  -drive if=pflash,format=raw,file=OVMF_VARS.fd \
  -drive if=none,format=qcow2,id=nvme0,file=drive.qcow2 \
  -device nvme,drive=nvme0,serial=1234

After running `sudo ecryptfs-setup-swap` and rebooting, I'm seeing /dev/mapper/cryptswap1 being populated but `swapon -s` and `free` shows that no swap partitions are enabled. If I run `sudo swapon -a` then the swap partition is enabled.

I'm trying to debug what's going on here but thought I'd mention it to you to see if you have gotten different results.

review: Needs Information

« Back to merge proposal