[xen-unstable-smoke test] 175226: regressions - FAIL

[xen-unstable-smoke test] 175226: regressions - FAIL

[osstest service owner]

flight 175226 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/175226/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf                   6 xen-build                fail REGR. vs. 175173

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl           1 build-check(1)               blocked  n/a
 test-amd64-amd64-libvirt     15 migrate-support-check        fail   never pass
 test-arm64-arm64-xl-xsm      15 migrate-support-check        fail   never pass
 test-arm64-arm64-xl-xsm      16 saverestore-support-check    fail   never pass

version targeted for testing:
 xen                  549b042943a57b748ce80070d1174e4ff5b8ef0b
baseline version:
 xen                  630dc3798e1d0d1b95f7be8b176563eb40e866e5

Last test of basis   175173  2022-12-13 15:00:27 Z    1 days
Testing same since   175199  2022-12-14 12:01:52 Z    0 days    6 attempts

------------------------------------------------------------
People who touched revisions under test:
  Demi Marie Obenour <demi@invisiblethingslab.com>
  Juergen Gross <jgross@suse.com>
  Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
  Viresh Kumar <viresh.kumar@linaro.org>

jobs:
 build-arm64-xsm                                              pass    
 build-amd64                                                  pass    
 build-armhf                                                  fail    
 build-amd64-libvirt                                          pass    
 test-armhf-armhf-xl                                          blocked 
 test-arm64-arm64-xl-xsm                                      pass    
 test-amd64-amd64-xl-qemuu-debianhvm-amd64                    pass    
 test-amd64-amd64-libvirt                                     pass    


------------------------------------------------------------
sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
    http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
    http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
    http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
    http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Not pushing.

------------------------------------------------------------
commit 549b042943a57b748ce80070d1174e4ff5b8ef0b
Author: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Date:   Wed Dec 14 12:04:26 2022 +0100

    drivers/char: support up to 1M BAR0 of xhci
    
    AMD's XHCI has BAR0 of 1M (compared to 64K on Intel). Map it as a whole
    (reserving more space in the fixmap). Make fixmap slot conditional on
    CONFIG_XHCI.
    
    Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
    Reviewed-by: Jan Beulich <jbeulich@suse.com>

commit dd54ea500be80f347402d75f3e4e7061e7db78d2
Author: Viresh Kumar <viresh.kumar@linaro.org>
Date:   Wed Dec 14 12:03:38 2022 +0100

    docs: add documentation for generic virtio devices
    
    This patch updates xl.cfg man page with details of generic Virtio device
    related information.
    
    Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
    Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
    Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

commit 953efa0b7b188458b18e4a727f3b1dfa77eacb61
Author: Viresh Kumar <viresh.kumar@linaro.org>
Date:   Wed Dec 14 12:03:25 2022 +0100

    xl: add support to parse generic virtio device
    
    This patch adds basic support for parsing generic Virtio backend.
    
    An example of domain configuration for mmio based Virtio I2C device is:
    virtio = ["type=virtio,device22,transport=mmio"]
    
    Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
    Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

commit 43ba5202e2eebd350161a8db674bf928c3e6f859
Author: Viresh Kumar <viresh.kumar@linaro.org>
Date:   Wed Dec 14 12:03:09 2022 +0100

    libxl: add support for generic virtio device
    
    This patch adds basic support for configuring and assisting generic
    Virtio backends, which could run in any domain.
    
    An example of domain configuration for mmio based Virtio I2C device is:
    virtio = ["type=virtio,device22,transport=mmio"]
    
    To make this work on Arm, allocate Virtio MMIO params (IRQ and memory
    region) and pass them to the backend and update guest device-tree to
    create a DT node for the Virtio devices.
    
    Add special support for I2C and GPIO devices, which require the
    "compatible" DT property to be set, among other device specific
    properties. Support for generic virtio devices is also added, which just
    need a MMIO node but not any special DT properties, for such devices the
    user needs to pass "virtio,device" in the "type" string.
    
    The parsing of generic virtio device configurations will be done in a
    separate commit.
    
    Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
    Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
    Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

commit db75092aea988b4be78c8273626f2ee40b4012b8
Author: Juergen Gross <jgross@suse.com>
Date:   Wed Dec 14 12:02:21 2022 +0100

    tools/xenstore: enhance hashtable implementation
    
    Today it is possible to set a flag when calling hashtable_destroy() in
    order to specify whether the data associated with the hashtable entries
    should be freed or not. The keys of the entries will always be freed.
    
    Change that by replacing the flag of hashtable_destroy() by two flags
    for create_hashtable() which will specify whether the data and/or the
    key of each entry should be freed or not.
    
    This will enable users to have the key e.g. as part of the data.
    
    Add a new function hashtable_iterate() to call a user specified
    function for each entry in the hashtable.
    
    Add new primes to the primetable in order to support smaller sizes of
    the hashtable. The primes are selected according to:
    
    https://planetmath.org/goodhashtableprimes
    
    Update the URL in the source as the old one wasn't correct any longer.
    
    Signed-off-by: Juergen Gross <jgross@suse.com>
    Reviewed-by: Julien Grall <jgrall@amazon.com>

commit bb65cbd81caaaaf325d23f63b4c2165960563459
Author: Juergen Gross <jgross@suse.com>
Date:   Wed Dec 14 12:02:04 2022 +0100

    tools/xenstore: preserve errno across corrupt()
    
    Let corrupt() preserve errno in order to be able to simplify error
    handling in future.
    
    This is rather easy as the errno value when entering corrupt() is
    saved already.
    
    Signed-off-by: Juergen Gross <jgross@suse.com>
    Reviewed-by: Julien Grall <jgrall@amazon.com>

commit 6a0d1914f0b19742436367a0019602992573bd4b
Author: Juergen Gross <jgross@suse.com>
Date:   Wed Dec 14 12:01:47 2022 +0100

    tools/xenstore: let tdb_logger() preserve errno
    
    tdb_logger() is called by TDB for logging errors. As errno is checked
    often after doing the logging, tdb_logger() should preserve errno.
    
    Signed-off-by: Juergen Gross <jgross@suse.com>
    Reviewed-by: Julien Grall <jgrall@amazon.com>

commit 8d7acf3f7d8d2555c78421dced45bc49f79ae806
Author: Demi Marie Obenour <demi@invisiblethingslab.com>
Date:   Wed Dec 14 12:00:35 2022 +0100

    EFI: relocate the ESRT when booting via multiboot2
    
    This was missed in the initial patchset.
    
    Move efi_relocate_esrt() up to avoid adding a forward declaration.
    
    Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
    Reviewed-by: Jan Beulich <jbeulich@suse.com>
(qemu changes not included)

Re: [xen-unstable-smoke test] 175226: regressions - FAIL

[Julien Grall]

Hi,

On 15/12/2022 01:41, osstest service owner wrote:
> flight 175226 xen-unstable-smoke real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/175226/
> 
> Regressions :-(
> 
> Tests which did not succeed and are blocking,
> including tests which could not be run:
>   build-armhf                   6 xen-build                fail REGR. vs. 175173

armhf/xen/tools/libs/light/../../../tools/config.h  -c -o libxl_genid.o 
libxl_genid.c
In file included from libxl_virtio.c:15:
libxl_virtio.c: In function 'libxl__set_xenstore_virtio':
libxl_internal.h:4388:51: error: format '%lu' expects argument of type 
'long unsigned int', but argument 3 has type 'uint64_t' {aka 'long long 
unsigned int'} [-Werror=format=]
  #define GCSPRINTF(fmt, ...) (libxl__sprintf((gc), (fmt), __VA_ARGS__))
                                                    ^~~~~
libxl_virtio.c:48:41: note: in expansion of macro 'GCSPRINTF'
      flexarray_append_pair(back, "base", GCSPRINTF("%lu", virtio->base));
                                          ^~~~~~~~~
libxl_internal.h:4388:51: error: format '%lu' expects argument of type 
'long unsigned int', but argument 3 has type 'uint64_t' {aka 'long long 
unsigned int'} [-Werror=format=]
  #define GCSPRINTF(fmt, ...) (libxl__sprintf((gc), (fmt), __VA_ARGS__))
                                                    ^~~~~
libxl_virtio.c:53:42: note: in expansion of macro 'GCSPRINTF'
      flexarray_append_pair(front, "base", GCSPRINTF("%lu", virtio->base));
                                           ^~~~~~~~~
cc1: all warnings being treated as errors
make[5]: *** 
[/home/osstest/build.175251.build-armhf/xen/tools/libs/light/../../../tools/Rules.mk:188: 
libxl_virtio.o] Error 1


This build breakage was introduced by "libxl: add support for generic 
virtio device". %lu will likely want to be switched to PRIx64.

Viresh, we need to unblock OSStest (our CI) as soon as possible. So can 
you look at it and confirm the rest of the tools build on arm32?

Cheers,

-- 
Julien Grall

Re: [xen-unstable-smoke test] 175226: regressions - FAIL

[Michal Orzel]

On 15/12/2022 09:34, Julien Grall wrote:
> 
> 
> Hi,
> 
> On 15/12/2022 01:41, osstest service owner wrote:
>> flight 175226 xen-unstable-smoke real [real]
>> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Flogs.test-lab.xenproject.org%2Fosstest%2Flogs%2F175226%2F&data=05%7C01%7Cmichal.orzel%40amd.com%7Cb8049c55691342b50a6608dade773f6f%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638066901002944667%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=GVUWKlOkecaJRZTcUqovWnc8WggDc4VokO%2BMKvHk9Qk%3D&reserved=0
>>
>> Regressions :-(
>>
>> Tests which did not succeed and are blocking,
>> including tests which could not be run:
>>   build-armhf                   6 xen-build                fail REGR. vs. 175173
> 
> armhf/xen/tools/libs/light/../../../tools/config.h  -c -o libxl_genid.o
> libxl_genid.c
> In file included from libxl_virtio.c:15:
> libxl_virtio.c: In function 'libxl__set_xenstore_virtio':
> libxl_internal.h:4388:51: error: format '%lu' expects argument of type
> 'long unsigned int', but argument 3 has type 'uint64_t' {aka 'long long
> unsigned int'} [-Werror=format=]
>   #define GCSPRINTF(fmt, ...) (libxl__sprintf((gc), (fmt), __VA_ARGS__))
>                                                     ^~~~~
> libxl_virtio.c:48:41: note: in expansion of macro 'GCSPRINTF'
>       flexarray_append_pair(back, "base", GCSPRINTF("%lu", virtio->base));
>                                           ^~~~~~~~~
> libxl_internal.h:4388:51: error: format '%lu' expects argument of type
> 'long unsigned int', but argument 3 has type 'uint64_t' {aka 'long long
> unsigned int'} [-Werror=format=]
>   #define GCSPRINTF(fmt, ...) (libxl__sprintf((gc), (fmt), __VA_ARGS__))
>                                                     ^~~~~
> libxl_virtio.c:53:42: note: in expansion of macro 'GCSPRINTF'
>       flexarray_append_pair(front, "base", GCSPRINTF("%lu", virtio->base));
>                                            ^~~~~~~~~
> cc1: all warnings being treated as errors
> make[5]: ***
> [/home/osstest/build.175251.build-armhf/xen/tools/libs/light/../../../tools/Rules.mk:188:
> libxl_virtio.o] Error 1
> 
> 
> This build breakage was introduced by "libxl: add support for generic
> virtio device". %lu will likely want to be switched to PRIx64.
> 
> Viresh, we need to unblock OSStest (our CI) as soon as possible. So can
> you look at it and confirm the rest of the tools build on arm32?
The failure is also observed on all the x86_32 builds:
https://gitlab.com/xen-project/xen/-/pipelines/722195904

> 
> Cheers,
> 
> --
> Julien Grall
> 

~Michal

[PATCH] libxl: virtio: Fix build error for 32-bit platforms

[Viresh Kumar]

The field 'base' in 'struct libxl_device_virtio' is defined as uint64,
while we are printing it with '%lu', which is 32bit only 32-bit
platforms. And so generates a error like:

  libxl_internal.h:4388:51: error: format '%lu' expects argument of type 'long
  unsigned int', but argument 3 has type 'uint64_t' {aka 'long long unsigned
  int'} [-Werror=format=]

Fix the same by using PRIx64 instead.

Now that the base name is available in hexadecimal format, prefix it
with '0x' as well, which strtoul() also depends upon since base passed
is 0.

Fixes: 43ba5202e2ee ("libxl: add support for generic virtio device")
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
Couldn't test on 32-bit platforms yet, but works fine for 64 bit one.

 tools/libs/light/libxl_virtio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/libs/light/libxl_virtio.c b/tools/libs/light/libxl_virtio.c
index 6a38def2faf5..2217bda8a253 100644
--- a/tools/libs/light/libxl_virtio.c
+++ b/tools/libs/light/libxl_virtio.c
@@ -45,12 +45,12 @@ static int libxl__set_xenstore_virtio(libxl__gc *gc, uint32_t domid,
     const char *transport = libxl_virtio_transport_to_string(virtio->transport);
 
     flexarray_append_pair(back, "irq", GCSPRINTF("%u", virtio->irq));
-    flexarray_append_pair(back, "base", GCSPRINTF("%lu", virtio->base));
+    flexarray_append_pair(back, "base", GCSPRINTF("0x%"PRIx64, virtio->base));
     flexarray_append_pair(back, "type", GCSPRINTF("%s", virtio->type));
     flexarray_append_pair(back, "transport", GCSPRINTF("%s", transport));
 
     flexarray_append_pair(front, "irq", GCSPRINTF("%u", virtio->irq));
-    flexarray_append_pair(front, "base", GCSPRINTF("%lu", virtio->base));
+    flexarray_append_pair(front, "base", GCSPRINTF("0x%"PRIx64, virtio->base));
     flexarray_append_pair(front, "type", GCSPRINTF("%s", virtio->type));
     flexarray_append_pair(front, "transport", GCSPRINTF("%s", transport));
 
-- 
2.31.1.272.g89b43f80a514

Re: [xen-unstable-smoke test] 175226: regressions - FAIL

[Viresh Kumar]

On 15-12-22, 08:34, Julien Grall wrote:
> This build breakage was introduced by "libxl: add support for generic virtio
> device". %lu will likely want to be switched to PRIx64.
> 
> Viresh, we need to unblock OSStest (our CI) as soon as possible. So can you
> look at it and confirm the rest of the tools build on arm32?

I am trying 32 bit build for the first time today and still haven't
got it fully working. I have sent a patch though which shall fix
this issue, will keep on getting the 32 bit build up and running.

-- 
viresh

Re: [PATCH] libxl: virtio: Fix build error for 32-bit platforms

[Anthony PERARD]

On Thu, Dec 15, 2022 at 07:01:40PM +0530, Viresh Kumar wrote:
> The field 'base' in 'struct libxl_device_virtio' is defined as uint64,
> while we are printing it with '%lu', which is 32bit only 32-bit
> platforms. And so generates a error like:
> 
>   libxl_internal.h:4388:51: error: format '%lu' expects argument of type 'long
>   unsigned int', but argument 3 has type 'uint64_t' {aka 'long long unsigned
>   int'} [-Werror=format=]
> 
> Fix the same by using PRIx64 instead.
> 
> Now that the base name is available in hexadecimal format, prefix it
> with '0x' as well, which strtoul() also depends upon since base passed
> is 0.
> 
> Fixes: 43ba5202e2ee ("libxl: add support for generic virtio device")
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> Couldn't test on 32-bit platforms yet, but works fine for 64 bit one.
> 
>  tools/libs/light/libxl_virtio.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libs/light/libxl_virtio.c b/tools/libs/light/libxl_virtio.c
> index 6a38def2faf5..2217bda8a253 100644
> --- a/tools/libs/light/libxl_virtio.c
> +++ b/tools/libs/light/libxl_virtio.c
> @@ -45,12 +45,12 @@ static int libxl__set_xenstore_virtio(libxl__gc *gc, uint32_t domid,
>      const char *transport = libxl_virtio_transport_to_string(virtio->transport);
>  
>      flexarray_append_pair(back, "irq", GCSPRINTF("%u", virtio->irq));
> -    flexarray_append_pair(back, "base", GCSPRINTF("%lu", virtio->base));
> +    flexarray_append_pair(back, "base", GCSPRINTF("0x%"PRIx64, virtio->base));

There is also PRIu64 that exist, which would be perfect to replace %u.
Could we use that instead?

I'd rather not have to think about which base is used for numbers in
xenstore. I can't find any hexadecimal numbers in xenstore for a simple
guest at the moment, so probably best to avoid adding one. And using
hexadecimal isn't needed to fix the build.

Thanks,

-- 
Anthony PERARD

Re: [xen-unstable-smoke test] 175226: regressions - FAIL

[Anthony PERARD]

On Thu, Dec 15, 2022 at 07:03:03PM +0530, Viresh Kumar wrote:
> On 15-12-22, 08:34, Julien Grall wrote:
> > This build breakage was introduced by "libxl: add support for generic virtio
> > device". %lu will likely want to be switched to PRIx64.
> > 
> > Viresh, we need to unblock OSStest (our CI) as soon as possible. So can you
> > look at it and confirm the rest of the tools build on arm32?
> 
> I am trying 32 bit build for the first time today and still haven't
> got it fully working. I have sent a patch though which shall fix
> this issue, will keep on getting the 32 bit build up and running.

You could just use our containers (that Gitlab CI uses) and run
something like that:

    CONTAINER=registry.gitlab.com/xen-project/xen/debian:unstable-i386 automation/scripts/containerize env XEN_TARGET_ARCH=x86_32 debug=n CC=gcc CXX=g++ automation/scripts/build

That would be something similar to what Gitlab CI is running.
And to get a shell:
    CONTAINER=registry.gitlab.com/xen-project/xen/debian:unstable-i386 automation/scripts/containerize

Cheers,

-- 
Anthony PERARD

Re: [PATCH] libxl: virtio: Fix build error for 32-bit platforms

[Jan Beulich]

On 15.12.2022 14:48, Anthony PERARD wrote:
> On Thu, Dec 15, 2022 at 07:01:40PM +0530, Viresh Kumar wrote:
>> The field 'base' in 'struct libxl_device_virtio' is defined as uint64,
>> while we are printing it with '%lu', which is 32bit only 32-bit
>> platforms. And so generates a error like:
>>
>>   libxl_internal.h:4388:51: error: format '%lu' expects argument of type 'long
>>   unsigned int', but argument 3 has type 'uint64_t' {aka 'long long unsigned
>>   int'} [-Werror=format=]
>>
>> Fix the same by using PRIx64 instead.
>>
>> Now that the base name is available in hexadecimal format, prefix it
>> with '0x' as well, which strtoul() also depends upon since base passed
>> is 0.
>>
>> Fixes: 43ba5202e2ee ("libxl: add support for generic virtio device")
>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>> ---
>> Couldn't test on 32-bit platforms yet, but works fine for 64 bit one.
>>
>>  tools/libs/light/libxl_virtio.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/libs/light/libxl_virtio.c b/tools/libs/light/libxl_virtio.c
>> index 6a38def2faf5..2217bda8a253 100644
>> --- a/tools/libs/light/libxl_virtio.c
>> +++ b/tools/libs/light/libxl_virtio.c
>> @@ -45,12 +45,12 @@ static int libxl__set_xenstore_virtio(libxl__gc *gc, uint32_t domid,
>>      const char *transport = libxl_virtio_transport_to_string(virtio->transport);
>>  
>>      flexarray_append_pair(back, "irq", GCSPRINTF("%u", virtio->irq));
>> -    flexarray_append_pair(back, "base", GCSPRINTF("%lu", virtio->base));
>> +    flexarray_append_pair(back, "base", GCSPRINTF("0x%"PRIx64, virtio->base));
> 
> There is also PRIu64 that exist, which would be perfect to replace %u.
> Could we use that instead?
> 
> I'd rather not have to think about which base is used for numbers in
> xenstore. I can't find any hexadecimal numbers in xenstore for a simple
> guest at the moment, so probably best to avoid adding one. And using
> hexadecimal isn't needed to fix the build.

Otoh an address formatted in decimal is pretty unusable to any human
(who might be inspecting xenstore for whatever reasons).

Jan

Re: [PATCH] libxl: virtio: Fix build error for 32-bit platforms

[Jan Beulich]

On 15.12.2022 14:31, Viresh Kumar wrote:
> The field 'base' in 'struct libxl_device_virtio' is defined as uint64,
> while we are printing it with '%lu', which is 32bit only 32-bit
> platforms. And so generates a error like:
> 
>   libxl_internal.h:4388:51: error: format '%lu' expects argument of type 'long
>   unsigned int', but argument 3 has type 'uint64_t' {aka 'long long unsigned
>   int'} [-Werror=format=]
> 
> Fix the same by using PRIx64 instead.
> 
> Now that the base name is available in hexadecimal format, prefix it
> with '0x' as well,

Which might better be done using "%#"PRIx64 ...

Jan

Re: [PATCH] libxl: virtio: Fix build error for 32-bit platforms

[Andrew Cooper]

On 15/12/2022 3:11 pm, Jan Beulich wrote:
> On 15.12.2022 14:48, Anthony PERARD wrote:
>> On Thu, Dec 15, 2022 at 07:01:40PM +0530, Viresh Kumar wrote:
>>> The field 'base' in 'struct libxl_device_virtio' is defined as uint64,
>>> while we are printing it with '%lu', which is 32bit only 32-bit
>>> platforms. And so generates a error like:
>>>
>>>   libxl_internal.h:4388:51: error: format '%lu' expects argument of type 'long
>>>   unsigned int', but argument 3 has type 'uint64_t' {aka 'long long unsigned
>>>   int'} [-Werror=format=]
>>>
>>> Fix the same by using PRIx64 instead.
>>>
>>> Now that the base name is available in hexadecimal format, prefix it
>>> with '0x' as well, which strtoul() also depends upon since base passed
>>> is 0.
>>>
>>> Fixes: 43ba5202e2ee ("libxl: add support for generic virtio device")
>>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>>> ---
>>> Couldn't test on 32-bit platforms yet, but works fine for 64 bit one.
>>>
>>>  tools/libs/light/libxl_virtio.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tools/libs/light/libxl_virtio.c b/tools/libs/light/libxl_virtio.c
>>> index 6a38def2faf5..2217bda8a253 100644
>>> --- a/tools/libs/light/libxl_virtio.c
>>> +++ b/tools/libs/light/libxl_virtio.c
>>> @@ -45,12 +45,12 @@ static int libxl__set_xenstore_virtio(libxl__gc *gc, uint32_t domid,
>>>      const char *transport = libxl_virtio_transport_to_string(virtio->transport);
>>>  
>>>      flexarray_append_pair(back, "irq", GCSPRINTF("%u", virtio->irq));
>>> -    flexarray_append_pair(back, "base", GCSPRINTF("%lu", virtio->base));
>>> +    flexarray_append_pair(back, "base", GCSPRINTF("0x%"PRIx64, virtio->base));
>> There is also PRIu64 that exist, which would be perfect to replace %u.
>> Could we use that instead?
>>
>> I'd rather not have to think about which base is used for numbers in
>> xenstore. I can't find any hexadecimal numbers in xenstore for a simple
>> guest at the moment, so probably best to avoid adding one. And using
>> hexadecimal isn't needed to fix the build.
> Otoh an address formatted in decimal is pretty unusable to any human
> (who might be inspecting xenstore for whatever reasons).

A consumer of xenstore has to cope with all bases anyway.  Anything that
doesn't is broken.

Keys ought to be expressed in the logical base for a human to read, and
hex for base is the right one in this case.

~Andrew

Re: [PATCH] libxl: virtio: Fix build error for 32-bit platforms

[Anthony PERARD]

On Thu, Dec 15, 2022 at 04:58:02PM +0000, Andrew Cooper wrote:
> A consumer of xenstore has to cope with all bases anyway.  Anything that
> doesn't is broken.

So libxl is broken, that good to know :-). Most keys read by libxl are
expected to be base 10, with some allowed to be in different base (as
they're a few that uses strtoul(,,0);)

So don't try to change the base of existing keys ;-).

For those virtio one in particular, it's probably ok. libxl doesn't
mind, and hopefully the consumer of those don't mind either.

-- 
Anthony PERARD

Re: [PATCH] libxl: virtio: Fix build error for 32-bit platforms

[Andrew Cooper]

On 15/12/2022 5:33 pm, Anthony PERARD wrote:
> On Thu, Dec 15, 2022 at 04:58:02PM +0000, Andrew Cooper wrote:
>> A consumer of xenstore has to cope with all bases anyway.  Anything that
>> doesn't is broken.
> So libxl is broken, that good to know :-).

Yes.  Really, yes.

This is sufficiently basic stuff for text based APIs/ABIs that it ought
to go without saying.

>  Most keys read by libxl are
> expected to be base 10, with some allowed to be in different base (as
> they're a few that uses strtoul(,,0);)

This is at least recoverable by switching to ,,0 uniformly.

That said, xenstore-paths.pandoc's attempt to describe the grammar
appears to be ambiguous.

That's the first place to fix.  I'll put a ticket on gitlab because I
don't have enough cycles to do this now.

~Andrew

Re: [PATCH] libxl: virtio: Fix build error for 32-bit platforms

[Andrew Cooper]

On 15/12/2022 1:31 pm, Viresh Kumar wrote:
> The field 'base' in 'struct libxl_device_virtio' is defined as uint64,
> while we are printing it with '%lu', which is 32bit only 32-bit
> platforms. And so generates a error like:
>
>   libxl_internal.h:4388:51: error: format '%lu' expects argument of type 'long
>   unsigned int', but argument 3 has type 'uint64_t' {aka 'long long unsigned
>   int'} [-Werror=format=]
>
> Fix the same by using PRIx64 instead.
>
> Now that the base name is available in hexadecimal format, prefix it
> with '0x' as well, which strtoul() also depends upon since base passed
> is 0.
>
> Fixes: 43ba5202e2ee ("libxl: add support for generic virtio device")
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

In order to unblock OSSTest, I've committed this with an adjusted commit
message, with the agreement on Anthony on IRC.

~Andrew

Re: [PATCH] libxl: virtio: Fix build error for 32-bit platforms

[Viresh Kumar]

On 15-12-22, 17:33, Anthony PERARD wrote:
> For those virtio one in particular, it's probably ok. libxl doesn't
> mind, and hopefully the consumer of those don't mind either.

FWIW, the consumer in this case, Rust based xen-vhost-frontent [1]
implementation, did break and I still need to fix it to read hex
properly :)

-- 
viresh

[1] https://github.com/vireshk/xen-vhost-frontend