Skip to content

Add VSOCK option for X410 setup#404

Open
crramirez wants to merge 2 commits intomasterfrom
g65oya-codex/modify-pengwin-setup-for-vsocks-support
Open

Add VSOCK option for X410 setup#404
crramirez wants to merge 2 commits intomasterfrom
g65oya-codex/modify-pengwin-setup-for-vsocks-support

Conversation

@crramirez
Copy link
Collaborator

Summary

  • allow selecting TCP or VSOCK connection when configuring X410
  • cover both connection types with unit tests

Testing

  • ./tests/x410.sh
  • shellcheck pengwin-setup.d/x410.sh rpm/pengwin-setup.d/x410.sh tests/x410.sh

https://chatgpt.com/codex/tasks/task_e_68b849b703188327985009959acde288

Copilot AI review requested due to automatic review settings September 3, 2025 14:47
@crramirez crramirez changed the base branch from codex to development September 3, 2025 14:55
@crramirez crramirez changed the base branch from development to master September 3, 2025 14:55
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds VSOCK connection support for X410 configuration, allowing users to choose between TCP and VSOCK connection types when setting up X410. The change introduces command-line arguments and interactive menu selection for connection type configuration.

  • Adds VSOCK and TCP command-line argument parsing for connection type selection
  • Implements interactive menu for connection type selection when no arguments provided
  • Creates separate configuration files based on selected connection type

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
pengwin-setup.d/x410.sh Adds connection type parsing and generates different X410 configurations for TCP vs VSOCK
rpm/pengwin-setup.d/x410.sh RPM variant with same connection type selection logic and configuration generation
tests/x410.sh Updates test coverage to verify both TCP and VSOCK connection configurations

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.


if [[ ${connection} == VSOCK ]]; then
sudo bash -c 'cat > /etc/profile.d/02-x410.sh' << EOP
#!/bin/sh
Copy link

Copilot AI Sep 3, 2025

Choose a reason for hiding this comment

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

Inconsistent shebang usage. The VSOCK configuration uses #!/bin/sh while the TCP configuration uses #!/bin/sh as well, but the main script uses bash features. Consider using #!/bin/bash for consistency since the script uses bash-specific features like [[ ]].

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As you can see the script is going into /etc/profile.d for maximum flexibility with zsh, those scripts must be tied to posix shell


if [[ ${connection} == VSOCK ]]; then
sudo bash -c 'cat > /etc/profile.d/02-x410.sh' << EOP
#!/bin/bash
Copy link

Copilot AI Sep 3, 2025

Choose a reason for hiding this comment

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

Inconsistent shebang between pengwin-setup.d and rpm versions. The RPM version uses #!/bin/bash for VSOCK while pengwin-setup.d uses #!/bin/sh. These should be consistent across both implementations.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant