Re: [kvm-unit-tests PATCH 8/9] scripts: Implement multiline strings for extra_params

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2023-10-19 at 12:50 +0200, Nico Boehr wrote:
> Quoting Nina Schoetterl-Glausch (2023-10-11 10:56:31)
> > 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@xxxxxxxxxxxxx>
> > ---
> > 
> > 
> > This could certainly be done differently, suggestions welcome.
> 
> I honestly do not have a better idea. If someone has, please bring it up!
> 
> [...]
> > diff --git a/scripts/common.bash b/scripts/common.bash
> > index 7b983f7d..738e64af 100644
> > --- a/scripts/common.bash
> > +++ b/scripts/common.bash
> > @@ -36,6 +36,17 @@ 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
> 
> I was actually unaware that read saves into REPLY by default and went
> looking for the REPLY variable to no avail. Can we make this more explicit
> like so:
> 
> while read -r -u $fd LINE; do
> 
> and replace REPLY with LINE below?

No, at least it's not so simple. man bash says:
If no names are supplied, the line read, *without the ending delimiter but otherwise unmodified*,
is assigned to the variable REPLY.
We don't want word splitting and white space being removed.
> 
> > +                               opts=${opts%\\$'\n'}
> 
> So we strip the \ at the end of the line, but if it's not there we preserve

We strip backslash newline, yes.
This way it's up to you if you want newlines in the string or not.
A bit of future proofing.
That is the only escaping I implemented, so right now you cannot have an
actual backslash at the end of the line.
The string is later eval'ed by bash which also does escaping,
but we cannot just rely on that, because by that time the last \n has
been dropped. If you had a backslash before you'll run into trouble.

> the line break? Is there a reason to do it this way? Why do we need the \
> at all, as far as I can see """ terminates the multi line string, so what's
> the purpose?
> 
> Did you test this in standalone mode?

I did not.





[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux