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
