Flash directly via flashcp when sysupgrade's verify-mount fails

[wkumik]

Problem

The local-file firmware flash runs sysupgrade --force_ver -n -z, which loop-mounts the uploaded rootfs to verify it before writing it. On devices whose currently-running kernel doesn't include the squashfs decompressor used by the new image (typically XZ), that verification mount fails:

RootFS
Update rootfs from /tmp/rootfs.squashfs
mount: mounting /dev/loop0 on /tmp/rootfs failed: Invalid argument
Unable to mount /tmp/rootfs! Aborting.

sysupgrade then aborts — but it has already flashed the kernel by that point, so the device reboots into a half-upgraded state (new kernel, old rootfs) and stays on its old firmware. To the user the flash just "didn't take," with no obvious reason.

This happens with perfectly valid images — it depends only on whether the old running kernel happens to include XZ squashfs support, which we don't control and which varies per device.

Fix

Before running sysupgrade, probe the same loop-mount sysupgrade would perform:

• Mountable → run sysupgrade exactly as before (no behavioural change for the vast majority of devices).
• Not mountable → flash the kernel and rootfs partitions directly with flashcp (a raw write needs no mount), erase the rootfs_data overlay to mirror sysupgrade -n, and reboot.

flashcp / flash_eraseall / /proc/mtd parsing are already used elsewhere in Companion (firmware restore, bootloader flash), so this follows existing conventions.

Safety

• Partitions are resolved by name from /proc/mtd (kernel, rootfs, rootfs_data); if kernel/rootfs aren't found, the direct flash aborts rather than guessing a partition.
• Each image is size-checked against its partition before writing, so an oversized file can never overflow into the next partition.
• flashcp presence is verified first.
• The fallback only triggers when the verify-mount fails — precisely the case where sysupgrade cannot succeed anyway. If the probe itself can't run, behaviour falls back to attempting sysupgrade (unchanged).

Tests

Adds SysUpgradeServiceTests covering the /proc/mtd parser (name→device/size mapping, case-insensitive lookup, CRLF handling, empty/garbage input).

Build: 0 errors. Tests: 64 passed.

---

Complements #190 (which makes a genuinely failed flash report an error instead of false "success"); this PR makes the flash actually succeed on the affected devices.

[mikecarr]

what new format? Is OpenIPC changing format?

[wkumik]

No new format — and no, OpenIPC isn't changing anything. That was bad wording in my description, sorry for the confusion.

XZ-compressed squashfs is what OpenIPC images already use (the published ssc338q images — RunCam WiFiLink, generic, apfpv — are all XZ). Nothing about the image side is new or unusual.

The problem is on the other side: the kernel already running on the device you're flashing. sysupgrade loop-mounts the new rootfs to verify it before writing it, and that mount is performed by the old kernel. Most kernels in the field can mount it and everything works — this PR changes nothing for them. Some can't, and there it goes wrong.

Since opening this I've traced the actual root cause, and it's worse than "the mount fails" — so I've raised a fix upstream in the firmware itself: OpenIPC/firmware#2220.

Three things are wrong with that verify-mount, all in its failure path:

1. It runs after the kernel is already written. do_update_kernel is called first, and the rootfs is only verified inside do_update_rootfs afterwards. So a rootfs problem is discovered once the kernel is already committed — the device is left with a new kernel on an old rootfs and doesn't recover on its own.

2. It can hang forever, not just fail. There is no output at all between echo "Update rootfs from $x" and the flash write — everything in between is a silent losetup + mount. So a wedged mount is indistinguishable from a dead tool. We hit exactly this on an SSC338Q air unit; verbatim from the log:

   Kernel updated to 06:49:22 2026-06-15     <- kernel written + verified
   RootFS
   Update rootfs from /tmp/rootfs.squashfs.ssc338q
   <<< zero further output, ever >>>
   

   That's the "stuck at 60% forever" reports — 60% being precisely the kernel-done / rootfs-about-to-start boundary.

3. It's fatal when it doesn't need to be. flashcp writes the partition raw and never needed the mount — sysupgrade already uses flashcp for the write. The mount is only a pre-check, so an image the old kernel can't read is being rejected on a criterion that stops applying the moment the new kernel boots.

#2220 fixes all three at the source (verify before the first write, bound the mount with a timeout, warn instead of aborting), and keeps the SoC check intact — that's what the mount is really there for.

So this PR is not about a format change, and it isn't really the fix anymore either — #2220 is. What this one still buys you is the devices already in the field: they're running the current sysupgrade and won't get the fixed one until they take new firmware, which they can't do if flashing is what's broken. Until then Companion can detect the case and write the partitions directly.

I've also just pushed a fix to this branch for a flaw the same investigation exposed in my own probe: it assumed the mount always fails, when it can hang — so an unbounded probe would have hung in exactly the case it exists to survive. It's now bounded on both sides, and a mount that doesn't answer counts as unmountable and takes the flashcp path.

Happy to reword the description to drop the "new image" phrasing that caused this.

[mikecarr]

Please rework this before merge.

Branch/build process first: PRs for this repo should target dev first, not master. The README and docs/development.md list the expected local checks, especially:

dotnet build Companion.Desktop/Companion.Desktop.csproj -c Release
dotnet test

GitHub Actions remains the release/source-of-truth path for official artifacts, but feature/fix work should be validated into dev before any promotion to master.

Specific issues I found while checking this against current dev:

1. This does not stack cleanly after #190. Applying this PR on top of dev + #190 fails in Companion/Services/SysUpgradeService.cs. Please rebase/rebuild this branch on current dev after #190 and keep #190's failure propagation behavior.

2. As written, PerformSysupgradeAsync still catches exceptions, logs/progresses them, and returns normally. That means direct-flash or sysupgrade failures can still be reported as success by the UI unless #190's rethrow behavior is preserved.

3. The mount probe command uses timeout 45 mount ... || mount .... If the timeout path kills or fails the first mount, this immediately runs a second unbounded mount. That can hang in the exact condition this fallback is meant to avoid. Please change this to use timeout when available, otherwise fall back to a bare mount only when the timeout command is missing, not after every mount failure.

The underlying firmware issue this PR targets is plausible, and the partition-name lookup plus size checks are the right direction. Because this writes raw MTD partitions with flashcp, it needs to be stacked on the failure-reporting fix and have the probe timeout fixed before it is safe to merge.

[mikecarr]

Follow-up on the raw MTD / flashcp fallback:

Companion does already use flashcp, but only in explicit recovery-style flows:

• restore from backup reads /proc/mtd, maps backup files like mtdblock2.bin to /dev/mtd2, then runs flashcp -v '<remotePath>' /dev/mtd{partitionIndex}
• bootloader replacement flashes directly to /dev/mtd0 and erases /dev/mtd1, behind explicit confirmation/warnings
• the docs also describe manual backup restore with flashcp

The normal uploaded firmware upgrade path currently delegates kernel/rootfs flashing to OpenIPC sysupgrade --force_ver -n -z --kernel=... --rootfs=.... So using flashcp here is not foreign to the app, but it changes the safety model: this PR would introduce direct raw MTD flashing as a fallback for the normal firmware upgrade flow.

Before this is safe to merge, I think the fallback needs the same kind of safeguards as the existing restore flow:

• read /proc/mtd on the target device
• map by partition name, not hard-coded assumptions where possible
• validate that kernel/rootfs targets are unambiguous and present
• fail loudly if the layout cannot be verified
• make sure exceptions propagate so the UI never reports success after a failed fallback
• keep the fallback clearly scoped to the sysupgrade verify-mount failure case

This is another reason I would keep this PR separate from #190, retarget/rebase it onto current dev, and revise it before merge.

[wkumik]

Thanks — that's a fair bar to set, and I agree with it: raw MTD writes in the normal upgrade path deserve the same care as the restore flow. I've retargeted and rebased this onto dev (it applies cleanly on top of #190).

Going through your list, I think we're closer than the description made it look — most of it is already in the diff, so let me point at the code rather than just assert it:

Read /proc/mtd on the target device — FlashImageDirectlyAsync starts with cat /proc/mtd and parses it; nothing is hard-coded.

Map by partition name, not hard-coded assumptions — ParseMtdPartitions builds a name → (device, size) map, case-insensitive, and the flash looks up "kernel" / "rootfs" / "rootfs_data" by name. /dev/mtd2 and /dev/mtd3 appear nowhere in the code; they only show up in a test fixture as the expected output of parsing a real SSC338Q table.

Validate that kernel/rootfs targets are unambiguous and present — if either name is missing we abort before writing anything:

if (!partitions.TryGetValue("kernel", out var kernelPartition) ||
    !partitions.TryGetValue("rootfs", out var rootfsPartition))
    throw new InvalidOperationException(
        "Could not find 'kernel' and 'rootfs' partitions in /proc/mtd; aborting direct flash to avoid writing the wrong partition.");

There's also a size check — an image larger than its partition is refused rather than allowed to run into the neighbouring one — and a flashcp presence check, both before any write. The overlay erase (the -n equivalent) is best-effort and skipped if rootfs_data isn't there.

Fail loudly if the layout cannot be verified — every one of those paths throws.

Make sure exceptions propagate so the UI never reports success after a failed fallback — this one is genuinely better now that #190 has landed. The outer catch in PerformSysupgradeAsync rethrows, so anything thrown above reaches FirmwareTabViewModel and surfaces as a real failure dialog. Before #190 it would have been swallowed and painted green. The two changes compose exactly the way you'd want.

Keep the fallback clearly scoped to the verify-mount failure case — it's only reachable from the else of the mount probe. If the running kernel can mount the image, sysupgrade runs unchanged and nothing about this PR is observable.

Agreed on keeping it separate from #190, and that's how it's structured.

One note on validation: dotnet test is no longer blocked on dev — the WfbTabViewModelTest wiring issue you hit was fixed upstream in 6af79e9. On the rebased branch I get a clean Release build and 63/63 passing, including the /proc/mtd parser tests (real table, CRLF, garbage input, case-insensitive lookup).

Worth saying plainly: I'd rather this PR became unnecessary than see it merged for its own sake. The actual bug is in sysupgrade and I've fixed it there in OpenIPC/firmware#2220 — verify the rootfs before the kernel is written, bound the mount with a timeout, warn instead of aborting. If that lands, new firmware stops creating these devices. This PR only exists for the units already in the field, which are running today's sysupgrade and can't receive the fixed one because flashing is the thing that's broken. That's the loop we're trying to break, and it's why people are currently recovering their air units by hand with wget + flashcp over SSH.

Happy to make any further changes you'd like, or to reword the description that caused the original confusion.

[mikecarr]

Thanks for rebasing this onto dev and for adding the /proc/mtd parser tests. I re-reviewed the branch locally against current dev. Validation passes on my side too:\n\n- dotnet test -v minimal -clp:ErrorsOnly: 63/63 passed\n- dotnet build Companion.Desktop/Companion.Desktop.csproj -c Release -clp:ErrorsOnly: passed after the usual Avalonia telemetry filesystem rerun\n\nI still do not think this is safe to merge yet. Two blocking issues remain in the direct-flash path:\n\n1. The direct flash commands can hang forever even when flashcp succeeds.\n\n In FlashImageDirectlyAsync, both flashcp calls use ExecuteCommandWithProgressAsync with disableTimeout: true and no custom isCommandComplete predicate. That helper's default completion predicate only looks for sysupgrade-style strings ("Unconditional reboot" / "Arguments written"), and a normal flashcp command does not disconnect the SSH session. With timeout disabled, the loop has no way to decide that flashcp completed, so the first flashcp can leave Companion waiting forever and never proceed to rootfs/reboot.\n\n Related: ExecuteCommandWithProgressAsync reports timeout/errors via progress text and returns; it does not throw. For raw MTD flashing we need command exit/failure to be explicit and propagated, not just displayed. Please either run these through a command API that gives an exit status and throw on non-zero, or add a reliable sentinel/exit-code wrapper plus a finite timeout and failure propagation.\n\n2. The mount probe still has the unbounded second mount path.\n\n The generated command is still effectively:\n\n timeout 45 mount ... || mount ...\n\n That means if timeout exists and the first mount times out or fails, the shell immediately starts a second bare mount. The client-side Task.WhenAny stops waiting after ~65s, but the remote command can still be running in the background while Companion starts direct flashcp. That is exactly the situation this probe is supposed to avoid.\n\n Please only use the bare mount as a fallback when the timeout command itself is missing, not after any mount failure/timeout. Something shaped like command -v timeout && timeout ... || mount ... still needs care because the || branch will also run for mount failure; it has to branch on timeout availability separately.\n\nThe overall approach is closer now: retargeting to dev, stacking on #190, partition-name lookup, size checks, and parser tests are all the right direction. But because this path writes raw MTD partitions, the command completion/failure semantics and the probe timeout behavior need to be correct before merge.

[wkumik]

Thanks for the careful re-review @mikecarr — both blocking issues are fixed in 53a992e.

1. Direct flash could hang / swallow failures. The two flashcp calls no longer go through ExecuteCommandWithProgressAsync with disableTimeout: true and the default sysupgrade-only completion predicate. They now run through a new FlashPartitionAsync helper that:

• appends ; echo RUBY_FLASHCP_EXIT:$? so flashcp's real exit status comes back over the shell stream;
• completes the instant that marker is parsed with a digit (isCommandComplete: _ => exitCode.HasValue) — the shell's own command echo contains the literal $? with no digit, so it can't be mistaken for completion;
• keeps a finite timeout (disableTimeout: false, 5 min kernel / 15 min rootfs);
• throws on a non-zero exit code and on no marker at all (timeout or dropped connection), so a half-written flash can never pass silently.

2. Unbounded second mount in the probe. Replaced timeout N mount … || mount … with a branch on the applet itself:

if command -v timeout >/dev/null 2>&1; then M="timeout N"; else M=""; fi
if $M mount -t squashfs -o loop,ro '…' "$d"; then echo RUBY_MOUNT_OK; …

Exactly one mount runs — bounded when timeout exists, bare only when the applet is genuinely absent (where the client-side wall-clock bound still covers it). No second mount fires on mount failure/timeout, and it avoids the A && B || C trap you flagged.

Validation on my side:

• dotnet test Companion.Tests -v minimal -clp:ErrorsOnly: 63/63 passed
• dotnet build Companion.Desktop -c Release -clp:ErrorsOnly: 0 errors

Not yet re-tested against physical hardware — flagging that since this is the raw-MTD path.

[mikecarr]

Re-reviewed the updated head (53a992e) against current dev. The two prior blockers are addressed in code now.\n\nValidated locally:\n\n- dotnet test -v minimal -clp:ErrorsOnly: 63/63 passed\n- dotnet build Companion.Desktop/Companion.Desktop.csproj -c Release -clp:ErrorsOnly: passed after the usual Avalonia telemetry filesystem rerun\n\nSpecific fixes checked:\n\n- direct flashcp now uses a finite-timeout FlashPartitionAsync helper, parses an explicit RUBY_FLASHCP_EXIT marker, and throws on non-zero/no marker instead of silently continuing\n- the mount probe now branches on timeout availability and no longer runs timeout mount || bare mount after every mount failure\n- PR remains targeted to dev and stacked on the #190 failure-propagation fix\n\nI am comfortable merging this into dev from a code review standpoint. Remaining caveat: the raw-MTD fallback path still deserves physical-device validation before promoting dev to master/release.