Make fu-powerd-plugin respect suspend_announced

[suprsrsly]

The ChromeOS daemon powerd will create a suspend_announced file when it begins suspending the system. Although the fu-powerd-plugin creates a lockfile to keep powerd from initiating suspend, it doesn't stop what it's doing if powerd has already started the suspend.

This patch checks for the suspend_announced file after creating its lockfile. If the suspend_announced file exists, clean up the lockfile, wait until the suspend is finished, and then proceed.

I've tested this like so:

1. Obtained a Coral Robo360 Chromebook (Lenovo 500e, Intel APL)
2. Applied this patch on top of Chrome OS around 15795.0.0
3. Built a test image and deployed it to my device.
4. Logged in and disabled rootfs verification with

for p in 2 4; do /usr/share/vboot/bin/make_dev_ssd.sh --remove_rootfs_verification --partitions $p; done

5. Modified /etc/init/fwupd.conf to mount /run/power_manager/power within the minijail chroot:

    exec syslog-cat --identifier="${UPSTART_JOB}" --severity_stderr=info \
     -- minijail0 --uts -l -p -N \
     -v -P /mnt/empty -b / -b /proc -t -r -b /dev,,1 -b /sys,,1 \
     -k /var,/var,tmpfs -b /var/cache/fwupd,,1 -b /var/lib/fwupd,,1 \
     -k run,/run,tmpfs -b /run/dbus -b /run/lock,,1 -b /run/shill \
     -b /run/dns-proxy -b /run/udev/data,,1 -b /var/lib/timezone \
+    -b /run/power_manager/power/ \
     ${efi_mounts} \
     -u fwupd -g fwupd \
     -G -c cap_sys_rawio+e \
     -- /usr/libexec/fwupd/fwupd --verbose --no-timestamp

6. Created the file /run/power_manager/power/suspend_announced
7. Started watching fwupd.log with tail -f /var/log/fwupd.log &
8. Started the Chrome OS test firmware.FwupdPowerdUpdateCheck.ac_powerpresent
   (https://chromium.googlesource.com/chromiumos/platform/tast-tests/+/f7c6b869b79515cb4e01bad80be8c895770a796a/src/go.chromium.org/tast-tests/cros/local/bundles/cros/firmware/fwupd_powerd_update_check.go)
9. Watched for the following logline in fwupd.conf and saw the update pause:

   "FuPluginPowerd       waiting due to presence of /run/power_manager/power/suspend_announced"

10. Deleted /run/power_manager/power/suspend_announced
11. Watched the update immediately continue:

   "FuPluginPowerd       suspend blocked"

Type of pull request:

• New plugin (Please include new plugin checklist)
• Code fix
• Feature
• Documentation

[hughsie]

Rather than being "reactive" I'd much rather this was "proactive" -- i.e. rather than starting the update and then aborting after the file has been downloaded, decompressed, parsed and the device put into update mode -- I'd much rather the daemon didn't even try to deploy the update in the first place. If you create a GFileMonitor pointing to this file, you can connect to the changed signal and then add/remove the FU_CONTEXT_FLAG_SYSTEM_INHIBIT flag from the per-system context instead.

[suprsrsly]

Hi hughsie,

I'm not sure if it's clear but this new code won't abort anything - it just hangs inside the loop in fu_powerd_plugin_block_suspend until that file at POWERD_SUSPEND_ANNOUNCED_PATH goes away.

But to your bigger point about registering a file watcher -

I'm concerned about this race condition:

• fwupd: starts up
• fwupd: GFileMonitor begins watching for suspend_announced
• fwupd: begins an update
• powerd: starts a suspend, creating suspend_announced
• fwupd: fu-powerd-plugin creates a lockfile
• fwupd: starts uploading firmware to a device
• powerd: suspends the system
• (the system resumes)
• GFileMonitor notices suspend_announced, and the callback sets FU_CONTEXT_FLAG_SYSTEM_INHIBIT (albeit too late)

(I don't know how GFileMonitor works, so maybe my assumptions about pathological timing don't apply - let me know)

WDYT?

[hughsie]

fwupd: begins an update
powerd: starts a suspend, creating suspend_announced

It's not possible to prevent this race unless fwupd can tell the system before starting "don't suspend" -- i.e. it has to come from the initiator rather than the executor. See https://github.com/fwupd/fwupd/blob/main/plugins/logind/fu-logind-plugin.c#L71 for what we do with logind.

Anything that's coming from the executor to the initiator has to either wait for a "reply", saying "yes, suspend" or "no, busy". Blocking in the plugin mid-update is a bad idea and means that everything depending on the mainloop running is frozen. For instance, it would be very easy to get into a live-lock condition if anything even indirectly was watching fwupd for a DBus signal before continuing with the power save action.

[suprsrsly]

Sorry for going quiet, just an update: I'm going to start looking at seeing how we can coordinate a handshake with fwupd like the way logind does as you've suggested. I'll write back here with any questions that I have (or let me know if there is a better place to ask them).

[hughsie]

Here is fine, thanks.