[Qemu-devel] [PATCH] configure: Don't implicitly hardcode list of KVM architectures

[Qemu-devel] [PATCH] configure: Don't implicitly hardcode list of KVM architectures

[Peter Maydell]

The code creating the symlink from linux-headers/asm to the
architecture specific linux-headers/asm-$arch directory was
implicitly hardcoding a list of KVM supporting architectures.
Add a default case for the common "Linux architecture name and
QEMU CPU name match" case, so future architectures will only
need to add code if they've managed to get mismatched names.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
This means there's now only one place where configure has a
list of KVM enabled targets (the check where we set CONFIG_KVM).
I think we have to have one list, but we don't need to have
more than one...

NB: this patch means we'll now set up an asm/ link for
s390 as well as s390x. That should be harmless (s390 can't be
trying to include any headers from asm/ or it wouldn't build).

 configure |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 0a3896e..c8d2895 100755
--- a/configure
+++ b/configure
@@ -3482,15 +3482,23 @@ if test "$linux" = "yes" ; then
   mkdir -p linux-headers
   case "$cpu" in
   i386|x86_64)
-    symlink "$source_path/linux-headers/asm-x86" linux-headers/asm
+    linux_arch=x86
     ;;
   ppcemb|ppc|ppc64)
-    symlink "$source_path/linux-headers/asm-powerpc" linux-headers/asm
+    linux_arch=powerpc
     ;;
   s390x)
-    symlink "$source_path/linux-headers/asm-s390" linux-headers/asm
+    linux_arch=s390
+    ;;
+  *)
+    # For most CPUs the kernel architecture name and QEMU CPU name match.
+    linux_arch="$cpu"
     ;;
   esac
+    # For non-KVM architectures we will not have asm headers.
+    if [ -e "$source_path/linux-headers/asm-$linux_arch" ]; then
+        symlink "$source_path/linux-headers/asm-$linux_arch" linux-headers/asm
+    fi
 fi
 
 for target in $target_list; do
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH] configure: Don't implicitly hardcode list of KVM architectures

[Peter Maydell]

Ping?

-- PMM

On 18 July 2012 12:10, Peter Maydell <peter.maydell@linaro.org> wrote:
> The code creating the symlink from linux-headers/asm to the
> architecture specific linux-headers/asm-$arch directory was
> implicitly hardcoding a list of KVM supporting architectures.
> Add a default case for the common "Linux architecture name and
> QEMU CPU name match" case, so future architectures will only
> need to add code if they've managed to get mismatched names.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This means there's now only one place where configure has a
> list of KVM enabled targets (the check where we set CONFIG_KVM).
> I think we have to have one list, but we don't need to have
> more than one...
>
> NB: this patch means we'll now set up an asm/ link for
> s390 as well as s390x. That should be harmless (s390 can't be
> trying to include any headers from asm/ or it wouldn't build).
>
>  configure |   14 +++++++++++---
>  1 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/configure b/configure
> index 0a3896e..c8d2895 100755
> --- a/configure
> +++ b/configure
> @@ -3482,15 +3482,23 @@ if test "$linux" = "yes" ; then
>    mkdir -p linux-headers
>    case "$cpu" in
>    i386|x86_64)
> -    symlink "$source_path/linux-headers/asm-x86" linux-headers/asm
> +    linux_arch=x86
>      ;;
>    ppcemb|ppc|ppc64)
> -    symlink "$source_path/linux-headers/asm-powerpc" linux-headers/asm
> +    linux_arch=powerpc
>      ;;
>    s390x)
> -    symlink "$source_path/linux-headers/asm-s390" linux-headers/asm
> +    linux_arch=s390
> +    ;;
> +  *)
> +    # For most CPUs the kernel architecture name and QEMU CPU name match.
> +    linux_arch="$cpu"
>      ;;
>    esac
> +    # For non-KVM architectures we will not have asm headers.
> +    if [ -e "$source_path/linux-headers/asm-$linux_arch" ]; then
> +        symlink "$source_path/linux-headers/asm-$linux_arch" linux-headers/asm
> +    fi
>  fi
>
>  for target in $target_list; do
> --
> 1.7.5.4
>
>

Re: [Qemu-devel] [PATCH] configure: Don't implicitly hardcode list of KVM architectures

[Andreas Färber]

Am 02.08.2012 17:41, schrieb Peter Maydell:
> Ping?

Alex? Christian?

> 
> -- PMM
> 
> On 18 July 2012 12:10, Peter Maydell <peter.maydell@linaro.org> wrote:
>> The code creating the symlink from linux-headers/asm to the
>> architecture specific linux-headers/asm-$arch directory was
>> implicitly hardcoding a list of KVM supporting architectures.
>> Add a default case for the common "Linux architecture name and
>> QEMU CPU name match" case, so future architectures will only
>> need to add code if they've managed to get mismatched names.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> This means there's now only one place where configure has a
>> list of KVM enabled targets (the check where we set CONFIG_KVM).
>> I think we have to have one list, but we don't need to have
>> more than one...
>>
>> NB: this patch means we'll now set up an asm/ link for
>> s390 as well as s390x. That should be harmless (s390 can't be
>> trying to include any headers from asm/ or it wouldn't build).
>>
>>  configure |   14 +++++++++++---
>>  1 files changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/configure b/configure
>> index 0a3896e..c8d2895 100755
>> --- a/configure
>> +++ b/configure
>> @@ -3482,15 +3482,23 @@ if test "$linux" = "yes" ; then
>>    mkdir -p linux-headers
>>    case "$cpu" in
>>    i386|x86_64)
>> -    symlink "$source_path/linux-headers/asm-x86" linux-headers/asm
>> +    linux_arch=x86
>>      ;;
>>    ppcemb|ppc|ppc64)
>> -    symlink "$source_path/linux-headers/asm-powerpc" linux-headers/asm
>> +    linux_arch=powerpc
>>      ;;
>>    s390x)
>> -    symlink "$source_path/linux-headers/asm-s390" linux-headers/asm
>> +    linux_arch=s390
>> +    ;;
>> +  *)
>> +    # For most CPUs the kernel architecture name and QEMU CPU name match.
>> +    linux_arch="$cpu"
>>      ;;
>>    esac
>> +    # For non-KVM architectures we will not have asm headers.
>> +    if [ -e "$source_path/linux-headers/asm-$linux_arch" ]; then
>> +        symlink "$source_path/linux-headers/asm-$linux_arch" linux-headers/asm
>> +    fi
>>  fi
>>
>>  for target in $target_list; do
>> --
>> 1.7.5.4

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] configure: Don't implicitly hardcode list of KVM architectures

[Alexander Graf]

On 02.08.2012, at 17:46, Andreas Färber wrote:

> Am 02.08.2012 17:41, schrieb Peter Maydell:
>> Ping?
> 
> Alex? Christian?

Looks good to me, and should probably go in through the kvm queue. Avi, Marcelo?

> 
>> 
>> -- PMM
>> 
>> On 18 July 2012 12:10, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> The code creating the symlink from linux-headers/asm to the
>>> architecture specific linux-headers/asm-$arch directory was
>>> implicitly hardcoding a list of KVM supporting architectures.
>>> Add a default case for the common "Linux architecture name and
>>> QEMU CPU name match" case, so future architectures will only
>>> need to add code if they've managed to get mismatched names.
>>> 
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>> ---
>>> This means there's now only one place where configure has a
>>> list of KVM enabled targets (the check where we set CONFIG_KVM).
>>> I think we have to have one list, but we don't need to have
>>> more than one...
>>> 
>>> NB: this patch means we'll now set up an asm/ link for
>>> s390 as well as s390x. That should be harmless (s390 can't be
>>> trying to include any headers from asm/ or it wouldn't build).
>>> 
>>> configure |   14 +++++++++++---
>>> 1 files changed, 11 insertions(+), 3 deletions(-)
>>> 
>>> diff --git a/configure b/configure
>>> index 0a3896e..c8d2895 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -3482,15 +3482,23 @@ if test "$linux" = "yes" ; then
>>>   mkdir -p linux-headers
>>>   case "$cpu" in
>>>   i386|x86_64)
>>> -    symlink "$source_path/linux-headers/asm-x86" linux-headers/asm
>>> +    linux_arch=x86
>>>     ;;
>>>   ppcemb|ppc|ppc64)
>>> -    symlink "$source_path/linux-headers/asm-powerpc" linux-headers/asm
>>> +    linux_arch=powerpc
>>>     ;;
>>>   s390x)
>>> -    symlink "$source_path/linux-headers/asm-s390" linux-headers/asm
>>> +    linux_arch=s390
>>> +    ;;
>>> +  *)
>>> +    # For most CPUs the kernel architecture name and QEMU CPU name match.
>>> +    linux_arch="$cpu"
>>>     ;;
>>>   esac
>>> +    # For non-KVM architectures we will not have asm headers.
>>> +    if [ -e "$source_path/linux-headers/asm-$linux_arch" ]; then
>>> +        symlink "$source_path/linux-headers/asm-$linux_arch" linux-headers/asm
>>> +    fi
>>> fi
>>> 
>>> for target in $target_list; do
>>> --
>>> 1.7.5.4
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] configure: Don't implicitly hardcode list of KVM architectures

[Andreas Färber]

Am 18.07.2012 13:10, schrieb Peter Maydell:
> The code creating the symlink from linux-headers/asm to the
> architecture specific linux-headers/asm-$arch directory was
> implicitly hardcoding a list of KVM supporting architectures.
> Add a default case for the common "Linux architecture name and
> QEMU CPU name match" case, so future architectures will only
> need to add code if they've managed to get mismatched names.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This means there's now only one place where configure has a
> list of KVM enabled targets (the check where we set CONFIG_KVM).
> I think we have to have one list, but we don't need to have
> more than one...
> 
> NB: this patch means we'll now set up an asm/ link for
> s390 as well as s390x. That should be harmless (s390 can't be
> trying to include any headers from asm/ or it wouldn't build).
> 
>  configure |   14 +++++++++++---
>  1 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/configure b/configure
> index 0a3896e..c8d2895 100755
> --- a/configure
> +++ b/configure
> @@ -3482,15 +3482,23 @@ if test "$linux" = "yes" ; then
>    mkdir -p linux-headers
>    case "$cpu" in
>    i386|x86_64)
> -    symlink "$source_path/linux-headers/asm-x86" linux-headers/asm
> +    linux_arch=x86
>      ;;
>    ppcemb|ppc|ppc64)
> -    symlink "$source_path/linux-headers/asm-powerpc" linux-headers/asm
> +    linux_arch=powerpc
>      ;;
>    s390x)
> -    symlink "$source_path/linux-headers/asm-s390" linux-headers/asm
> +    linux_arch=s390
> +    ;;
> +  *)
> +    # For most CPUs the kernel architecture name and QEMU CPU name match.
> +    linux_arch="$cpu"
>      ;;
>    esac
> +    # For non-KVM architectures we will not have asm headers.
> +    if [ -e "$source_path/linux-headers/asm-$linux_arch" ]; then
> +        symlink "$source_path/linux-headers/asm-$linux_arch" linux-headers/asm
> +    fi

One minor nit: The indentation seems off here - esac 2 spaces, if 4.

Andreas

>  fi
>  
>  for target in $target_list; do
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] configure: Don't implicitly hardcode list of KVM architectures

[Peter Maydell]

On 2 August 2012 17:01, Andreas Färber <afaerber@suse.de> wrote:
> One minor nit: The indentation seems off here - esac 2 spaces, if 4.

configure's indentation is pretty inconsistent generally between
2 or 4 spaces. I usually go for 4 for new code but don't reindent
old code. (There's a lot more 2-space than 4-space I think but
4 is the qemu coding style :-))

-- PMM

Re: [Qemu-devel] [PATCH] configure: Don't implicitly hardcode list of KVM architectures

[Andreas Färber]

Am 02.08.2012 18:22, schrieb Peter Maydell:
> On 2 August 2012 17:01, Andreas Färber <afaerber@suse.de> wrote:
>> One minor nit: The indentation seems off here - esac 2 spaces, if 4.
> 
> configure's indentation is pretty inconsistent generally between
> 2 or 4 spaces. I usually go for 4 for new code but don't reindent
> old code. (There's a lot more 2-space than 4-space I think but
> 4 is the qemu coding style :-))

A matter of interpretation, I see our Coding Style as applying to C
code, not strictly to shell or Python or Perl code.

Why I pointed this out is because that's effectively like writing:

        ...
    }
        if (foo) {
                ...
        }
}

So could you please either use 2-space indentation for your inserted if
or re-indent the case block above, so that there is not this break?

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] configure: Don't implicitly hardcode list of KVM architectures

[Marcelo Tosatti]

On Thu, Aug 02, 2012 at 05:53:37PM +0200, Alexander Graf wrote:
> 
> On 02.08.2012, at 17:46, Andreas Färber wrote:
> 
> > Am 02.08.2012 17:41, schrieb Peter Maydell:
> >> Ping?
> > 
> > Alex? Christian?
> 
> Looks good to me, and should probably go in through the kvm queue. Avi, Marcelo?

Sure, please send the patch against uq/master (with [PATCH uq/master] on
the subject line) to kvm@vger.

uq/master is a branch of 
git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git.