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

qemudriver: add support for netdev option in add_port_forward #1391

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aparcar
Copy link
Contributor

@aparcar aparcar commented May 5, 2024

Description

In case there are more user network interfaces defined, allow to specific the netdev. If omitted, an empty string is passed and the behaviour is like before.

This is useful for testing OpenWrt inside a QEMU instance, since OpenWrt expects by default a LAN interface on eth0 and a WAN interface (with uplink) on eth1. Previously the port forward would always be added to eth1, which doesn't support SSH due to firewall policies. By adding the netdev to the forward function, the SSHDriver works as expected.

Checklist

  • Documentation for the feature
  • Tests for the feature
  • The arguments and description in doc/configuration.rst have been updated
  • Add a section on how to use the feature to doc/usage.rst
  • Add a section on how to use the feature to doc/development.rst
  • PR has been tested
  • Man pages have been regenerated

In case there are more `user` network interfaces defined, allow to
specific the netdev. If omitted, an empty string is passed and the
behaviour is like before.

This is useful for testing OpenWrt inside a QEMU instance, since OpenWrt
expects by default a LAN interface on eth0 and a WAN interface (with
uplink) on eth1. Previously the port forward would always be added to
eth1, which doesn't support SSH due to firewall policies. By adding the
netdev to the forward function, the SSHDriver works as expected.

Signed-off-by: Paul Spooren <[email protected]>
@aparcar
Copy link
Contributor Author

aparcar commented May 5, 2024

I'm not sure if this feature is to OpenWrt specific

Copy link

codecov bot commented May 5, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 62.2%. Comparing base (722838b) to head (3858db5).
Report is 16 commits behind head on master.

Files Patch % Lines
labgrid/driver/qemudriver.py 50.0% 2 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff           @@
##           master   #1391   +/-   ##
======================================
  Coverage    62.2%   62.2%           
======================================
  Files         164     164           
  Lines       12191   12198    +7     
======================================
+ Hits         7584    7589    +5     
- Misses       4607    4609    +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -14,6 +14,10 @@ New Features in 24.0
used to pass additional options for the disk directly to QEMU
- labgrid-client now has a ``write-files`` subcommand to copy files onto mass
storage devices.
- The `QEMUDriver` now supports a ``netdev`` argumet which can be added to the
``tadd_port_forward`` in case there are more than one network interfaces
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
``tadd_port_forward`` in case there are more than one network interfaces
``add_port_forward`` in case there are more than one network interfaces

@jluebbe
Copy link
Member

jluebbe commented Jul 17, 2024

Looks good otherwise. Please fix the typo, rebase and mark it as "ready for review".

Copy link
Contributor

@sjg20 sjg20 left a comment

Choose a reason for hiding this comment

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

with typo fixed

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.

4 participants