Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent attempting to firmware update while another in progress #48

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jjcarstens
Copy link
Member

@jjcarstens jjcarstens commented May 29, 2022

Attempting to run multiple firmware updates at the same time could potentially be bad. So this adds a quick check if the fwup port is running and returns a more useful error to the SSH connection if it is

Fixes #36

@@ -210,6 +211,14 @@ defmodule SSHSubsystemFwup do
end
end

defp check_fwup_not_running() do
if Enum.any?(Port.list(), &("#{Port.info(&1)[:name]}" =~ ~r/fwup/)) do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, this is clever enough that I feel compelled to think if there's another way. But, I can definitely see how this works and prevents two instances of fwup running.

Fwiw, the best other idea I have at the moment would be for fwup to create a lock file based on the output when it runs.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya, I had the same thought, researched a bit, then decided to just start with the easiest

We could also just wrap this port process in a singleton genserver or something as well which would control fwup processes. I'm open to whatever.

It seems like checking for a running fwup port process is going to be just as effective as checking for a file on disk. The file seems like more risk to me in case the fwup process crashes without cleaning it up and thinks look to still be running (but I might guess you know the C way to get around that 😂)

@joshk
Copy link

joshk commented Jan 8, 2024

@jjcarstens @fhunleth is this issue and PR still relevant?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ssh fwup vs. nerves_hub fwup race condition?
3 participants