[PATCH kvm-unit-tests] arch-run: more qemu binary search cleanup

[PATCH kvm-unit-tests] arch-run: more qemu binary search cleanup

[Andrew Jones]

We use tabs in scripts/arch-run.bash, which the import of the
function forgot to convert to.  Also, while touching all lines
to add the tabs, apply some more cleanups and simplifications.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 scripts/arch-run.bash | 39 ++++++++++++++++++---------------------
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index d526d98bcc40..5c10828e30f9 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -134,27 +134,24 @@ migration_cmd ()
 	fi
 }
 
-# qemu binary search function for all arches
 search_qemu_binary ()
 {
-    local save_path=$PATH
-    local qemucmd QEMUFOUND qemu
-    export PATH=$PATH:/usr/libexec
-    for qemucmd in ${QEMU:-qemu-system-$ARCH_NAME qemu-kvm}
-    do
-        unset QEMUFOUND
-        unset qemu
-        if ! [ -z "${QEMUFOUND=$(${qemucmd} --help 2>/dev/null  | grep "QEMU")}" ]
-        then
-            qemu="${qemucmd}"
-            break
-        fi
-    done
-
-    if [ -z "${QEMUFOUND}" ]; then
-        echo "A QEMU binary was not found, You can set a custom location by using the QEMU=<path> environment variable"
-        exit 2
-    fi
-    which $qemu
-    export PATH=$save_path
+	local save_path=$PATH
+	local qemucmd qemu
+
+	export PATH=$PATH:/usr/libexec
+	for qemucmd in ${QEMU:-qemu-system-$ARCH_NAME qemu-kvm}; do
+		if $qemucmd --help 2>/dev/null | grep -q 'QEMU'; then
+			qemu="$qemucmd"
+			break
+		fi
+	done
+
+	if [ -z "$qemu" ]; then
+		echo "A QEMU binary was not found."
+		echo "You can set a custom location by using the QEMU=<path> environment variable."
+		exit 2
+	fi
+	command -v $qemu
+	export PATH=$save_path
 }
-- 
2.9.3

Re: [PATCH kvm-unit-tests] arch-run: more qemu binary search cleanup

[Balamuruhan S]

It looks good and I have tested this patch in x86 and PowerPC.

I tried to use git am <patch> but it fails with,

# git am ./test.patch 
Patch format detection failed.

so I did git apply and tested. Henceforth I will use tabs for indentation.

Regards,
Bala

On Mon, Apr 24, 2017 at 11:26:18AM +0200, Andrew Jones wrote:
> We use tabs in scripts/arch-run.bash, which the import of the
> function forgot to convert to.  Also, while touching all lines
> to add the tabs, apply some more cleanups and simplifications.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  scripts/arch-run.bash | 39 ++++++++++++++++++---------------------
>  1 file changed, 18 insertions(+), 21 deletions(-)
> 
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index d526d98bcc40..5c10828e30f9 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -134,27 +134,24 @@ migration_cmd ()
>  	fi
>  }
> 
> -# qemu binary search function for all arches
>  search_qemu_binary ()
>  {
> -    local save_path=$PATH
> -    local qemucmd QEMUFOUND qemu
> -    export PATH=$PATH:/usr/libexec
> -    for qemucmd in ${QEMU:-qemu-system-$ARCH_NAME qemu-kvm}
> -    do
> -        unset QEMUFOUND
> -        unset qemu
> -        if ! [ -z "${QEMUFOUND=$(${qemucmd} --help 2>/dev/null  | grep "QEMU")}" ]
> -        then
> -            qemu="${qemucmd}"
> -            break
> -        fi
> -    done
> -
> -    if [ -z "${QEMUFOUND}" ]; then
> -        echo "A QEMU binary was not found, You can set a custom location by using the QEMU=<path> environment variable"
> -        exit 2
> -    fi
> -    which $qemu
> -    export PATH=$save_path
> +	local save_path=$PATH
> +	local qemucmd qemu
> +
> +	export PATH=$PATH:/usr/libexec
> +	for qemucmd in ${QEMU:-qemu-system-$ARCH_NAME qemu-kvm}; do
> +		if $qemucmd --help 2>/dev/null | grep -q 'QEMU'; then
> +			qemu="$qemucmd"
> +			break
> +		fi
> +	done
> +
> +	if [ -z "$qemu" ]; then
> +		echo "A QEMU binary was not found."
> +		echo "You can set a custom location by using the QEMU=<path> environment variable."
> +		exit 2
> +	fi
> +	command -v $qemu
> +	export PATH=$save_path
>  }
> -- 
> 2.9.3
> 

Re: [PATCH kvm-unit-tests] arch-run: more qemu binary search cleanup

[Andrew Jones]

On Mon, Apr 24, 2017 at 06:17:14PM +0530, Balamuruhan S wrote:
> It looks good and I have tested this patch in x86 and PowerPC.
> 
> I tried to use git am <patch> but it fails with,
> 
> # git am ./test.patch 
> Patch format detection failed.
> 
> so I did git apply and tested.

Thanks for the testing.

> Henceforth I will use tabs for indentation.

We never officially decided on a bash coding standard (we should probably
do that, as we have over 500 lines of bash now). Original files are spaces
(or mostly, occasionally you'll find a tab...), but when I wrote a couple
new bash files I used tabs, and scripts/arch-run.bash is one of those
files. While tabs might not have been the right choice, I'd prefer that at
least per file consistency is maintained.

Thanks,
drew

> 
> Regards,
> Bala
> 
> On Mon, Apr 24, 2017 at 11:26:18AM +0200, Andrew Jones wrote:
> > We use tabs in scripts/arch-run.bash, which the import of the
> > function forgot to convert to.  Also, while touching all lines
> > to add the tabs, apply some more cleanups and simplifications.
> > 
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  scripts/arch-run.bash | 39 ++++++++++++++++++---------------------
> >  1 file changed, 18 insertions(+), 21 deletions(-)
> > 
> > diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> > index d526d98bcc40..5c10828e30f9 100644
> > --- a/scripts/arch-run.bash
> > +++ b/scripts/arch-run.bash
> > @@ -134,27 +134,24 @@ migration_cmd ()
> >  	fi
> >  }
> > 
> > -# qemu binary search function for all arches
> >  search_qemu_binary ()
> >  {
> > -    local save_path=$PATH
> > -    local qemucmd QEMUFOUND qemu
> > -    export PATH=$PATH:/usr/libexec
> > -    for qemucmd in ${QEMU:-qemu-system-$ARCH_NAME qemu-kvm}
> > -    do
> > -        unset QEMUFOUND
> > -        unset qemu
> > -        if ! [ -z "${QEMUFOUND=$(${qemucmd} --help 2>/dev/null  | grep "QEMU")}" ]
> > -        then
> > -            qemu="${qemucmd}"
> > -            break
> > -        fi
> > -    done
> > -
> > -    if [ -z "${QEMUFOUND}" ]; then
> > -        echo "A QEMU binary was not found, You can set a custom location by using the QEMU=<path> environment variable"
> > -        exit 2
> > -    fi
> > -    which $qemu
> > -    export PATH=$save_path
> > +	local save_path=$PATH
> > +	local qemucmd qemu
> > +
> > +	export PATH=$PATH:/usr/libexec
> > +	for qemucmd in ${QEMU:-qemu-system-$ARCH_NAME qemu-kvm}; do
> > +		if $qemucmd --help 2>/dev/null | grep -q 'QEMU'; then
> > +			qemu="$qemucmd"
> > +			break
> > +		fi
> > +	done
> > +
> > +	if [ -z "$qemu" ]; then
> > +		echo "A QEMU binary was not found."
> > +		echo "You can set a custom location by using the QEMU=<path> environment variable."
> > +		exit 2
> > +	fi
> > +	command -v $qemu
> > +	export PATH=$save_path
> >  }
> > -- 
> > 2.9.3
> > 
> 

Re: [PATCH kvm-unit-tests] arch-run: more qemu binary search cleanup

[Balamuruhan S]

On Mon, Apr 24, 2017 at 03:02:01PM +0200, Andrew Jones wrote:
> On Mon, Apr 24, 2017 at 06:17:14PM +0530, Balamuruhan S wrote:
> > It looks good and I have tested this patch in x86 and PowerPC.
> > 
> > I tried to use git am <patch> but it fails with,
> > 
> > # git am ./test.patch 
> > Patch format detection failed.
> > 
> > so I did git apply and tested.
> 
> Thanks for the testing.
> 
> > Henceforth I will use tabs for indentation.
> 
> We never officially decided on a bash coding standard (we should probably
> do that, as we have over 500 lines of bash now). Original files are spaces
> (or mostly, occasionally you'll find a tab...), but when I wrote a couple
> new bash files I used tabs, and scripts/arch-run.bash is one of those
> files. While tabs might not have been the right choice, I'd prefer that at
> least per file consistency is maintained.

Sure drew.

Thanks,
Bala

> 
> Thanks,
> drew
> 
> > 
> > Regards,
> > Bala
> > 
> > On Mon, Apr 24, 2017 at 11:26:18AM +0200, Andrew Jones wrote:
> > > We use tabs in scripts/arch-run.bash, which the import of the
> > > function forgot to convert to.  Also, while touching all lines
> > > to add the tabs, apply some more cleanups and simplifications.
> > > 
> > > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > > ---
> > >  scripts/arch-run.bash | 39 ++++++++++++++++++---------------------
> > >  1 file changed, 18 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> > > index d526d98bcc40..5c10828e30f9 100644
> > > --- a/scripts/arch-run.bash
> > > +++ b/scripts/arch-run.bash
> > > @@ -134,27 +134,24 @@ migration_cmd ()
> > >  	fi
> > >  }
> > > 
> > > -# qemu binary search function for all arches
> > >  search_qemu_binary ()
> > >  {
> > > -    local save_path=$PATH
> > > -    local qemucmd QEMUFOUND qemu
> > > -    export PATH=$PATH:/usr/libexec
> > > -    for qemucmd in ${QEMU:-qemu-system-$ARCH_NAME qemu-kvm}
> > > -    do
> > > -        unset QEMUFOUND
> > > -        unset qemu
> > > -        if ! [ -z "${QEMUFOUND=$(${qemucmd} --help 2>/dev/null  | grep "QEMU")}" ]
> > > -        then
> > > -            qemu="${qemucmd}"
> > > -            break
> > > -        fi
> > > -    done
> > > -
> > > -    if [ -z "${QEMUFOUND}" ]; then
> > > -        echo "A QEMU binary was not found, You can set a custom location by using the QEMU=<path> environment variable"
> > > -        exit 2
> > > -    fi
> > > -    which $qemu
> > > -    export PATH=$save_path
> > > +	local save_path=$PATH
> > > +	local qemucmd qemu
> > > +
> > > +	export PATH=$PATH:/usr/libexec
> > > +	for qemucmd in ${QEMU:-qemu-system-$ARCH_NAME qemu-kvm}; do
> > > +		if $qemucmd --help 2>/dev/null | grep -q 'QEMU'; then
> > > +			qemu="$qemucmd"
> > > +			break
> > > +		fi
> > > +	done
> > > +
> > > +	if [ -z "$qemu" ]; then
> > > +		echo "A QEMU binary was not found."
> > > +		echo "You can set a custom location by using the QEMU=<path> environment variable."
> > > +		exit 2
> > > +	fi
> > > +	command -v $qemu
> > > +	export PATH=$save_path
> > >  }
> > > -- 
> > > 2.9.3
> > > 
> > 
> 

Re: [PATCH kvm-unit-tests] arch-run: more qemu binary search cleanup

[Paolo Bonzini]

On 24/04/2017 11:26, Andrew Jones wrote:
> We use tabs in scripts/arch-run.bash, which the import of the
> function forgot to convert to.  Also, while touching all lines
> to add the tabs, apply some more cleanups and simplifications.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>

Queued, thanks.

Paolo

> ---
>  scripts/arch-run.bash | 39 ++++++++++++++++++---------------------
>  1 file changed, 18 insertions(+), 21 deletions(-)
> 
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index d526d98bcc40..5c10828e30f9 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -134,27 +134,24 @@ migration_cmd ()
>  	fi
>  }
>  
> -# qemu binary search function for all arches
>  search_qemu_binary ()
>  {
> -    local save_path=$PATH
> -    local qemucmd QEMUFOUND qemu
> -    export PATH=$PATH:/usr/libexec
> -    for qemucmd in ${QEMU:-qemu-system-$ARCH_NAME qemu-kvm}
> -    do
> -        unset QEMUFOUND
> -        unset qemu
> -        if ! [ -z "${QEMUFOUND=$(${qemucmd} --help 2>/dev/null  | grep "QEMU")}" ]
> -        then
> -            qemu="${qemucmd}"
> -            break
> -        fi
> -    done
> -
> -    if [ -z "${QEMUFOUND}" ]; then
> -        echo "A QEMU binary was not found, You can set a custom location by using the QEMU=<path> environment variable"
> -        exit 2
> -    fi
> -    which $qemu
> -    export PATH=$save_path
> +	local save_path=$PATH
> +	local qemucmd qemu
> +
> +	export PATH=$PATH:/usr/libexec
> +	for qemucmd in ${QEMU:-qemu-system-$ARCH_NAME qemu-kvm}; do
> +		if $qemucmd --help 2>/dev/null | grep -q 'QEMU'; then
> +			qemu="$qemucmd"
> +			break
> +		fi
> +	done
> +
> +	if [ -z "$qemu" ]; then
> +		echo "A QEMU binary was not found."
> +		echo "You can set a custom location by using the QEMU=<path> environment variable."
> +		exit 2
> +	fi
> +	command -v $qemu
> +	export PATH=$save_path
>  }
>