[Qemu-devel] [PATCH 0/4] build: fix failure when host provides too old libfdt

[Qemu-devel] [PATCH 0/4] build: fix failure when host provides too old libfdt

[Philippe Mathieu-Daudé]

Hi, I already hit this problem on Shippable and tried to resolve it for 2.10;
and shamefully hit it again yesterday on a Debian 8 host with distrib libfdt
installed. I wondered how to fix this without root access and realized my
previous Shippable kludge wasn't the best fix :)

Here we go adding the QEMU_LDFLAGS to enforce local libfdt in library path
before the system one.
This time it looks like the correct fix, also scalable for other libraries.

It would be great to have QEMU 2.12 buildable in Debian<9, but nobody
complained since 2.10 so this can wait 2.13 (this is not a regression neither).

Regards,

Phil.

Jan Kiszka (1):
  build: Silence dtc directory creation

Philippe Mathieu-Daudé (3):
  configure: Really use local libfdt if the system one is too old
  configure: Display if libfdt is from system or git
  shippable: Remove Debian 8 libfdt kludge

 configure      | 15 +++++++++------
 Makefile       |  2 +-
 rules.mak      |  2 +-
 .shippable.yml |  8 --------
 4 files changed, 11 insertions(+), 16 deletions(-)

-- 
2.17.0

[Qemu-devel] [PATCH for-2.12? 1/4] configure: Really use local libfdt if the system one is too old

[Philippe Mathieu-Daudé]

QEMU requires libfdt version >= 1.4.2.
If the host has an older libfdt installed, the configure script will use
a (git cloned) local version.

Example with Debian 8:
    $ dpkg-query --showformat='${Version}\n' --show libfdt-dev
    1.4.0+dfsg-1
    $ ./configure
    [...]
    fdt support       yes          # from git submodule 'dtc'

If this case occurs, the linker will have 2 different libfdt available in
the library search path. The default behavior is to search the system path
first, then the local path.

Even if the configure script noticed the libfdt is too old and clone a more
recent locally, when linking the system library is selected first, and the
link process eventually fails:

      LINK    mips64el-softmmu/qemu-system-mips64el
    ../hw/core/loader-fit.o: In function `load_fit':
    /root/src/github.com/philmd/qemu/hw/core/loader-fit.c:278: undefined reference to `fdt_first_subnode'
    /root/src/github.com/philmd/qemu/hw/core/loader-fit.c:286: undefined reference to `fdt_next_subnode'
    /root/src/github.com/philmd/qemu/hw/core/loader-fit.c:277: undefined reference to `fdt_first_subnode'
    collect2: error: ld returned 1 exit status
    Makefile:201: recipe for target 'qemu-system-mips64el' failed
    make[1]: *** [qemu-system-mips64el] Error 1

QEMU already uses a kludge to enforce local CFLAGS before system ones for
libpixman and libfdt, add a similar kludge for the LDFLAGS to enforce using
the local libfdt.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 configure | 6 +++++-
 rules.mak | 2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 0a19b033bc..162e804b49 100755
--- a/configure
+++ b/configure
@@ -3773,7 +3773,8 @@ EOF
               symlink "$source_path/dtc/scripts" "dtc/scripts"
           fi
           fdt_cflags="-I\$(SRC_PATH)/dtc/libfdt"
-          fdt_libs="-L\$(BUILD_DIR)/dtc/libfdt $fdt_libs"
+          fdt_ldflags="-L\$(BUILD_DIR)/dtc/libfdt"
+          fdt_libs="$fdt_libs"
       elif test "$fdt" = "yes" ; then
           # Not a git build & no libfdt found, prompt for system install
           error_exit "DTC (libfdt) version >= 1.4.2 not present." \
@@ -5715,6 +5716,7 @@ echo_version() {
 
 # prepend pixman and ftd flags after all config tests are done
 QEMU_CFLAGS="$pixman_cflags $fdt_cflags $QEMU_CFLAGS"
+QEMU_LDFLAGS="$fdt_ldflags $QEMU_LDFLAGS"
 libs_softmmu="$pixman_libs $libs_softmmu"
 
 echo "Install prefix    $prefix"
@@ -5745,6 +5747,7 @@ echo "ARFLAGS           $ARFLAGS"
 echo "CFLAGS            $CFLAGS"
 echo "QEMU_CFLAGS       $QEMU_CFLAGS"
 echo "LDFLAGS           $LDFLAGS"
+echo "QEMU_LDFLAGS      $QEMU_LDFLAGS"
 echo "make              $make"
 echo "install           $install"
 echo "python            $python"
@@ -6679,6 +6682,7 @@ else
 fi
 echo "LDFLAGS=$LDFLAGS" >> $config_host_mak
 echo "LDFLAGS_NOPIE=$LDFLAGS_NOPIE" >> $config_host_mak
+echo "QEMU_LDFLAGS=$QEMU_LDFLAGS" >> $config_host_mak
 echo "LD_REL_FLAGS=$LD_REL_FLAGS" >> $config_host_mak
 echo "LD_I386_EMULATION=$ld_i386_emulation" >> $config_host_mak
 echo "LIBS+=$LIBS" >> $config_host_mak
diff --git a/rules.mak b/rules.mak
index 93a07027b0..04c7f74d07 100644
--- a/rules.mak
+++ b/rules.mak
@@ -73,7 +73,7 @@ expand-objs = $(strip $(sort $(filter %.o,$1)) \
 # must link with the C++ compiler, not the plain C compiler.
 LINKPROG = $(or $(CXX),$(CC))
 
-LINK = $(call quiet-command, $(LINKPROG) $(QEMU_CFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ \
+LINK = $(call quiet-command, $(LINKPROG) $(QEMU_LDFLAGS) $(QEMU_CFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ \
        $(call process-archive-undefs, $1) \
        $(version-obj-y) $(call extract-libs,$1) $(LIBS),"LINK","$(TARGET_DIR)$@")
 
-- 
2.17.0

Re: [Qemu-devel] [PATCH for-2.12? 1/4] configure: Really use local libfdt if the system one is too old

[Thomas Huth]

On 16.04.2018 01:05, Philippe Mathieu-Daudé wrote:
> QEMU requires libfdt version >= 1.4.2.
> If the host has an older libfdt installed, the configure script will use
> a (git cloned) local version.
> 
> Example with Debian 8:
>     $ dpkg-query --showformat='${Version}\n' --show libfdt-dev
>     1.4.0+dfsg-1
>     $ ./configure
>     [...]
>     fdt support       yes          # from git submodule 'dtc'
> 
> If this case occurs, the linker will have 2 different libfdt available in
> the library search path. The default behavior is to search the system path
> first, then the local path.
> 
> Even if the configure script noticed the libfdt is too old and clone a more
> recent locally, when linking the system library is selected first, and the
> link process eventually fails:
> 
>       LINK    mips64el-softmmu/qemu-system-mips64el
>     ../hw/core/loader-fit.o: In function `load_fit':
>     /root/src/github.com/philmd/qemu/hw/core/loader-fit.c:278: undefined reference to `fdt_first_subnode'
>     /root/src/github.com/philmd/qemu/hw/core/loader-fit.c:286: undefined reference to `fdt_next_subnode'
>     /root/src/github.com/philmd/qemu/hw/core/loader-fit.c:277: undefined reference to `fdt_first_subnode'
>     collect2: error: ld returned 1 exit status
>     Makefile:201: recipe for target 'qemu-system-mips64el' failed
>     make[1]: *** [qemu-system-mips64el] Error 1
> 
> QEMU already uses a kludge to enforce local CFLAGS before system ones for
> libpixman and libfdt, add a similar kludge for the LDFLAGS to enforce using
> the local libfdt.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  configure | 6 +++++-
>  rules.mak | 2 +-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/configure b/configure
> index 0a19b033bc..162e804b49 100755
> --- a/configure
> +++ b/configure
> @@ -3773,7 +3773,8 @@ EOF
>                symlink "$source_path/dtc/scripts" "dtc/scripts"
>            fi
>            fdt_cflags="-I\$(SRC_PATH)/dtc/libfdt"
> -          fdt_libs="-L\$(BUILD_DIR)/dtc/libfdt $fdt_libs"
> +          fdt_ldflags="-L\$(BUILD_DIR)/dtc/libfdt"
> +          fdt_libs="$fdt_libs"
>        elif test "$fdt" = "yes" ; then
>            # Not a git build & no libfdt found, prompt for system install
>            error_exit "DTC (libfdt) version >= 1.4.2 not present." \
> @@ -5715,6 +5716,7 @@ echo_version() {
>  
>  # prepend pixman and ftd flags after all config tests are done
>  QEMU_CFLAGS="$pixman_cflags $fdt_cflags $QEMU_CFLAGS"
> +QEMU_LDFLAGS="$fdt_ldflags $QEMU_LDFLAGS"
>  libs_softmmu="$pixman_libs $libs_softmmu"

I think the pixman lines could be removed now that we do not have an
internal pixman version anymore (but that's something for a separate patch)

>  echo "Install prefix    $prefix"
> @@ -5745,6 +5747,7 @@ echo "ARFLAGS           $ARFLAGS"
>  echo "CFLAGS            $CFLAGS"
>  echo "QEMU_CFLAGS       $QEMU_CFLAGS"
>  echo "LDFLAGS           $LDFLAGS"
> +echo "QEMU_LDFLAGS      $QEMU_LDFLAGS"
>  echo "make              $make"
>  echo "install           $install"
>  echo "python            $python"
> @@ -6679,6 +6682,7 @@ else
>  fi
>  echo "LDFLAGS=$LDFLAGS" >> $config_host_mak
>  echo "LDFLAGS_NOPIE=$LDFLAGS_NOPIE" >> $config_host_mak
> +echo "QEMU_LDFLAGS=$QEMU_LDFLAGS" >> $config_host_mak
>  echo "LD_REL_FLAGS=$LD_REL_FLAGS" >> $config_host_mak
>  echo "LD_I386_EMULATION=$ld_i386_emulation" >> $config_host_mak
>  echo "LIBS+=$LIBS" >> $config_host_mak
> diff --git a/rules.mak b/rules.mak
> index 93a07027b0..04c7f74d07 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -73,7 +73,7 @@ expand-objs = $(strip $(sort $(filter %.o,$1)) \
>  # must link with the C++ compiler, not the plain C compiler.
>  LINKPROG = $(or $(CXX),$(CC))
>  
> -LINK = $(call quiet-command, $(LINKPROG) $(QEMU_CFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ \
> +LINK = $(call quiet-command, $(LINKPROG) $(QEMU_LDFLAGS) $(QEMU_CFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ \
>         $(call process-archive-undefs, $1) \
>         $(version-obj-y) $(call extract-libs,$1) $(LIBS),"LINK","$(TARGET_DIR)$@")

Looks reasonable.

Reviewed-by: Thomas Huth <thuth@redhat.com>

[Qemu-devel] [PATCH 2/4] configure: Display if libfdt is from system or git

[Philippe Mathieu-Daudé]

The configure script outputs "yes" regardless which libfdt is used:

  ./configure
  [...]
  fdt support       yes

Sometimes you can have both system and local git version available,
change the configure script to display which library got selected:

  debian8$ dpkg-query --showformat='${Version}\n' --show libfdt-dev
  1.4.0+dfsg-1

  debian8$ ./configure
  [...]
  fdt support       git

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 configure | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/configure b/configure
index 162e804b49..056b391f22 100755
--- a/configure
+++ b/configure
@@ -3758,15 +3758,14 @@ int main(void) { fdt_first_subnode(0, 0); return 0; }
 EOF
   if compile_prog "" "$fdt_libs" ; then
     # system DTC is good - use it
-    fdt=yes
+    fdt=system
   else
       # have GIT checkout, so activate dtc submodule
       if test -e "${source_path}/.git" ; then
           git_submodules="${git_submodules} dtc"
       fi
       if test -d "${source_path}/dtc/libfdt" || test -e "${source_path}/.git" ; then
-          fdt=yes
-          dtc_internal="yes"
+          fdt=git
           mkdir -p dtc
           if [ "$pwd_is_source_path" != "y" ] ; then
               symlink "$source_path/dtc/Makefile" "dtc/Makefile"
@@ -6307,7 +6306,7 @@ fi
 if test "$preadv" = "yes" ; then
   echo "CONFIG_PREADV=y" >> $config_host_mak
 fi
-if test "$fdt" = "yes" ; then
+if test "$fdt" != "no" ; then
   echo "CONFIG_FDT=y" >> $config_host_mak
 fi
 if test "$membarrier" = "yes" ; then
@@ -7109,7 +7108,7 @@ echo "QEMU_CFLAGS+=$cflags" >> $config_target_mak
 
 done # for target in $targets
 
-if [ "$dtc_internal" = "yes" ]; then
+if [ "$fdt" = "git" ]; then
   echo "config-host.h: subdir-dtc" >> $config_host_mak
 fi
 if [ "$capstone" = "git" -o "$capstone" = "internal" ]; then
-- 
2.17.0

Re: [Qemu-devel] [PATCH 2/4] configure: Display if libfdt is from system or git

[Thomas Huth]

On 16.04.2018 01:05, Philippe Mathieu-Daudé wrote:
> The configure script outputs "yes" regardless which libfdt is used:
> 
>   ./configure
>   [...]
>   fdt support       yes
> 
> Sometimes you can have both system and local git version available,
> change the configure script to display which library got selected:
> 
>   debian8$ dpkg-query --showformat='${Version}\n' --show libfdt-dev
>   1.4.0+dfsg-1
> 
>   debian8$ ./configure
>   [...]
>   fdt support       git
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  configure | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/configure b/configure
> index 162e804b49..056b391f22 100755
> --- a/configure
> +++ b/configure
> @@ -3758,15 +3758,14 @@ int main(void) { fdt_first_subnode(0, 0); return 0; }
>  EOF
>    if compile_prog "" "$fdt_libs" ; then
>      # system DTC is good - use it
> -    fdt=yes
> +    fdt=system
>    else
>        # have GIT checkout, so activate dtc submodule
>        if test -e "${source_path}/.git" ; then
>            git_submodules="${git_submodules} dtc"
>        fi
>        if test -d "${source_path}/dtc/libfdt" || test -e "${source_path}/.git" ; then
> -          fdt=yes
> -          dtc_internal="yes"
> +          fdt=git

That's ok for git checkout, but what about release tarballs? I'd strange
to see "git" here. So maybe better use "internal" instead of "git" here?

OTOH, capstone is also using the term "git" here, so this is just
consistent here...

 Thomas

Re: [Qemu-devel] [PATCH 2/4] configure: Display if libfdt is from system or git

[Philippe Mathieu-Daudé]

Hi Thomas,

On 04/16/2018 02:16 AM, Thomas Huth wrote:
> On 16.04.2018 01:05, Philippe Mathieu-Daudé wrote:
>> The configure script outputs "yes" regardless which libfdt is used:
>>
>>   ./configure
>>   [...]
>>   fdt support       yes
>>
>> Sometimes you can have both system and local git version available,
>> change the configure script to display which library got selected:
>>
>>   debian8$ dpkg-query --showformat='${Version}\n' --show libfdt-dev
>>   1.4.0+dfsg-1
>>
>>   debian8$ ./configure
>>   [...]
>>   fdt support       git
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  configure | 9 ++++-----
>>  1 file changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/configure b/configure
>> index 162e804b49..056b391f22 100755
>> --- a/configure
>> +++ b/configure
>> @@ -3758,15 +3758,14 @@ int main(void) { fdt_first_subnode(0, 0); return 0; }
>>  EOF
>>    if compile_prog "" "$fdt_libs" ; then
>>      # system DTC is good - use it
>> -    fdt=yes
>> +    fdt=system
>>    else
>>        # have GIT checkout, so activate dtc submodule
>>        if test -e "${source_path}/.git" ; then
>>            git_submodules="${git_submodules} dtc"
>>        fi
>>        if test -d "${source_path}/dtc/libfdt" || test -e "${source_path}/.git" ; then
>> -          fdt=yes
>> -          dtc_internal="yes"
>> +          fdt=git
> 
> That's ok for git checkout, but what about release tarballs? I'd strange
> to see "git" here. So maybe better use "internal" instead of "git" here?
> 
> OTOH, capstone is also using the term "git" here, so this is just
> consistent here...

Having both use "internal" sounds a good cleanup.

[Qemu-devel] [PATCH 3/4] shippable: Remove Debian 8 libfdt kludge

[Philippe Mathieu-Daudé]

This kludge was added in a825ca06137, but a cleaner and more generic
fix is now available (see ##COMMIT_CONFIGURE_LIBFDT_LDFLAGS_SHA##).

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
If a maintainer take the whole series, I hope he'll be kind enough to update
##COMMIT_CONFIGURE_LIBFDT_LDFLAGS_SHA## with the sha of the patch #1 :) Thanks!

 .shippable.yml | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/.shippable.yml b/.shippable.yml
index 60f2ce9218..f74a3de3ff 100644
--- a/.shippable.yml
+++ b/.shippable.yml
@@ -35,13 +35,5 @@ build:
     options: "-e HOME=/root"
   ci:
     - unset CC
-    # some targets require newer up to date packages, for example TARGET_LIST matching
-    # aarch64*-softmmu|arm*-softmmu|ppc*-softmmu|microblaze*-softmmu|mips64el-softmmu)
-    # see the configure script:
-    #    error_exit "DTC (libfdt) version >= 1.4.2 not present. Your options:"
-    #    "  (1) Preferred: Install the DTC (libfdt) devel package"
-    #    "  (2) Fetch the DTC submodule, using:"
-    #    "      git submodule update --init dtc"
-    - dpkg --compare-versions `dpkg-query --showformat='${Version}' --show libfdt-dev` ge 1.4.2 || git submodule update --init dtc
     - ./configure ${QEMU_CONFIGURE_OPTS} --target-list=${TARGET_LIST}
     - make -j$(($(getconf _NPROCESSORS_ONLN) + 1))
-- 
2.17.0

[Qemu-devel] [PATCH 4/4] build: Silence dtc directory creation

[Philippe Mathieu-Daudé]

From: Jan Kiszka <jan.kiszka@siemens.com>

Align with other mkdir calls.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <0dd4c8f5-d60e-e564-652f-cd0101f6ee68@web.de>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 727ef118f3..fc6f21001f 100644
--- a/Makefile
+++ b/Makefile
@@ -485,7 +485,7 @@ subdir-dtc: .git-submodule-status dtc/libfdt dtc/tests
 	$(call quiet-command,$(MAKE) $(DTC_MAKE_ARGS) CPPFLAGS="$(DTC_CPPFLAGS)" CFLAGS="$(DTC_CFLAGS)" LDFLAGS="$(LDFLAGS)" ARFLAGS="$(ARFLAGS)" CC="$(CC)" AR="$(AR)" LD="$(LD)" $(SUBDIR_MAKEFLAGS) libfdt/libfdt.a,)
 
 dtc/%: .git-submodule-status
-	mkdir -p $@
+	@mkdir -p $@
 
 # Overriding CFLAGS causes us to lose defines added in the sub-makefile.
 # Not overriding CFLAGS leads to mis-matches between compilation modes.
-- 
2.17.0

Re: [Qemu-devel] [PATCH 0/4] build: fix failure when host provides too old libfdt

[Paolo Bonzini]

On 16/04/2018 01:05, Philippe Mathieu-Daudé wrote:
> Hi, I already hit this problem on Shippable and tried to resolve it for 2.10;
> and shamefully hit it again yesterday on a Debian 8 host with distrib libfdt
> installed. I wondered how to fix this without root access and realized my
> previous Shippable kludge wasn't the best fix :)
> 
> Here we go adding the QEMU_LDFLAGS to enforce local libfdt in library path
> before the system one.
> This time it looks like the correct fix, also scalable for other libraries.
> 
> It would be great to have QEMU 2.12 buildable in Debian<9, but nobody
> complained since 2.10 so this can wait 2.13 (this is not a regression neither).
> 
> Regards,
> 
> Phil.
> 
> Jan Kiszka (1):
>   build: Silence dtc directory creation
> 
> Philippe Mathieu-Daudé (3):
>   configure: Really use local libfdt if the system one is too old
>   configure: Display if libfdt is from system or git
>   shippable: Remove Debian 8 libfdt kludge
> 
>  configure      | 15 +++++++++------
>  Makefile       |  2 +-
>  rules.mak      |  2 +-
>  .shippable.yml |  8 --------
>  4 files changed, 11 insertions(+), 16 deletions(-)
> 

Queued all four, thanks!

Paolo