Re: [PATCH 2/6] isolation, conf: Set c->fd_tap from eary parse of --fd

On Wed, Jul 15, 2026 at 09:12:35AM +0200, Stefano Brivio wrote:

On Tue, 14 Jul 2026 19:29:22 +1000 David Gibson wrote:

...

We parse --fd twice: once in isolate_initial() just to avoid clobbering the passed in fd. Then we parse it "for real" in conf(), to set c->fd_tap and other configuration variables.

Change this, so that we return the value parsed early from isolate_initial() and set c->fd_tap from that. This doesn't accomplish much immediately, but will make some further cleanups possible.

Signed-off-by: David Gibson --- conf.c | 2 +- isolation.c | 6 +++++- isolation.h | 2 +- passt.c | 2 +- 4 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/conf.c b/conf.c index 6d83daef..41c9d557 100644 --- a/conf.c +++ b/conf.c @@ -1593,7 +1593,7 @@ void conf(struct ctx *c, int argc, char **argv) c->fd_control_listen = c->fd_control = -1; break; case 'F': - c->fd_tap = conf_tap_fd(optarg); + /* --fd was parsed early and c->fd_tap set in main() */ c->one_off = true; *c->sock_path = 0; break; diff --git a/isolation.c b/isolation.c index 07b281bb..4bb79e34 100644 --- a/isolation.c +++ b/isolation.c @@ -203,8 +203,10 @@ static int move_root(void) * - close all open files except for standard streams and the one from --fd * Mustn't: * - remove filesystem access (we need to access files during setup) + * + * Return: fd number from --fd, or -1 if not specified

I think this is a rather weird value to return for a function that's called isolate_initial().

Agreed, but there are reasons..

Are patches 1/2 and 2/2 really needed for the rest of the series? I don't quite see the connection.

1) (minor) The new semantics of conf_tap_fd() introduced in 3/6 work better (IMO) for its use in close_open_files(), but don't work for its reuse during the "main" arg parse. Removing that second parse avoids the problem. 2) (major) 4/6 significantly simplifies the close_range() logic, but it involves *moving* the tap descriptor given with --fd (using dup2(2). That means if we parsed it again, it would now be wrong. I think the weirdness would also be mitigated, if close_open_files() became isolate_fds(), and was called directly from main() rather than via isolate_initial().

I'm still reviewing the rest of the series.

-- Stefano

-- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson