diff mbox series

[kvm-unit-tests,09/10] scripts: Implement multiline strings for extra_params

Message ID 20231020144900.2213398-10-nsg@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series s390x: topology: Fixes and extension | expand

Commit Message

Nina Schoetterl-Glausch Oct. 20, 2023, 2:48 p.m. UTC
Implement a rudimentary form only.
extra_params can get long when passing a lot of arguments to qemu.
Multiline strings help with readability of the .cfg file.
Multiline strings begin and end with """, which must occur on separate
lines.

For example:
extra_params = """-cpu max,ctop=on -smp cpus=1,cores=16,maxcpus=128 \
-append '-drawers 2 -books 2 -sockets 2 -cores 16' \
-device max-s390x-cpu,core-id=31,drawer-id=0,book-id=0,socket-id=0"""

The command string built with extra_params is eval'ed by the runtime
script, so the newlines need to be escaped with \.

Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
---
 scripts/common.bash  | 16 ++++++++++++++++
 scripts/runtime.bash |  4 ++--
 2 files changed, 18 insertions(+), 2 deletions(-)

Comments

Thomas Huth Oct. 25, 2023, 5:50 p.m. UTC | #1
On 20/10/2023 16.48, Nina Schoetterl-Glausch wrote:
> Implement a rudimentary form only.
> extra_params can get long when passing a lot of arguments to qemu.
> Multiline strings help with readability of the .cfg file.
> Multiline strings begin and end with """, which must occur on separate
> lines.
> 
> For example:
> extra_params = """-cpu max,ctop=on -smp cpus=1,cores=16,maxcpus=128 \
> -append '-drawers 2 -books 2 -sockets 2 -cores 16' \
> -device max-s390x-cpu,core-id=31,drawer-id=0,book-id=0,socket-id=0"""
> 
> The command string built with extra_params is eval'ed by the runtime
> script, so the newlines need to be escaped with \.
> 
> Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> ---
>   scripts/common.bash  | 16 ++++++++++++++++
>   scripts/runtime.bash |  4 ++--
>   2 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/common.bash b/scripts/common.bash
> index 7b983f7d..b9413d68 100644
> --- a/scripts/common.bash
> +++ b/scripts/common.bash
> @@ -36,6 +36,22 @@ function for_each_unittest()
>   			kernel=$TEST_DIR/${BASH_REMATCH[1]}
>   		elif [[ $line =~ ^smp\ *=\ *(.*)$ ]]; then
>   			smp=${BASH_REMATCH[1]}
> +		elif [[ $line =~ ^extra_params\ *=\ *'"""'(.*)$ ]]; then
> +			opts=${BASH_REMATCH[1]}$'\n'
> +			while read -r -u $fd; do
> +				#escape backslash newline, but not double backslash
> +				if [[ $opts =~ [^\\]*(\\*)$'\n'$ ]]; then
> +					if (( ${#BASH_REMATCH[1]} % 2 == 1 )); then
> +						opts=${opts%\\$'\n'}
> +					fi
> +				fi
> +				if [[ "$REPLY" =~ ^(.*)'"""'[:blank:]*$ ]]; then
> +					opts+=${BASH_REMATCH[1]}
> +					break
> +				else
> +					opts+=$REPLY$'\n'
> +				fi
> +			done

Phew, TIL that there is something like $'\n' in bash ...
Now with that knowledge, the regular expression make sense 8-)

Reviewed-by: Thomas Huth <thuth@redhat.com>
Nina Schoetterl-Glausch Oct. 25, 2023, 5:58 p.m. UTC | #2
On Wed, 2023-10-25 at 19:50 +0200, Thomas Huth wrote:
> On 20/10/2023 16.48, Nina Schoetterl-Glausch wrote:
> > Implement a rudimentary form only.
> > extra_params can get long when passing a lot of arguments to qemu.
> > Multiline strings help with readability of the .cfg file.
> > Multiline strings begin and end with """, which must occur on separate
> > lines.
> > 
> > For example:
> > extra_params = """-cpu max,ctop=on -smp cpus=1,cores=16,maxcpus=128 \
> > -append '-drawers 2 -books 2 -sockets 2 -cores 16' \
> > -device max-s390x-cpu,core-id=31,drawer-id=0,book-id=0,socket-id=0"""
> > 
> > The command string built with extra_params is eval'ed by the runtime
> > script, so the newlines need to be escaped with \.
> > 
> > Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> > ---
> >   scripts/common.bash  | 16 ++++++++++++++++
> >   scripts/runtime.bash |  4 ++--
> >   2 files changed, 18 insertions(+), 2 deletions(-)
> > 
> > diff --git a/scripts/common.bash b/scripts/common.bash
> > index 7b983f7d..b9413d68 100644
> > --- a/scripts/common.bash
> > +++ b/scripts/common.bash
> > @@ -36,6 +36,22 @@ function for_each_unittest()
> >   			kernel=$TEST_DIR/${BASH_REMATCH[1]}
> >   		elif [[ $line =~ ^smp\ *=\ *(.*)$ ]]; then
> >   			smp=${BASH_REMATCH[1]}
> > +		elif [[ $line =~ ^extra_params\ *=\ *'"""'(.*)$ ]]; then
> > +			opts=${BASH_REMATCH[1]}$'\n'
> > +			while read -r -u $fd; do
> > +				#escape backslash newline, but not double backslash
> > +				if [[ $opts =~ [^\\]*(\\*)$'\n'$ ]]; then
> > +					if (( ${#BASH_REMATCH[1]} % 2 == 1 )); then
> > +						opts=${opts%\\$'\n'}
> > +					fi
> > +				fi
> > +				if [[ "$REPLY" =~ ^(.*)'"""'[:blank:]*$ ]]; then
> > +					opts+=${BASH_REMATCH[1]}
> > +					break
> > +				else
> > +					opts+=$REPLY$'\n'
> > +				fi
> > +			done
> 
> Phew, TIL that there is something like $'\n' in bash ...
> Now with that knowledge, the regular expression make sense 8-)

Uh yeah, it's very write only, with backslash escaping, " inside ' and $'\n'.
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> 
Thanks!
diff mbox series

Patch

diff --git a/scripts/common.bash b/scripts/common.bash
index 7b983f7d..b9413d68 100644
--- a/scripts/common.bash
+++ b/scripts/common.bash
@@ -36,6 +36,22 @@  function for_each_unittest()
 			kernel=$TEST_DIR/${BASH_REMATCH[1]}
 		elif [[ $line =~ ^smp\ *=\ *(.*)$ ]]; then
 			smp=${BASH_REMATCH[1]}
+		elif [[ $line =~ ^extra_params\ *=\ *'"""'(.*)$ ]]; then
+			opts=${BASH_REMATCH[1]}$'\n'
+			while read -r -u $fd; do
+				#escape backslash newline, but not double backslash
+				if [[ $opts =~ [^\\]*(\\*)$'\n'$ ]]; then
+					if (( ${#BASH_REMATCH[1]} % 2 == 1 )); then
+						opts=${opts%\\$'\n'}
+					fi
+				fi
+				if [[ "$REPLY" =~ ^(.*)'"""'[:blank:]*$ ]]; then
+					opts+=${BASH_REMATCH[1]}
+					break
+				else
+					opts+=$REPLY$'\n'
+				fi
+			done
 		elif [[ $line =~ ^extra_params\ *=\ *(.*)$ ]]; then
 			opts=${BASH_REMATCH[1]}
 		elif [[ $line =~ ^groups\ *=\ *(.*)$ ]]; then
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index ada8ffd7..fc156f2f 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -15,7 +15,7 @@  extract_summary()
 # We assume that QEMU is going to work if it tried to load the kernel
 premature_failure()
 {
-    local log="$(eval $(get_cmdline _NO_FILE_4Uhere_) 2>&1)"
+    local log="$(eval "$(get_cmdline _NO_FILE_4Uhere_)" 2>&1)"
 
     echo "$log" | grep "_NO_FILE_4Uhere_" |
         grep -q -e "could not \(load\|open\) kernel" -e "error loading" &&
@@ -168,7 +168,7 @@  function run()
     # 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) \
+    summary=$(eval "$cmdline" 2> >(RUNTIME_log_stderr $testname) \
                              > >(tee >(RUNTIME_log_stdout $testname $kernel) | extract_summary))
     ret=$?
     [ "$KUT_STANDALONE" != "yes" ] && echo > >(RUNTIME_log_stdout $testname $kernel)