Re: [kvm-unit-tests PATCH v2 04/18] run_tests: Introduce unittest parameter 'qemu_params'

[Andrew Jones <andrew.jones@linux.dev>]

On Wed, Feb 12, 2025 at 01:40:51PM +0000, Alexandru Elisei wrote:
...
> > > @@ -80,7 +80,7 @@ function run()
> > >      local groups="$2"
> > >      local smp="$3"
> > >      local kernel="$4"
> > > -    local opts="$5"
> > > +    local qemu_opts="$5"
> > >      local arch="$6"
> > >      local machine="$7"
> > >      local check="${CHECK:-$8}"
> > > @@ -179,9 +179,9 @@ function run()
> > >          echo $cmdline
> > >      fi
> > >  
> > > -    # extra_params in the config file may contain backticks that need to be
> > > -    # expanded, so use eval to start qemu.  Use "> >(foo)" instead of a pipe to
> > > -    # preserve the exit status.
> > > +    # qemu_params/extra_params in the config file may contain backticks that
> > > +    # need to be expanded, so use eval to start qemu.  Use "> >(foo)" instead of
> > > +    # a pipe to preserve the exit status.
> > >      summary=$(eval "$cmdline" 2> >(RUNTIME_log_stderr $testname) \
> > >                               > >(tee >(RUNTIME_log_stdout $testname $kernel) | extract_summary))
> > >      ret=$?
> > > -- 
> > > 2.47.1
> > >
> > 
> > Hmm, I'll keep reading the series, but it seems like we should be choosing
> > generic names like 'extra_params' and 'opts' that we plan to use for both
> > QEMU and kvmtool since they both have the concepts of "options" and "extra
> > params".
> 
> I'm afraid I don't follow you. 'qemu_params' was chosen because it uses
> qemu-specific syntax. Same for 'kvmtool_params', introduced later in the
> series. Are you referring to unittests.cfg or to something else?
>

I didn't like the renaming of opts to qemu_opts since both kvmtool and
qemu have "options", so it seems like we should be generalizing variable
names rather than making them more specific. I see later how there may
be a need for qemu_options, kvmtool_options, and unit test
cmdline_options in the unittests.cfg, though. However, here, it seems
like we could still use 'opts' for the variable and just use another
variable to determine if we parse qemu_options or kvmtool_options, since
there shouldn't be a need to parse both.

Thanks,
drew