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.

Sign up for GitHub

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

Jump to bottom

Fix: tty not found error for COM passthrough #4391

Merged
merged 2 commits into from
Oct 22, 2024
Merged

Fix: tty not found error for COM passthrough #4391

merged 2 commits into from
Oct 22, 2024

Conversation

roja-zededa
Copy link
Contributor

Adding agetty in build pkgs xen-tools/Dockerfile to fix "tty not found" error

eriknordmark
eriknordmark approved these changes Oct 21, 2024
View reviewed changes
Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

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

LGTM

@eriknordmark eriknordmark added the stable Should be backported to stable release(s) label Oct 21, 2024
@roja-zededa
Fix: tty not found error for COM passthrough
eed0c1b 
Adding agetty in build pkgs xen-tools/Dockerfile to fix "tty not found" error (1)
Removing "tty" backend driver and replacing it with "serial" in kvm.go (2)

Signed-off-by: Roja Eswaran <roja@zededa.com>
@roja-zededa roja-zededa changed the title Add pkg agetty Fix: tty not found error for COM passthrough Oct 21, 2024
@roja-zededa
Fixing kvm_test.go
a7c135c 
Replaced backend tty with serial in the unit tests as the newer qemu version doesn't
recognize "tty".

Signed-off-by: Roja Eswaran <roja@zededa.com>
@roja-zededa roja-zededa self-assigned this Oct 21, 2024
@roja-zededa
Copy link
Contributor Author

@eriknordmark Could you please kickoff the remaining tests?

@eriknordmark
Copy link
Contributor

@eriknordmark Could you please kickoff the remaining tests?

It passed all the eden tests earlier today. See https://github.com/lf-edge/eve/actions/runs/11445356764?pr=4391
But still running the tests which actually do serial port passthrough. Should have results in an hour or so.

@eriknordmark
Copy link
Contributor

It failed with the same error as before.
But I wonder if the issue that backend = "tty" should now be backend = "serial". I'm giving that a try.

@roja-zededa
Copy link
Contributor Author

roja-zededa commented Oct 21, 2024 •
edited
Loading

It failed with the same error as before. But I wonder if the issue that backend = "tty" should now be backend = "serial". I'm giving that a try.

This PR fixes the "tty not found " error in my local node where an app tries to have a direct attachment to the COM serial port. Coud you please share the error trace?

@eriknordmark
Copy link
Contributor

FWIW to get it to work on the devices in the lab we use for regression testing we need the backend=serial. Note that when running on a virtual platform we use backend=serial already,
So that might be why you are not seeing the second issue?

eriknordmark
eriknordmark approved these changes Oct 22, 2024
View reviewed changes
Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

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

LGTM

@eriknordmark eriknordmark merged commit 74c7469 into lf-edge:master Oct 22, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eriknordmark eriknordmark eriknordmark approved these changes

@OhmSpectator OhmSpectator Awaiting requested review from OhmSpectator OhmSpectator is a code owner

@rene rene Awaiting requested review from rene rene is a code owner

@rucoder rucoder Awaiting requested review from rucoder

@shjala shjala Awaiting requested review from shjala

@rouming rouming Awaiting requested review from rouming rouming is a code owner

Assignees

@roja-zededa roja-zededa

Labels
stable Should be backported to stable release(s)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@roja-zededa @eriknordmark