Re: [PATCH v1 09/12] accel/xen/xen-all: export xenstore_record_dm_state

[Alex Bennée <alex.bennee@xxxxxxxxxx>]

"Garhwal, Vikram" <vikram.garhwal@xxxxxxx> writes:

> 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.

My cross build environment failed:

  ../../configure' '--disable-docs' '--disable-tools' 
'--cross-prefix=aarch64-linux-gnu-' '--enable-xen' 
'--target-list=i386-softmmu,x86_64-softmmu,arm-softmmu,aarch64-softmmu' 
'--disable-tpm'

On a Debian Bullseye with:

  11:30:20 [root@zen:~] # dpkg -l libxen\*
  Desired=Unknown/Install/Remove/Purge/Hold
  | Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
  |/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
  ||/ Name                       Version                 Architecture 
Description
  
+++-==========================-=======================-============-====================================================
  ii  libxen-dev:arm64           4.14.5+24-g87d90d511c-1 arm64        Public 
headers and libs for Xen
  ii  libxencall1:amd64          4.14.5+24-g87d90d511c-1 amd64        Xen 
runtime library - libxencall
  ii  libxencall1:arm64          4.14.5+24-g87d90d511c-1 arm64        Xen 
runtime library - libxencall
  ii  libxendevicemodel1:amd64   4.14.5+24-g87d90d511c-1 amd64        Xen 
runtime libraries - libxendevicemodel
  ii  libxendevicemodel1:arm64   4.14.5+24-g87d90d511c-1 arm64        Xen 
runtime libraries - libxendevicemodel
  ii  libxenevtchn1:amd64        4.14.5+24-g87d90d511c-1 amd64        Xen 
runtime libraries - libxenevtchn
  ii  libxenevtchn1:arm64        4.14.5+24-g87d90d511c-1 arm64        Xen 
runtime libraries - libxenevtchn
  ii  libxenforeignmemory1:amd64 4.14.5+24-g87d90d511c-1 amd64        Xen 
runtime libraries - libxenforeignmemory
  ii  libxenforeignmemory1:arm64 4.14.5+24-g87d90d511c-1 arm64        Xen 
runtime libraries - libxenforeignmemory
  ii  libxengnttab1:amd64        4.14.5+24-g87d90d511c-1 amd64        Xen 
runtime libraries - libxengnttab
  ii  libxengnttab1:arm64        4.14.5+24-g87d90d511c-1 arm64        Xen 
runtime libraries - libxengnttab
  ii  libxenhypfs1:amd64         4.14.5+24-g87d90d511c-1 amd64        Xen 
runtime library - libxenhypfs
  ii  libxenhypfs1:arm64         4.14.5+24-g87d90d511c-1 arm64        Xen 
runtime library - libxenhypfs
  ii  libxenmisc4.14:amd64       4.14.5+24-g87d90d511c-1 amd64        Xen 
runtime libraries - miscellaneous, versioned ABI
  ii  libxenmisc4.14:arm64       4.14.5+24-g87d90d511c-1 arm64        Xen 
runtime libraries - miscellaneous, versioned ABI
  ii  libxenstore3.0:amd64       4.14.5+24-g87d90d511c-1 amd64        Xen 
runtime libraries - libxenstore
  ii  libxenstore3.0:arm64       4.14.5+24-g87d90d511c-1 arm64        Xen 
runtime libraries - libxenstore
  ii  libxentoolcore1:amd64      4.14.5+24-g87d90d511c-1 amd64        Xen 
runtime libraries - libxentoolcore
  ii  libxentoolcore1:arm64      4.14.5+24-g87d90d511c-1 arm64        Xen 
runtime libraries - libxentoolcore
  ii  libxentoollog1:amd64       4.14.5+24-g87d90d511c-1 amd64        Xen 
runtime libraries - libxentoollog
  ii  libxentoollog1:arm64       4.14.5+24-g87d90d511c-1 arm64        Xen 
runtime libraries - libxentoollog

But also a bunch of cross builds on the CI system:

  https://gitlab.com/stsquad/qemu/-/pipelines/677956972/failures

>
>  
>
> Regards,
>
> Vikram
>
>  
>
> From: Alex Bennée <alex.bennee@xxxxxxxxxx>
> Date: Thursday, October 27, 2022 at 2:24 AM
> To: Garhwal, Vikram <vikram.garhwal@xxxxxxx>
> Cc: qemu-devel@xxxxxxxxxx <qemu-devel@xxxxxxxxxx>, Stabellini, Stefano 
> <stefano.stabellini@xxxxxxx>, Stefano
> Stabellini <sstabellini@xxxxxxxxxx>, Anthony Perard 
> <anthony.perard@xxxxxxxxxx>, Paul Durrant <paul@xxxxxxx>,
> xen-devel@xxxxxxxxxxxxxxxxxxxx <xen-devel@xxxxxxxxxxxxxxxxxxxx>
> Subject: Re: [PATCH v1 09/12] accel/xen/xen-all: export 
> xenstore_record_dm_state
>
> Vikram Garhwal <vikram.garhwal@xxxxxxx> writes:
>
>> xenstore_record_dm_state() will also be used in aarch64 xenpv machine.
>>
>> Signed-off-by: Vikram Garhwal <vikram.garhwal@xxxxxxx>
>> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxx>
>> ---
>>  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