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

Need a better way to enforce sequence of code update steps #6

Closed
anoo1 opened this issue Jul 29, 2020 · 5 comments
Closed

Need a better way to enforce sequence of code update steps #6

anoo1 opened this issue Jul 29, 2020 · 5 comments

Comments

@anoo1
Copy link
Contributor

anoo1 commented Jul 29, 2020

Some code update functions use systemd service files to execute scripts and perform Delete and Write operations. But these systemd service files aren't been monitored for completion so race conditions can occur. As a workaround, sleep() have been added to some of the functions.

Opening this issue to explore better options than arbitrary sleep calls, since they still not guaranteed that the service has finished, it's just a value based on observation.

Some ideas in no particular order:

@leiyu-bytedance
Copy link
Contributor

Is it possible to add some Wants/Before/After configs in the unit files to solve the issue?
The good part is that the C++ code does not have to call the system units one-by-one anymore, instead, it calls one systemd unit, and the unit will start other systemd units by sequence.

@bradbishop
Copy link
Member

bradbishop commented Aug 3, 2020

The good part is that the C++ code does not have to call the system units one-by-one

If we have scripts calling other scripts I wonder if that could be simplified or reduced down to a single script?

Are there any other benefits besides the one above to using systemd over a more conventional implementation for launching scripts/programs, such as this one:
https://github.com/openbmc/jsnbd/blob/master/nbd-proxy.c#L379

@anoo1
Copy link
Contributor Author

anoo1 commented Aug 3, 2020

Is it possible to add some Wants/Before/After configs in the unit files to solve the issue?

Partially. The remaining issue is that we want the function to return until the script (run by a service file) is complete. For example a Delete or Set Priority call would not return to the caller until the operations finished.

Are there any other benefits besides the one above to using system over a more conventional implementation for launching scripts/programs

Hadn't thought of that. That could be an option, instead of executing the script via a service file.

@anoo1
Copy link
Contributor Author

anoo1 commented Jan 20, 2021

Adding function for running commands in child process:
https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-bmc-code-mgmt/+/39844

geissonator pushed a commit that referenced this issue Jan 28, 2021
As issue #6 describes, the code
makes use of the systemd services to execute commands via a
script, but calls to systemd service files are async, so if
there is a need to wait for the service to finish executing,
workarounds like sleep commands have been added to the code, ex:
60f5ccf

Create a function that would execute a command in a child
process so that it's possible to wait for it to finish. In
addition, errors can be more obvious by checking the return
of the execute function and taking action to notify the caller
of the error instead of relying on system unit dependencies
to run recovery actions on error.

Tested: Called new execute function from the code and verified
        it was successful. Ex:
        utils::execute("/bin/mkdir", "/tmp/test-execute");

Change-Id: I81a6aa0a50276abb6aba40196a214629ec9baa13
Signed-off-by: Adriana Kobylak <anoo@us.ibm.com>
@anoo1
Copy link
Contributor Author

anoo1 commented Jan 28, 2021

Closing. First function (factory reset) migrated to exec instead of using systemd service files.

@anoo1 anoo1 closed this as completed Jan 28, 2021
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

No branches or pull requests

3 participants