[Qemu-devel] [PATCH 02/22] configure: early test for supported targets

Paolo Bonzini posted 22 patches 8 years, 7 months ago
There is a newer version of this series
[Qemu-devel] [PATCH 02/22] configure: early test for supported targets
Posted by Paolo Bonzini 8 years, 7 months ago
Check for unsupported targets in target_list, and print an
error early in the configuration process.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configure | 65 ++++++++++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 43 insertions(+), 22 deletions(-)

diff --git a/configure b/configure
index 0f14e79..6d1a6a9 100755
--- a/configure
+++ b/configure
@@ -40,14 +40,18 @@ printf " '%s'" "$0" "$@" >> config.log
 echo >> config.log
 echo "#" >> config.log
 
-error_exit() {
-    echo
+print_error() {
+    (echo
     echo "ERROR: $1"
     while test -n "$2"; do
         echo "       $2"
         shift
     done
-    echo
+    echo) >&2
+}
+
+error_exit() {
+    print_error "$@"
     exit 1
 }
 
@@ -207,6 +211,25 @@ supported_xen_target() {
     return 1
 }
 
+supported_target() {
+    case "$1" in
+        ${target_name}-softmmu) ;;
+        ${target_name}-linux-user)
+            if test "$linux" != "yes"; then
+                print_error "Target '$target' is only available on a Linux host"
+                return 1
+            fi
+            ;;
+        ${target_name}-bsd-user)
+            if test "$bsd" != "yes"; then
+                print_error "Target '$target' is only available on a BSD host"
+                return 1
+            fi
+            ;;
+    esac
+    return 0
+}
+
 # default parameters
 source_path=$(dirname "$0")
 cpu=""
@@ -1734,23 +1757,27 @@ if test "$solaris" = "yes" ; then
 fi
 
 if test -z "${target_list+xxx}" ; then
-    target_list="$default_target_list"
+    for target in $default_target_list; do
+        supported_target $target 2>/dev/null && \
+            target_list="$target_list $target"
+    done
+    target_list="${target_list# }"
 else
     target_list=$(echo "$target_list" | sed -e 's/,/ /g')
+    for target in $target_list; do
+        # Check that we recognised the target name; this allows a more
+        # friendly error message than if we let it fall through.
+        case " $default_target_list " in
+            *" $target "*)
+                ;;
+            *)
+                error_exit "Unknown target name '$target'"
+                ;;
+        esac
+        supported_target $target || exit 1
+    done
 fi
 
-# Check that we recognised the target name; this allows a more
-# friendly error message than if we let it fall through.
-for target in $target_list; do
-    case " $default_target_list " in
-        *" $target "*)
-            ;;
-        *)
-            error_exit "Unknown target name '$target'"
-            ;;
-    esac
-done
-
 # see if system emulation was really requested
 case " $target_list " in
   *"-softmmu "*) softmmu=yes
@@ -6050,16 +6077,10 @@ case "$target" in
     target_softmmu="yes"
     ;;
   ${target_name}-linux-user)
-    if test "$linux" != "yes" ; then
-      error_exit "Target '$target' is only available on a Linux host"
-    fi
     target_user_only="yes"
     target_linux_user="yes"
     ;;
   ${target_name}-bsd-user)
-    if test "$bsd" != "yes" ; then
-      error_exit "Target '$target' is only available on a BSD host"
-    fi
     target_user_only="yes"
     target_bsd_user="yes"
     ;;
-- 
1.8.3.1



Re: [Qemu-devel] [PATCH 02/22] configure: early test for supported targets
Posted by Richard Henderson 8 years, 7 months ago
On 07/03/2017 09:34 AM, Paolo Bonzini wrote:
> Check for unsupported targets in target_list, and print an
> error early in the configuration process.
> 
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> ---
>   configure | 65 ++++++++++++++++++++++++++++++++++++++++++---------------------
>   1 file changed, 43 insertions(+), 22 deletions(-)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~


Re: [Qemu-devel] [PATCH 02/22] configure: early test for supported targets
Posted by Daniel P. Berrange 8 years, 7 months ago
On Mon, Jul 03, 2017 at 06:34:33PM +0200, Paolo Bonzini wrote:
> Check for unsupported targets in target_list, and print an
> error early in the configuration process.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  configure | 65 ++++++++++++++++++++++++++++++++++++++++++---------------------
>  1 file changed, 43 insertions(+), 22 deletions(-)
> 
> diff --git a/configure b/configure
> index 0f14e79..6d1a6a9 100755
> --- a/configure
> +++ b/configure
> @@ -40,14 +40,18 @@ printf " '%s'" "$0" "$@" >> config.log
>  echo >> config.log
>  echo "#" >> config.log
>  
> -error_exit() {
> -    echo
> +print_error() {
> +    (echo
>      echo "ERROR: $1"
>      while test -n "$2"; do
>          echo "       $2"
>          shift
>      done
> -    echo
> +    echo) >&2
> +}
> +
> +error_exit() {
> +    print_error "$@"
>      exit 1
>  }
>  
> @@ -207,6 +211,25 @@ supported_xen_target() {
>      return 1
>  }
>  
> +supported_target() {
> +    case "$1" in
> +        ${target_name}-softmmu) ;;

Nit-pick, I'd prefer the ';;' on a separate line, as currently it at
first looks like the -softmmu line will fallthrough to the -linux-user
block below. Took a while before I noticed the trailing ;;

Or alternatively add a blank line between cases

> +        ${target_name}-linux-user)
> +            if test "$linux" != "yes"; then
> +                print_error "Target '$target' is only available on a Linux host"
> +                return 1
> +            fi
> +            ;;
> +        ${target_name}-bsd-user)
> +            if test "$bsd" != "yes"; then
> +                print_error "Target '$target' is only available on a BSD host"
> +                return 1
> +            fi
> +            ;;

Add a wildcard match as a catch-all to avoid falling through to the
success codepath on unknown items ?

> +    esac
> +    return 0
> +}
> +
>  # default parameters
>  source_path=$(dirname "$0")
>  cpu=""
> @@ -1734,23 +1757,27 @@ if test "$solaris" = "yes" ; then
>  fi
>  
>  if test -z "${target_list+xxx}" ; then
> -    target_list="$default_target_list"
> +    for target in $default_target_list; do
> +        supported_target $target 2>/dev/null && \
> +            target_list="$target_list $target"
> +    done

I'm not sure what the benefit of this change is ?  It is calling
supported_target but throwing away anything it prints. So if there
was a mistake in the default target list setup, we'd never see it,
just silently discard it.

> +    target_list="${target_list# }"
>  else
>      target_list=$(echo "$target_list" | sed -e 's/,/ /g')
> +    for target in $target_list; do
> +        # Check that we recognised the target name; this allows a more
> +        # friendly error message than if we let it fall through.
> +        case " $default_target_list " in
> +            *" $target "*)
> +                ;;
> +            *)
> +                error_exit "Unknown target name '$target'"
> +                ;;
> +        esac
> +        supported_target $target || exit 1
> +    done
>  fi
>  
> -# Check that we recognised the target name; this allows a more
> -# friendly error message than if we let it fall through.
> -for target in $target_list; do
> -    case " $default_target_list " in
> -        *" $target "*)
> -            ;;
> -        *)
> -            error_exit "Unknown target name '$target'"
> -            ;;
> -    esac
> -done
> -

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 02/22] configure: early test for supported targets
Posted by Paolo Bonzini 8 years, 7 months ago

On 04/07/2017 10:32, Daniel P. Berrange wrote:
>>  if test -z "${target_list+xxx}" ; then
>> -    target_list="$default_target_list"
>> +    for target in $default_target_list; do
>> +        supported_target $target 2>/dev/null && \
>> +            target_list="$target_list $target"
>> +    done
> 
> I'm not sure what the benefit of this change is ?  It is calling
> supported_target but throwing away anything it prints. So if there
> was a mistake in the default target list setup, we'd never see it,
> just silently discard it.

As of this patch there is no benefit, but it is what makes patch 3 drop
non-hardware-accelerated targets for --disable-tcg.

Paolo