script_run: base serial buffer size on command length #2429
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AdamWill
force-pushed
the
long-serial-command
branch
from
3123dd2
to
48c546c
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2429 +/- ##
==========================================
+ Coverage 95.12% 95.19% +0.07%
==========================================
Files 155 156 +1
Lines 15474 15701 +227
==========================================
+ Hits 14719 14946 +227
Misses 755 755 ☔ View full report in Codecov by Sentry. |
okurz
requested changes
Jan 10, 2024
okurz
added a commit
to okurz/os-autoinst
that referenced
this pull request
Jan 11, 2024
AdamWill
force-pushed
the
long-serial-command
branch
from
48c546c
to
2933539
Compare
|
I don't think the failures have anything to do with the PR (okurz's other PR is failing on the same things). |
If you use script_run at a serial console to run a command more than 4096 characters long, the wait_serial that waits until the whole command has made it across the serial connection will fail, because it uses the default buffer size (4096), so it will never see the full command. This means there'll be a 90 second timeout before the command is actually run, and a red frame in the results. This solves the problem by always sizing the buffer to the length of the command, plus 128 extra bytes (for the marker string, and some more for safety). Issue: https://progress.opensuse.org/issues/153304 Signed-off-by: Adam Williamson <awilliam@redhat.com>
AdamWill
force-pushed
the
long-serial-command
branch
from
2933539
to
2542836
Compare
AdamWill
changed the title
okurz
approved these changes
Jan 12, 2024
okurz
added a commit
to okurz/os-autoinst
that referenced
this pull request
Jan 12, 2024
Yes, sorry about that. @b10n1k was experimenting with more CI checks. I have reverted the according commits for now and will rebase here @Mergifyio rebase |
okurz
added a commit
to okurz/os-autoinst
that referenced
this pull request
Jan 15, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If you use script_run at a serial console to run a command more than 4096 characters long, the wait_serial that waits until the whole command has made it across the serial connection will fail, because it uses the default buffer size (4096), so it will never see the full command. This means there'll be a 90 second timeout before the command is actually run, and a red frame in the results.
This solves the problem by always sizing the buffer to the length of the command, plus 128 extra bytes (for the marker string, and some more for safety).
Issue: https://progress.opensuse.org/issues/153304