* [Qemu-devel] [PATCH] configure: Don't implicitly hardcode list of KVM architectures @ 2012-07-18 11:10 Peter Maydell 2012-08-02 15:41 ` Peter Maydell 2012-08-02 16:01 ` Andreas Färber 0 siblings, 2 replies; 8+ messages in thread From: Peter Maydell @ 2012-07-18 11:10 UTC (permalink / raw) To: qemu-devel; +Cc: patches 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: Don't implicitly hardcode list of KVM architectures 2012-07-18 11:10 [Qemu-devel] [PATCH] configure: Don't implicitly hardcode list of KVM architectures Peter Maydell @ 2012-08-02 15:41 ` Peter Maydell 2012-08-02 15:46 ` Andreas Färber 2012-08-02 16:01 ` Andreas Färber 1 sibling, 1 reply; 8+ messages in thread From: Peter Maydell @ 2012-08-02 15:41 UTC (permalink / raw) To: qemu-devel; +Cc: patches 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 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: Don't implicitly hardcode list of KVM architectures 2012-08-02 15:41 ` Peter Maydell @ 2012-08-02 15:46 ` Andreas Färber 2012-08-02 15:53 ` Alexander Graf 0 siblings, 1 reply; 8+ messages in thread From: Andreas Färber @ 2012-08-02 15:46 UTC (permalink / raw) To: Alexander Graf, Christian Borntraeger; +Cc: Peter Maydell, qemu-devel, patches 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: Don't implicitly hardcode list of KVM architectures 2012-08-02 15:46 ` Andreas Färber @ 2012-08-02 15:53 ` Alexander Graf 2012-08-02 20:31 ` Marcelo Tosatti 0 siblings, 1 reply; 8+ messages in thread From: Alexander Graf @ 2012-08-02 15:53 UTC (permalink / raw) To: Andreas Färber Cc: Peter Maydell, patches, Marcelo Tosatti, qemu-devel qemu-devel, Christian Borntraeger, Avi Kivity 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: Don't implicitly hardcode list of KVM architectures 2012-08-02 15:53 ` Alexander Graf @ 2012-08-02 20:31 ` Marcelo Tosatti 0 siblings, 0 replies; 8+ messages in thread From: Marcelo Tosatti @ 2012-08-02 20:31 UTC (permalink / raw) To: Alexander Graf Cc: Peter Maydell, patches, qemu-devel qemu-devel, Christian Borntraeger, Avi Kivity, Andreas Färber 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: Don't implicitly hardcode list of KVM architectures 2012-07-18 11:10 [Qemu-devel] [PATCH] configure: Don't implicitly hardcode list of KVM architectures Peter Maydell 2012-08-02 15:41 ` Peter Maydell @ 2012-08-02 16:01 ` Andreas Färber 2012-08-02 16:22 ` Peter Maydell 1 sibling, 1 reply; 8+ messages in thread From: Andreas Färber @ 2012-08-02 16:01 UTC (permalink / raw) To: Peter Maydell Cc: Alexander Graf, Avi Kivity, Marcelo Tosatti, qemu-devel, patches 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: Don't implicitly hardcode list of KVM architectures 2012-08-02 16:01 ` Andreas Färber @ 2012-08-02 16:22 ` Peter Maydell 2012-08-02 16:29 ` Andreas Färber 0 siblings, 1 reply; 8+ messages in thread From: Peter Maydell @ 2012-08-02 16:22 UTC (permalink / raw) To: Andreas Färber Cc: Alexander Graf, Avi Kivity, Marcelo Tosatti, qemu-devel, patches 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: Don't implicitly hardcode list of KVM architectures 2012-08-02 16:22 ` Peter Maydell @ 2012-08-02 16:29 ` Andreas Färber 0 siblings, 0 replies; 8+ messages in thread From: Andreas Färber @ 2012-08-02 16:29 UTC (permalink / raw) To: Peter Maydell Cc: Alexander Graf, Avi Kivity, Marcelo Tosatti, qemu-devel, patches 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-08-02 20:37 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-07-18 11:10 [Qemu-devel] [PATCH] configure: Don't implicitly hardcode list of KVM architectures Peter Maydell 2012-08-02 15:41 ` Peter Maydell 2012-08-02 15:46 ` Andreas Färber 2012-08-02 15:53 ` Alexander Graf 2012-08-02 20:31 ` Marcelo Tosatti 2012-08-02 16:01 ` Andreas Färber 2012-08-02 16:22 ` Peter Maydell 2012-08-02 16:29 ` Andreas Färber
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).