Skip to content

Commit a48b6fb

Browse files
claywilkinsonCQ Bot
authored andcommitted
[ffx][emu] Assign host ports when using qemu
This adds assigning the host port to a port that is forwarded out of the guest. Older versions of QEMU automatically assigned an unused port to complete the mappings, however QEMU 9 does not do this and so ffx does this as part staging the instance. Bug: 384957072 Change-Id: I2f9ad920459e1c4dfa6caa781a9f47aceb1c4225 Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1178082 Commit-Queue: Clayton Wilkinson <[email protected]> Reviewed-by: Casey Dahlin <[email protected]>
1 parent e499b82 commit a48b6fb

File tree

4 files changed

+64
-5
lines changed

4 files changed

+64
-5
lines changed

src/developer/ffx/plugins/emulator/engines/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ if (is_host) {
2727
"//src/developer/ffx/lib/emulator_instance:lib",
2828
"//src/developer/ffx/lib/errors:lib",
2929
"//src/developer/ffx/lib/fho:lib",
30+
"//src/developer/ffx/lib/port_picker:lib",
3031
"//src/developer/ffx/lib/ssh:lib",
3132
"//src/developer/ffx/lib/target:lib",
3233
"//src/developer/ffx/plugins/emulator/common:ffx_emulator_common",

src/developer/ffx/plugins/emulator/engines/src/lib.rs

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ use emulator_instance::{
1818
};
1919
use errors::ffx_bail;
2020
use ffx_emulator_config::EmulatorEngine;
21-
use fho::{bug, return_user_error, Result};
21+
use fho::{bug, return_bug, return_user_error, Result};
22+
use port_picker::{is_free_tcp_port, pick_unused_port};
2223
use qemu_based::crosvm::CrosvmEngine;
2324
use qemu_based::femu::FemuEngine;
2425
use qemu_based::qemu::QemuEngine;
@@ -207,3 +208,50 @@ pub fn process_flags_from_str(text: &str, emu_config: &EmulatorConfiguration) ->
207208
arg_templates::process_flags_from_str(text, emu_config)
208209
.map_err(|e| bug!("Error processing flags: {e}"))
209210
}
211+
212+
#[allow(dead_code)]
213+
/// Ensures all ports are mapped with available port values, assigning free ports any that are
214+
/// missing, and making sure there are no conflicts within the map.
215+
pub(crate) fn finalize_port_mapping(emu_config: &mut EmulatorConfiguration) -> Result<()> {
216+
let port_map = &mut emu_config.host.port_map;
217+
let mut used_ports = Vec::new();
218+
for (name, port) in port_map {
219+
if let Some(value) = port.host {
220+
if is_free_tcp_port(value).is_some() && !used_ports.contains(&value) {
221+
// This port is good, so we claim it to make sure there are no conflicts later.
222+
used_ports.push(value);
223+
} else {
224+
return_user_error!("Host port {} was mapped to multiple guest ports.", value);
225+
}
226+
} else {
227+
tracing::warn!(
228+
"No host-side port specified for '{:?}', a host port will be dynamically \
229+
assigned. Check `ffx emu show {}` to see which port is assigned.",
230+
name,
231+
emu_config.runtime.name
232+
);
233+
234+
// There have been some incidents in automated tests of the same port
235+
// being returned multiple times.
236+
// So we'll try multiple times and avoid duplicates.
237+
for _ in 0..10 {
238+
if let Some(value) = pick_unused_port() {
239+
if !used_ports.contains(&value) {
240+
port.host = Some(value);
241+
used_ports.push(value);
242+
break;
243+
} else {
244+
tracing::warn!("pick unused port returned: {} multiple times\n", value);
245+
}
246+
} else {
247+
tracing::warn!("pick unused port returned: None\n");
248+
}
249+
}
250+
if !port.host.is_some() {
251+
return_bug!("Unable to assign a host port for '{}'. Terminating emulation.", name);
252+
}
253+
}
254+
}
255+
tracing::debug!("Port map finalized: {:?}\n", &emu_config.host.port_map);
256+
Ok(())
257+
}

src/developer/ffx/plugins/emulator/engines/src/qemu_based/mod.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -583,7 +583,10 @@ pub(crate) trait QemuBasedEngine: EmulatorEngine {
583583
// Capture the port mappings for user mode networking.
584584
let now = fuchsia_async::MonotonicInstant::now();
585585
match self.read_port_mappings().await {
586-
Ok(_) => (),
586+
Ok(_) => {
587+
tracing::debug!("Writing updated mappings");
588+
self.save_to_disk().await?;
589+
}
587590
Err(e) => {
588591
if self.is_running().await {
589592
return Err(e);
@@ -905,8 +908,6 @@ pub(crate) trait QemuBasedEngine: EmulatorEngine {
905908
if modified
906909
&& !self.emu_config().host.port_map.values().any(|m| m.host.is_none())
907910
{
908-
tracing::debug!("Writing updated mappings");
909-
self.save_to_disk().await?;
910911
return Ok(());
911912
}
912913
} else {

src/developer/ffx/plugins/emulator/engines/src/qemu_based/qemu/mod.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ use crate::qemu_based::QemuBasedEngine;
1212
use async_trait::async_trait;
1313
use emulator_instance::{
1414
write_to_disk, CpuArchitecture, EmulatorConfiguration, EmulatorInstanceData,
15-
EmulatorInstanceInfo, EmulatorInstances, EngineState, EngineType, PointingDevice,
15+
EmulatorInstanceInfo, EmulatorInstances, EngineState, EngineType, NetworkingMode,
16+
PointingDevice,
1617
};
1718
use ffx_config::EnvironmentContext;
1819
use ffx_emulator_common::config::QEMU_TOOL;
@@ -62,6 +63,14 @@ impl EmulatorEngine for QemuEngine {
6263
}
6364

6465
async fn stage(&mut self) -> Result<()> {
66+
// QEMU 9+ does not support auto-assignment of host port
67+
// when forwarding guest ports mapped via user networking.
68+
// So we need to assign any missing host ports now, before
69+
// continuing to stage the instance.
70+
if self.emu_config().host.networking == NetworkingMode::User {
71+
crate::finalize_port_mapping(self.emu_config_mut())?;
72+
}
73+
6574
let result = <Self as QemuBasedEngine>::stage(&mut self)
6675
.await
6776
.and_then(|()| self.validate_staging());

0 commit comments

Comments
 (0)