Re: [libvirt] [PATCH v2 1/3] tests: qemuxml2argv: make CAPS_LATEST arch generic

2018-07-24 Thread Peter Krempa
On Tue, Jul 24, 2018 at 13:31:04 +0200, Boris Fiuczynski wrote:
> From: Bjoern Walk 
> 
> Testing with the latest capabilities has been x86_64 centric. Let's
> remove the hardcoded architecture and give the user the ability to
> specify the desired architecture in the macro.
> 
> Signed-off-by: Bjoern Walk 
> Reviewed-by: Boris Fiuczynski 
> ---
>  tests/qemuxml2argvtest.c | 96 ++--
>  1 file changed, 54 insertions(+), 42 deletions(-)

[...]

> @@ -746,9 +758,9 @@ mymain(void)
>  # define DO_TEST_CAPS_VER(name, ver) \
>  DO_TEST_CAPS_ARCH_VER(name, "x86_64", ver)
>  
> -# define DO_TEST_CAPS_LATEST(name) \
> -DO_TEST_CAPS_INTERNAL(name, "x86_64-latest", NULL, 0, 0, "x86_64", \
> -  capslatest_x86_64, true)
> +# define DO_TEST_CAPS_LATEST(name, arch) \
> +DO_TEST_CAPS_INTERNAL(name, arch "-latest", NULL, 0, 0, arch, \
> +  virHashLookup(capslatest, arch), true)

Please add a DO_TEST_CAPS_LATEST_ARCH version rather than modifying all
of the calls. We have a vast majority of x86_64 tests so we should not
need to change them.



signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 1/3] tests: qemuxml2argv: make CAPS_LATEST arch generic

2018-07-24 Thread Boris Fiuczynski

On 07/24/2018 01:59 PM, Peter Krempa wrote:

On Tue, Jul 24, 2018 at 13:31:04 +0200, Boris Fiuczynski wrote:

From: Bjoern Walk 

Testing with the latest capabilities has been x86_64 centric. Let's
remove the hardcoded architecture and give the user the ability to
specify the desired architecture in the macro.

Signed-off-by: Bjoern Walk 
Reviewed-by: Boris Fiuczynski 
---
  tests/qemuxml2argvtest.c | 96 ++--
  1 file changed, 54 insertions(+), 42 deletions(-)


[...]


@@ -746,9 +758,9 @@ mymain(void)
  # define DO_TEST_CAPS_VER(name, ver) \
  DO_TEST_CAPS_ARCH_VER(name, "x86_64", ver)
  
-# define DO_TEST_CAPS_LATEST(name) \

-DO_TEST_CAPS_INTERNAL(name, "x86_64-latest", NULL, 0, 0, "x86_64", \
-  capslatest_x86_64, true)
+# define DO_TEST_CAPS_LATEST(name, arch) \
+DO_TEST_CAPS_INTERNAL(name, arch "-latest", NULL, 0, 0, arch, \
+  virHashLookup(capslatest, arch), true)


Please add a DO_TEST_CAPS_LATEST_ARCH version rather than modifying all
of the calls. We have a vast majority of x86_64 tests so we should not
need to change them.


I think that too many code changes (they are already available in this 
patch anyway) should not be the reason to hide the architecture x86_64 
in DO_TEST_CAPS_LATEST and to introduce a new macro 
DO_TEST_CAPS_LATEST_ARCH for all other architectures (including x86_64). 
Would you prefer architecture specific macros instead? e.g. rename 
DO_TEST_CAPS_LATEST to DO_TEST_CAPS_LATEST_X86_64 and create a new 
DO_TEST_CAPS_LATEST_S390.






--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list




--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list