[libvirt] [PATCH] tests: skip qemublocktest if building without YAJL

Daniel P. Berrangé posted 1 patch 5 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180904110110.13591-1-berrange@redhat.com
Test syntax-check passed
tests/qemublocktest.c | 36 +++++++++++++++++++++++-------------
1 file changed, 23 insertions(+), 13 deletions(-)
[libvirt] [PATCH] tests: skip qemublocktest if building without YAJL
Posted by Daniel P. Berrangé 5 years, 6 months ago
If no JSON parser is available qemublocktest fails, so skip its execution.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---

Pushed as a build fix for platforms without JSON parser installed

 tests/qemublocktest.c | 36 +++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 0c335abc5b..8e8773e6a8 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -19,17 +19,19 @@
 #include <stdlib.h>
 
 #include "testutils.h"
-#include "testutilsqemu.h"
-#include "testutilsqemuschema.h"
-#include "virstoragefile.h"
-#include "virstring.h"
-#include "virlog.h"
-#include "qemu/qemu_block.h"
-#include "qemu/qemu_qapi.h"
 
-#include "qemu/qemu_command.h"
+#if WITH_YAJL
+# include "testutilsqemu.h"
+# include "testutilsqemuschema.h"
+# include "virstoragefile.h"
+# include "virstring.h"
+# include "virlog.h"
+# include "qemu/qemu_block.h"
+# include "qemu/qemu_qapi.h"
 
-#define VIR_FROM_THIS VIR_FROM_NONE
+# include "qemu/qemu_command.h"
+
+# define VIR_FROM_THIS VIR_FROM_NONE
 
 VIR_LOG_INIT("tests.storagetest");
 
@@ -355,7 +357,7 @@ mymain(void)
 
     virTestCounterReset("qemu storage source xml->json->xml ");
 
-#define TEST_JSON_FORMAT(tpe, xmlstr) \
+# define TEST_JSON_FORMAT(tpe, xmlstr) \
     do { \
         xmljsonxmldata.type = tpe; \
         xmljsonxmldata.xml = xmlstr; \
@@ -364,7 +366,7 @@ mymain(void)
             ret = -1; \
     } while (0)
 
-#define TEST_JSON_FORMAT_NET(xmlstr)\
+# define TEST_JSON_FORMAT_NET(xmlstr) \
     TEST_JSON_FORMAT(VIR_STORAGE_TYPE_NETWORK, xmlstr)
 
     TEST_JSON_FORMAT(VIR_STORAGE_TYPE_FILE, "<source file='/path/to/file'/>\n");
@@ -417,7 +419,7 @@ mymain(void)
                          "  <host name='example.com' port='9999'/>\n"
                          "</source>\n");
 
-#define TEST_DISK_TO_JSON_FULL(nme, fl) \
+# define TEST_DISK_TO_JSON_FULL(nme, fl) \
     do { \
         diskxmljsondata.name = nme; \
         diskxmljsondata.props = NULL; \
@@ -435,7 +437,7 @@ mymain(void)
         testQemuDiskXMLToPropsClear(&diskxmljsondata); \
     } while (0)
 
-#define TEST_DISK_TO_JSON(nme) TEST_DISK_TO_JSON_FULL(nme, false)
+# define TEST_DISK_TO_JSON(nme) TEST_DISK_TO_JSON_FULL(nme, false)
 
     if (!(diskxmljsondata.schema = testQEMUSchemaLoad())) {
         ret = -1;
@@ -500,4 +502,12 @@ mymain(void)
     return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }
 
+#else
+static int
+mymain(void)
+{
+    return EXIT_AM_SKIP;
+}
+#endif
+
 VIR_TEST_MAIN(mymain)
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: skip qemublocktest if building without YAJL
Posted by Andrea Bolognani 5 years, 6 months ago
On Tue, 2018-09-04 at 12:01 +0100, Daniel P. Berrangé wrote:
> If no JSON parser is available qemublocktest fails, so skip its execution.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> 
> Pushed as a build fix for platforms without JSON parser installed

Which platforms? All machines in our CI environments should have
yajl installed.

Either way, IIRC during the switch to Jansson we also made the
JSON parser mandatory when building the QEMU driver, a change
which I believe we might have reverted when switching back to
yajl: can we just have that back? Or are there some issues
preventing us from doing so?

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: skip qemublocktest if building without YAJL
Posted by Daniel P. Berrangé 5 years, 6 months ago
On Tue, Sep 04, 2018 at 02:00:21PM +0200, Andrea Bolognani wrote:
> On Tue, 2018-09-04 at 12:01 +0100, Daniel P. Berrangé wrote:
> > If no JSON parser is available qemublocktest fails, so skip its execution.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> > 
> > Pushed as a build fix for platforms without JSON parser installed
> 
> Which platforms? All machines in our CI environments should have
> yajl installed.

It wasn't CI - it was my own FreeBSD machine without yajl

> Either way, IIRC during the switch to Jansson we also made the
> JSON parser mandatory when building the QEMU driver, a change
> which I believe we might have reverted when switching back to
> yajl: can we just have that back? Or are there some issues
> preventing us from doing so?

All our other tests have WITH_YAJL checks in them, this one was the
exception.  Possibly they could all be changed with WITH_QEMU
instead but that's more than I want todo as a build fix.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: skip qemublocktest if building without YAJL
Posted by Andrea Bolognani 5 years, 6 months ago
On Tue, 2018-09-04 at 13:05 +0100, Daniel P. Berrangé wrote:
> > > Pushed as a build fix for platforms without JSON parser installed
> > 
> > Which platforms? All machines in our CI environments should have
> > yajl installed.
> 
> It wasn't CI - it was my own FreeBSD machine without yajl

I see someone hasn't been using lcitool :P

> > Either way, IIRC during the switch to Jansson we also made the
> > JSON parser mandatory when building the QEMU driver, a change
> > which I believe we might have reverted when switching back to
> > yajl: can we just have that back? Or are there some issues
> > preventing us from doing so?
> 
> All our other tests have WITH_YAJL checks in them, this one was the
> exception.  Possibly they could all be changed with WITH_QEMU
> instead but that's more than I want todo as a build fix.

Fair enough. I still think we want to re-introduce the behavior
described above.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: skip qemublocktest if building without YAJL
Posted by Daniel P. Berrangé 5 years, 6 months ago
On Tue, Sep 04, 2018 at 02:10:56PM +0200, Andrea Bolognani wrote:
> On Tue, 2018-09-04 at 13:05 +0100, Daniel P. Berrangé wrote:
> > > > Pushed as a build fix for platforms without JSON parser installed
> > > 
> > > Which platforms? All machines in our CI environments should have
> > > yajl installed.
> > 
> > It wasn't CI - it was my own FreeBSD machine without yajl
> 
> I see someone hasn't been using lcitool :P

Note the CI machines are just a representative set of build targets, they
are not providing exhaustive coverage. If we have a --without-yajl option,
then we should be able to sucessfully build without yajl, even if the OS
is capable of supporting yajl.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: skip qemublocktest if building without YAJL
Posted by Ján Tomko 5 years, 6 months ago
On Tue, Sep 04, 2018 at 02:00:21PM +0200, Andrea Bolognani wrote:
>On Tue, 2018-09-04 at 12:01 +0100, Daniel P. Berrangé wrote:
>> If no JSON parser is available qemublocktest fails, so skip its execution.
>>
>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>> ---
>>
>> Pushed as a build fix for platforms without JSON parser installed
>
>Which platforms? All machines in our CI environments should have
>yajl installed.
>
>Either way, IIRC during the switch to Jansson we also made the
>JSON parser mandatory when building the QEMU driver, a change
>which I believe we might have reverted when switching back to
>yajl: can we just have that back? Or are there some issues
>preventing us from doing so?

Yes, as a part of the Jansson revert I also reverted the build defaults:
commit 5a58b5ed6803e71e32e5d6f8c6e3b68874d085fb
    Revert "build: switch --with-qemu default from yes to check"
commit f204cf51035f51b979dec18ee526e418139fa874
    Revert "build: require Jansson if QEMU driver is enabled"

Because I unwisely dropped the WITH_YAJL->WITH_JSON rename that was
present in earlier series and they used 'WITH_JANSSON'.


At runtime, we require JSON for all the supported QEMUs, so the only
thing stopping us from re-adding the check is that we need to change
the QEMU driver default from 'yes' to 'check':
https://www.redhat.com/archives/libvir-list/2018-May/msg01060.html

(which, in combination with a change of the required library left some
developers confused when the QEMU driver quietly stopped building)

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