Re: [PATCH 3/9] conf: Detect vhost-user mode earlier

[Stefano Brivio <sbrivio@redhat.com>]

On Tue, 11 Mar 2025 17:03:12 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> We detect our operating mode in conf_mode(), unless we're using vhost-user
> mode, in which case we change it later when we parse the --vhost-user
> option.  That means we need to delay parsing the --repair-path option (for
> vhost-user only) until still later.
> 
> However, there are many other places in the main option parsing loop which
> also rely on mode.  We get away with those, because they happen to be able
> to treat passt and vhost-user modes identically.  This is potentially
> confusing, though.  So, move setting of MODE_VU into conf_mode() so
> c->mode always has its final value from that point onwards.
> 
> To match, we move the parsing of --repair-path back into the main option
> parsing loop.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  conf.c | 43 ++++++++++++++++++++++++++-----------------
>  1 file changed, 26 insertions(+), 17 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index 2022ea1d..b58e2a6e 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -998,10 +998,23 @@ pasta_opts:
>   *
>   * Return: mode to operate in, PASTA or PASST
>   */
> -/* cppcheck-suppress constParameter */
>  enum passt_modes conf_mode(int argc, char *argv[])
>  {
> +	int vhost_user = 0;
> +	const struct option optvu[] = {
> +		{"vhost-user",	no_argument,		&vhost_user,	1 },
> +		{ 0 },
> +	};
>  	char argv0[PATH_MAX], *basearg0;
> +	int name;
> +
> +	optind = 0;
> +	do {
> +		name = getopt_long(argc, argv, "-:", optvu, NULL);
> +	} while (name != -1);
> +
> +	if (vhost_user)
> +		return MODE_VU;
>  
>  	if (argc < 1)
>  		die("Cannot determine argv[0]");
> @@ -1604,9 +1617,8 @@ void conf(struct ctx *c, int argc, char **argv)
>  
>  			die("Invalid host nameserver address: %s", optarg);
>  		case 25:
> -			if (c->mode == MODE_PASTA)
> -				die("--vhost-user is for passt mode only");

This check should now be moved to conf_mode() instead of being dropped,
otherwise you can do:

$ ./pasta -f --vhost-user

and at this point, the mode is MODE_VU, so it's all fine, but I don't
think it's intended (...or is it?).

> -			c->mode = MODE_VU;
> +			/* Already handled in conf_mode() */
> +			ASSERT(c->mode == MODE_VU);
>  			break;
>  		case 26:
>  			vu_print_capabilities();

Pre-existing, but now we can fix this: case 26 (--print-capabilities)
should only be accepted if (c->mode == MODE_VU).

It can also be done in another patch I would say, if you don't want to
re-spin this.

-- 
Stefano