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

[Stefano Brivio <sbrivio@redhat.com>]

On Tue, 14 Jul 2026 19:29:22 +1000
David Gibson <david@gibson.dropbear.id.au> 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 <david@gibson.dropbear.id.au>
> ---
>  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(). Are patches 1/2 and 2/2 really needed for the
rest of the series? I don't quite see the connection.

I'm still reviewing the rest of the series.

-- 
Stefano