virt-acpi-build.c uses warn_report. However, it doesn't include
qemu/error-report.h directly, it include qemu/error-report.h via trace.h
if we enable log trace backend. But if we disable the log trace backend
(e.g., --enable-trace-backends=nop), then virt-acpi-build.c will not
include qemu/error-report.h any more and it will lead to build errors.
Include qemu/error-report.h directly in virt-acpi-build.c to avoid the
errors.

Signed-off-by: Peng Liang <tcx4c70@gmail.com>
---
 hw/arm/virt-acpi-build.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 55f2706bc9..44cf9f3312 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -57,6 +57,7 @@
 #include "migration/vmstate.h"
 #include "hw/acpi/ghes.h"
 #include "hw/acpi/viot.h"
+#include "qemu/error-report.h"
 
 #define ARM_SPI_BASE 32
 
-- 
2.41.0

06.07.2023 18:08, Peng Liang wrote:
> virt-acpi-build.c uses warn_report. However, it doesn't include
> qemu/error-report.h directly, it include qemu/error-report.h via trace.h
> if we enable log trace backend. But if we disable the log trace backend
> (e.g., --enable-trace-backends=nop), then virt-acpi-build.c will not
> include qemu/error-report.h any more and it will lead to build errors.
> Include qemu/error-report.h directly in virt-acpi-build.c to avoid the
> errors.

Applied to my trivial-patches tree, moving it higher as suggested by Ani
(to sort more alphabetically) and adding the Fixes: line as suggested
by Philippe.

Thanks!

/mjt

On 6/7/23 17:08, Peng Liang wrote:
> virt-acpi-build.c uses warn_report. However, it doesn't include
> qemu/error-report.h directly, it include qemu/error-report.h via trace.h
> if we enable log trace backend. But if we disable the log trace backend
> (e.g., --enable-trace-backends=nop), then virt-acpi-build.c will not
> include qemu/error-report.h any more and it will lead to build errors.
> Include qemu/error-report.h directly in virt-acpi-build.c to avoid the
> errors.
> 

Fixes: 451b157041 ("acpi: Align the size to 128k")
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> Signed-off-by: Peng Liang <tcx4c70@gmail.com>
> ---
>   hw/arm/virt-acpi-build.c | 1 +
>   1 file changed, 1 insertion(+)

> On 06-Jul-2023, at 8:38 PM, Peng Liang <tcx4c70@gmail.com> wrote:
> 
> virt-acpi-build.c uses warn_report. However, it doesn't include
> qemu/error-report.h directly, it include qemu/error-report.h via trace.h
> if we enable log trace backend. But if we disable the log trace backend
> (e.g., --enable-trace-backends=nop), then virt-acpi-build.c will not
> include qemu/error-report.h any more and it will lead to build errors.
> Include qemu/error-report.h directly in virt-acpi-build.c to avoid the
> errors.

I am able to generate the following error on latest upstream QEMU:

../hw/arm/virt-acpi-build.c: In function ‘virt_acpi_build’:
../hw/arm/virt-acpi-build.c:1045:9: error: implicit declaration of function ‘warn_report’ [-Werror=implicit-function-declaration]
 1045 |         warn_report("ACPI table size %u exceeds %d bytes,"
      |         ^~~~~~~~~~~
../hw/arm/virt-acpi-build.c:1045:9: error: nested extern declaration of ‘warn_report’ [-Werror=nested-externs]
../hw/arm/virt-acpi-build.c:1048:9: error: implicit declaration of function ‘error_printf’ [-Werror=implicit-function-declaration]
 1048 |         error_printf("Try removing CPUs, NUMA nodes, memory slots"
      |         ^~~~~~~~~~~~
../hw/arm/virt-acpi-build.c:1048:9: error: nested extern declaration of ‘error_printf’ [-Werror=nested-externs]


> 
> Signed-off-by: Peng Liang <tcx4c70@gmail.com>
> ---
> hw/arm/virt-acpi-build.c | 1 +
> 1 file changed, 1 insertion(+)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 55f2706bc9..44cf9f3312 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -57,6 +57,7 @@
> #include "migration/vmstate.h"
> #include "hw/acpi/ghes.h"
> #include "hw/acpi/viot.h"
> +#include "qemu/error-report.h"

I would put this a little bit higher up  right after “qemu/bitmap.h” , like :

--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -29,6 +29,7 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/bitmap.h"
+#include "qemu/error-report.h"
 #include "trace.h"
 #include "hw/core/cpu.h"
 #include "target/arm/cpu.h”

but other than that, your change does fix the build.

Hence, modulo the above,

Reviewed-by: Ani Sinha <anisinha@redhat.com>


> 
> #define ARM_SPI_BASE 32
> 
> -- 
> 2.41.0
> 

I'm not sure whether the error has already been fixed by others or not. 
If so, please just ignore this patch.

On 07/06/2023 23:08, Peng Liang wrote:
> virt-acpi-build.c uses warn_report. However, it doesn't include
> qemu/error-report.h directly, it include qemu/error-report.h via trace.h
> if we enable log trace backend. But if we disable the log trace backend
> (e.g., --enable-trace-backends=nop), then virt-acpi-build.c will not
> include qemu/error-report.h any more and it will lead to build errors.
> Include qemu/error-report.h directly in virt-acpi-build.c to avoid the
> errors.
> 
> Signed-off-by: Peng Liang <tcx4c70@gmail.com>
> ---
>   hw/arm/virt-acpi-build.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 55f2706bc9..44cf9f3312 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -57,6 +57,7 @@
>   #include "migration/vmstate.h"
>   #include "hw/acpi/ghes.h"
>   #include "hw/acpi/viot.h"
> +#include "qemu/error-report.h"
>   
>   #define ARM_SPI_BASE 32
>