Re: [kvm-unit-tests PATCH 6/6] runtime: make the MAX_SMP loop a function

[Andrew Jones <drjones@redhat.com>]

On Tue, Jan 26, 2016 at 12:08:54PM +0100, Paolo Bonzini wrote:
> 
> 
> On 22/01/2016 15:58, Andrew Jones wrote:
> > We can now source scripts/runtime.bash multiple times without
> > always running the MAX_SMP loop. runtime.bash is now ready to
> > be sourced by arch run scripts (when not running in standalone
> > mode). Patches coming soon will add a couple of arch-neutral
> > functions to the arch run scripts.
> > 
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  run_tests.sh            |  2 ++
> >  scripts/mkstandalone.sh |  1 +
> >  scripts/runtime.bash    | 12 +++++++++---
> >  3 files changed, 12 insertions(+), 3 deletions(-)
> > 
> > diff --git a/run_tests.sh b/run_tests.sh
> > index 558b8e7431d8e..c0c5a72b1ceee 100755
> > --- a/run_tests.sh
> > +++ b/run_tests.sh
> > @@ -28,6 +28,8 @@ EOF
> >  RUNTIME_arch_run="./$TEST_DIR/run"
> >  source scripts/runtime.bash
> >  
> > +probe_max_smp
> > +
> >  while getopts "g:hv" opt; do
> >      case $opt in
> >          g)
> > diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> > index b0f1e7c098afb..c31a5f10519a1 100755
> > --- a/scripts/mkstandalone.sh
> > +++ b/scripts/mkstandalone.sh
> > @@ -60,6 +60,7 @@ generate_test ()
> >  
> >  	cat scripts/runtime.bash
> >  
> > +	echo probe_max_smp
> >  	echo "run ${args[@]}"
> >  }
> >  
> > diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> > index 63d1b9653007b..dd700f24cffa3 100644
> > --- a/scripts/runtime.bash
> > +++ b/scripts/runtime.bash
> > @@ -1,5 +1,9 @@
> >  : "${RUNTIME_arch_run?}"
> >  
> > +if [ -z "$MAX_SMP" ]; then
> > +    MAX_SMP=$(getconf _NPROCESSORS_CONF)
> > +fi
> > +
> >  qemu=${QEMU:-qemu-system-$ARCH}
> >  
> >  function run()
> > @@ -57,7 +61,6 @@ function run()
> >      return $ret
> >  }
> >  
> > -MAX_SMP=$(getconf _NPROCESSORS_CONF)
> >  #
> >  # Probe for MAX_SMP, in case it's less than the number of host cpus.
> >  #
> > @@ -67,7 +70,10 @@ MAX_SMP=$(getconf _NPROCESSORS_CONF)
> >  # "arm/arm64: KVM: Remove 'config KVM_ARM_MAX_VCPUS'". So, at some
> >  # point when maintaining the while loop gets too tiresome, we can
> >  # just remove it...
> > -while $RUNTIME_arch_run _NO_FILE_4Uhere_ -smp $MAX_SMP \
> > +function probe_max_smp()
> > +{
> > +    while $RUNTIME_arch_run _NO_FILE_4Uhere_ -smp $MAX_SMP \
> >  		|& grep -qi 'exceeds max CPUs'; do
> >  	((--MAX_SMP))
> > -done
> > +    done
> > +}
> > 
> 
> This kinda goes against the idea of removing duplication between
> runtests and mkstandalone...
> 
> I wonder if you should instead remove the
> 
>     : "${RUNTIME_arch_run?}"
> 
> and conditionalize the MAX_SMP test on the existence of
> RUNTIME_arch_run.  Is it set during execution of the arch run scripts?

Let's drop this patch. Originally I planned on adding more functions to
scripts/runtime.bash, in my next series (coming soon), which would require
the arch-run scripts to source scripts/runtime.bash. Now, I think it's
cleaner to just create a new file to source, scripts/arch-run.bash.

Thanks,
drew