Thanks, Alex, for reviewing this one. I built for all the archs and it was
fine. Can you please share more about what environment builds are breaking? So,
I can test the changes for v2.
Regards,
Vikram
From: Alex Bennée <alex.ben...@linaro.org>
Date: Thursday, October 27, 2022 at 2:24 AM
To: Garhwal, Vikram <vikram.garh...@amd.com>
Cc: qemu-devel@nongnu.org <qemu-devel@nongnu.org>, Stabellini, Stefano
<stefano.stabell...@amd.com>, Stefano Stabellini <sstabell...@kernel.org>,
Anthony Perard <anthony.per...@citrix.com>, Paul Durrant <p...@xen.org>,
xen-de...@lists.xenproject.org <xen-de...@lists.xenproject.org>
Subject: Re: [PATCH v1 09/12] accel/xen/xen-all: export xenstore_record_dm_state
Vikram Garhwal <vikram.garh...@amd.com> writes:
> xenstore_record_dm_state() will also be used in aarch64 xenpv machine.
>
> Signed-off-by: Vikram Garhwal <vikram.garh...@amd.com>
> Signed-off-by: Stefano Stabellini <stefano.stabell...@amd.com>
> ---
> accel/xen/xen-all.c | 2 +-
> include/hw/xen/xen.h | 2 ++
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/accel/xen/xen-all.c b/accel/xen/xen-all.c
> index 69aa7d018b..276625b78b 100644
> --- a/accel/xen/xen-all.c
> +++ b/accel/xen/xen-all.c
> @@ -100,7 +100,7 @@ void xenstore_store_pv_console_info(int i, Chardev *chr)
> }
>
>
> -static void xenstore_record_dm_state(struct xs_handle *xs, const char *state)
> +void xenstore_record_dm_state(struct xs_handle *xs, const char *state)
> {
> char path[50];
>
> diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
> index afdf9c436a..31e9538a5c 100644
> --- a/include/hw/xen/xen.h
> +++ b/include/hw/xen/xen.h
> @@ -9,6 +9,7 @@
> */
>
> #include "exec/cpu-common.h"
> +#include <xenstore.h>
This is breaking a bunch of the builds and generally we try and avoid
adding system includes in headers (apart from osdep.h) for this reason.
In fact there is a comment just above to that fact.
I think you can just add struct xs_handle to typedefs.h (or maybe just
xen.h) and directly include xenstore.h in xen-all.c following the usual
rules:
https://qemu.readthedocs.io/en/latest/devel/style.html#include-directives
It might be worth doing an audit to see what else is including xen.h
needlessly or should be using sysemu/xen.h.
>
> /* xen-machine.c */
> enum xen_mode {
> @@ -31,5 +32,6 @@ qemu_irq *xen_interrupt_controller_init(void);
> void xenstore_store_pv_console_info(int i, Chardev *chr);
>
> void xen_register_framebuffer(struct MemoryRegion *mr);
> +void xenstore_record_dm_state(struct xs_handle *xs, const char *state);
>
> #endif /* QEMU_HW_XEN_H */
--
Alex Bennée
