| Message ID | 1342609849-12618-1-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
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 > >
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
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
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 >
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
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
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.
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
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(-)