From 326ecf3898d2e6256b2992d60a2a6bc6853c918d Mon Sep 17 00:00:00 2001 From: Frank Hunleth Date: Sun, 15 Dec 2024 21:20:29 -0500 Subject: [PATCH] Parse and return fwup ops errors This turns on fwup output framing so that the error output is clean even if there are warnings. --- .credo.exs | 1 + lib/nerves_runtime/fwup_ops.ex | 59 +++++++++++++++++++++--- mix.exs | 3 +- mix.lock | 1 + test/fixture/Makefile | 6 +++ test/fixture/ops-fail.conf | 63 ++++++++++++++++++++++++++ test/fixture/ops-fail.fw | Bin 0 -> 283 bytes test/nerves_runtime/fwup_ops_test.exs | 10 ++++ 8 files changed, 135 insertions(+), 8 deletions(-) create mode 100644 test/fixture/Makefile create mode 100644 test/fixture/ops-fail.conf create mode 100644 test/fixture/ops-fail.fw diff --git a/.credo.exs b/.credo.exs index 709bbed..6854a2c 100644 --- a/.credo.exs +++ b/.credo.exs @@ -5,6 +5,7 @@ name: "default", strict: true, checks: [ + {CredoBinaryPatterns.Check.Consistency.Pattern}, {Credo.Check.Refactor.MapInto, false}, {Credo.Check.Warning.LazyLogging, false}, {Credo.Check.Readability.LargeNumbers, only_greater_than: 86400}, diff --git a/lib/nerves_runtime/fwup_ops.ex b/lib/nerves_runtime/fwup_ops.ex index 560f54d..3296846 100644 --- a/lib/nerves_runtime/fwup_ops.ex +++ b/lib/nerves_runtime/fwup_ops.ex @@ -43,7 +43,7 @@ defmodule Nerves.Runtime.FwupOps do def revert(opts \\ []) do reboot? = Keyword.get(opts, :reboot, true) - with :ok <- run_fwup("revert", opts) do + with {:ok, _} <- run_fwup("revert", opts) do if reboot? do Nerves.Runtime.reboot() else @@ -61,7 +61,7 @@ defmodule Nerves.Runtime.FwupOps do """ @spec prevent_revert(options()) :: :ok | {:error, reason :: any} def prevent_revert(opts \\ []) do - run_fwup("prevent-revert", opts) + run_fwup("prevent-revert", opts) |> ignore_success_results() end @doc """ @@ -75,7 +75,7 @@ defmodule Nerves.Runtime.FwupOps do """ @spec validate(options()) :: :ok | {:error, reason :: any} def validate(opts \\ []) do - run_fwup("validate", opts) + run_fwup("validate", opts) |> ignore_success_results() end @doc """ @@ -92,7 +92,7 @@ defmodule Nerves.Runtime.FwupOps do def factory_reset(opts \\ []) do reboot? = Keyword.get(opts, :reboot, true) - with :ok <- run_fwup("factory-reset", opts) do + with {:ok, _} <- run_fwup("factory-reset", opts) do if reboot? do # Graceful shutdown can cause writes to happen that may undo parts of # the factory reset, so ungracefully reboot to minimize the time @@ -110,15 +110,36 @@ defmodule Nerves.Runtime.FwupOps do with {:ok, ops_fw} <- ops_fw_path(opts), {:ok, fwup} <- fwup_path(opts) do - params = [ops_fw, "-t", task, "-d", devpath, "-q", "-U", "--enable-trim"] + params = [ + "-a", + "-i", + ops_fw, + "-t", + task, + "-d", + devpath, + "-q", + "-U", + "--enable-trim", + "--framing" + ] case System.cmd(fwup, params, cmd_opts) do - {_, 0} -> :ok - {result, _} -> {:error, result} + {results, 0} -> {:ok, results} + {result, _} -> output_to_error(result) end end end + defp output_to_error(raw_result) do + with {:ok, result} <- deframe(raw_result, []) do + Enum.find(result, {:error, "Unknown"}, &find_error/1) + end + end + + defp find_error({:error, _message}), do: true + defp find_error(_status), do: false + defp fwup_path(opts) do fwup_path = opts[:fwup_path] || Application.get_env(:nerves_runtime, :fwup_path, "fwup") @@ -141,4 +162,28 @@ defmodule Nerves.Runtime.FwupOps do true -> {:error, "ops.fw or revert.fw not found in Nerves system"} end end + + defp ignore_success_results({:ok, _}), do: :ok + defp ignore_success_results(other), do: other + + defp deframe(<>, acc) do + case decode(payload) do + {:ok, result} -> deframe(rest, [result | acc]) + {:error, _} -> {:error, "Invalid framing"} + end + end + + defp deframe(<<>>, acc) do + {:ok, Enum.reverse(acc)} + end + + defp deframe(_, _acc) do + {:error, "Invalid framing"} + end + + defp decode(<<"OK", _result::16, _meassage::binary>>), do: {:ok, :ok} + defp decode(<<"ER", _error_code::16, message::binary>>), do: {:ok, {:error, message}} + defp decode(<<"WN", _code::16, meassage::binary>>), do: {:ok, {:warning, meassage}} + defp decode(<<"PR", percent::16>>), do: {:ok, {:progress, percent}} + defp decode(_), do: {:error, "Invalid message"} end diff --git a/mix.exs b/mix.exs index 6344a27..73c931a 100644 --- a/mix.exs +++ b/mix.exs @@ -42,7 +42,8 @@ defmodule Nerves.Runtime.MixProject do {:nerves_uevent, "~> 0.1.0"}, {:ex_doc, "~> 0.22", only: :docs, runtime: false}, {:dialyxir, "~> 1.1", only: :dev, runtime: false}, - {:credo, "~> 1.5", only: :dev, runtime: false} + {:credo, "~> 1.5", only: :dev, runtime: false}, + {:credo_binary_patterns, "~> 0.2.2", only: :dev, runtime: false} ] end diff --git a/mix.lock b/mix.lock index 8520d22..20d381f 100644 --- a/mix.lock +++ b/mix.lock @@ -1,6 +1,7 @@ %{ "bunt": {:hex, :bunt, "1.0.0", "081c2c665f086849e6d57900292b3a161727ab40431219529f13c4ddcf3e7a44", [:mix], [], "hexpm", "dc5f86aa08a5f6fa6b8096f0735c4e76d54ae5c9fa2c143e5a1fc7c1cd9bb6b5"}, "credo": {:hex, :credo, "1.7.10", "6e64fe59be8da5e30a1b96273b247b5cf1cc9e336b5fd66302a64b25749ad44d", [:mix], [{:bunt, "~> 0.2.1 or ~> 1.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2 or ~> 1.0", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "71fbc9a6b8be21d993deca85bf151df023a3097b01e09a2809d460348561d8cd"}, + "credo_binary_patterns": {:hex, :credo_binary_patterns, "0.2.6", "cfcaca0bc5c6447b96c5a03eff175c28f86c486be8e95d55b360fb90c2dd18bd", [:mix], [{:credo, "~> 1.6", [hex: :credo, repo: "hexpm", optional: false]}], "hexpm", "d36a2b56ad72bdf3183ccc81d7e7821e78c97de7c127bc8dd99a5f05ca702187"}, "dialyxir": {:hex, :dialyxir, "1.4.5", "ca1571ac18e0f88d4ab245f0b60fa31ff1b12cbae2b11bd25d207f865e8ae78a", [:mix], [{:erlex, ">= 0.2.7", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm", "b0fb08bb8107c750db5c0b324fa2df5ceaa0f9307690ee3c1f6ba5b9eb5d35c3"}, "earmark_parser": {:hex, :earmark_parser, "1.4.41", "ab34711c9dc6212dda44fcd20ecb87ac3f3fce6f0ca2f28d4a00e4154f8cd599", [:mix], [], "hexpm", "a81a04c7e34b6617c2792e291b5a2e57ab316365c2644ddc553bb9ed863ebefa"}, "elixir_make": {:hex, :elixir_make, "0.9.0", "6484b3cd8c0cee58f09f05ecaf1a140a8c97670671a6a0e7ab4dc326c3109726", [:mix], [], "hexpm", "db23d4fd8b757462ad02f8aa73431a426fe6671c80b200d9710caf3d1dd0ffdb"}, diff --git a/test/fixture/Makefile b/test/fixture/Makefile new file mode 100644 index 0000000..0e43ed9 --- /dev/null +++ b/test/fixture/Makefile @@ -0,0 +1,6 @@ +all: ops-fail.fw ops.fw + +%.fw: %.conf + fwup -c -f $< -o $@ + +.PHONY: all diff --git a/test/fixture/ops-fail.conf b/test/fixture/ops-fail.conf new file mode 100644 index 0000000..2ed0e2b --- /dev/null +++ b/test/fixture/ops-fail.conf @@ -0,0 +1,63 @@ +# Post-installation firmware operations (UNIT TEST VERSION) +# +# Run: fwup -c -f ops-fail.conf -o ops-fail.fw +# +# Tasks include: +# +# * `factory-reset` - Clear out the writable filesystem and any other writable +# areas so that they can be re-initialized on the next boot. +# * `prevent-revert` - Prevent `revert` from working until the next firmware +# * `revert` - Revert to the previous firmware if it's still available +# * `validate` - Mark this firmware as a good update. +# * `status` - Print out which partition is active (`a` or `b`) +# +# To use: +# +# 1. Run `fwup -c -f fwup-ops.conf -o ops.fw` and copy ops.fw to +# the device. This is done automatically as part of the Nerves system +# build process. The file is stored in `/usr/share/fwup/ops.fw`. +# 2. On the device, run `fwup -t -d /dev/rootdisk0 --enable-trim /usr/share/fwup/ops.fw`. +# 3. Reboot after running `revert` or `factory-reset`. + +require-fwup-version="1.0.0" + +## +# factory-reset +## +task factory-reset { + on-init { error("factory-reset error") } +} + +## +# prevent-revert +# +# Pass `--enable-trim` to also clear out the partition that no longer should be used. +## +task prevent-revert { + on-init { error("prevent-revert error") } +} + +## +# revert +## +task revert { + on-init { error("revert error") } +} + +## +# status +# +# Run "fwup /usr/share/fwup/ops.fw -t status -d /dev/rootdisk0 -q -U" to check the status. +## +task status { + on-init { error("status error") } +} + +## +# validate +# +# The fwup configuration for this device always validates, so this doesn't do anything. +## +task validate { + on-init { error("validate error") } +} diff --git a/test/fixture/ops-fail.fw b/test/fixture/ops-fail.fw new file mode 100644 index 0000000000000000000000000000000000000000..f8af7f621c36a4ab7801d51f857adbfe637f20db GIT binary patch literal 283 zcmWIWW@Zs#-~d9QW&M#1NPv@plOZ>?BvCIpKQApbgq4BuOIv(;@0o*KhYWbw9#qd? z^d#r_mwoM;vz!XH7<+0P)qKwt;cw+sZMSc+mw&7+yqNp_o*hj(Qy#HgPn2;pc=a?> z@yBkpe->_zth;B4$huyt<+ePhVzsuQnOmXwk?m~u==Jf%KCI%$ngZ_%-cj8qubyK6 zb-KXUSxehjoE2X5W2Ri(M)sej(rL^A-s~K!pH!}G1v