Re: [libvirt] [PATCHv4 13/15] build: switch --with-qemu default from yes to check

2018-07-23 Thread Andrea Bolognani
On Mon, 2018-07-23 at 10:27 +0100, Daniel P. Berrangé wrote:
> On Mon, Jul 23, 2018 at 09:38:47AM +0200, Andrea Bolognani wrote:
> > Partially answering myself (morning coffee hasn't quite kicked in
> > just yet), the MinGW .spec doesn't list yajl as a BuildRequires,
> > which means that either we're not hitting any of the code paths you
> > mention on that platform, or we've been producing completely useless
> > Windows builds for a while now :)
> 
> The remote driver works fine without JSON being required.

That being the case, not having jansson as mandatory sounds like the
proper thing to do, at least from the theoretical point of view: even
if you build without it, you can still get *some* functionality out
of libvirt, so it's kinda hard to argue for it being a strict
requirement the same way something like libxml2 is.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCHv4 13/15] build: switch --with-qemu default from yes to check

2018-07-23 Thread Daniel P . Berrangé
On Mon, Jul 23, 2018 at 09:38:47AM +0200, Andrea Bolognani wrote:
> On Mon, 2018-07-23 at 09:27 +0200, Andrea Bolognani wrote:
> > On Fri, 2018-07-20 at 11:33 +0100, Daniel P. Berrangé wrote:
> > > On Fri, Jul 20, 2018 at 12:24:50PM +0200, Ján Tomko wrote:
> > > > I do not oppose reverting this bit and failing by default if we don't
> > > > have a JSON library (as Andrea mentioned, more drivers might possibly
> > > > require a working JSON implementation).
> > > 
> > > We use virjson.h throughout src/util and src/rpc so that code is
> > > not even driver specific, however, there are not currently mingw
> > > pacakges for jansson in Fedora at least. Maybe it will work, but
> > > we would need someone todo the work to add that to Fedora before
> > > we can consider making this mandatory.
> > 
> > So does the MinGW build work at all without jansson, considering its
> > functionality is fairly reduced in scope compared to the Unix builds?
> 
> Partially answering myself (morning coffee hasn't quite kicked in
> just yet), the MinGW .spec doesn't list yajl as a BuildRequires,
> which means that either we're not hitting any of the code paths you
> mention on that platform, or we've been producing completely useless
> Windows builds for a while now :)

The remote driver works fine without JSON being required.

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] [PATCHv4 13/15] build: switch --with-qemu default from yes to check

2018-07-23 Thread Andrea Bolognani
On Mon, 2018-07-23 at 09:27 +0200, Andrea Bolognani wrote:
> On Fri, 2018-07-20 at 11:33 +0100, Daniel P. Berrangé wrote:
> > On Fri, Jul 20, 2018 at 12:24:50PM +0200, Ján Tomko wrote:
> > > I do not oppose reverting this bit and failing by default if we don't
> > > have a JSON library (as Andrea mentioned, more drivers might possibly
> > > require a working JSON implementation).
> > 
> > We use virjson.h throughout src/util and src/rpc so that code is
> > not even driver specific, however, there are not currently mingw
> > pacakges for jansson in Fedora at least. Maybe it will work, but
> > we would need someone todo the work to add that to Fedora before
> > we can consider making this mandatory.
> 
> So does the MinGW build work at all without jansson, considering its
> functionality is fairly reduced in scope compared to the Unix builds?

Partially answering myself (morning coffee hasn't quite kicked in
just yet), the MinGW .spec doesn't list yajl as a BuildRequires,
which means that either we're not hitting any of the code paths you
mention on that platform, or we've been producing completely useless
Windows builds for a while now :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCHv4 13/15] build: switch --with-qemu default from yes to check

2018-07-23 Thread Andrea Bolognani
On Fri, 2018-07-20 at 11:33 +0100, Daniel P. Berrangé wrote:
> On Fri, Jul 20, 2018 at 12:24:50PM +0200, Ján Tomko wrote:
> > I do not oppose reverting this bit and failing by default if we don't
> > have a JSON library (as Andrea mentioned, more drivers might possibly
> > require a working JSON implementation).
> 
> We use virjson.h throughout src/util and src/rpc so that code is
> not even driver specific, however, there are not currently mingw
> pacakges for jansson in Fedora at least. Maybe it will work, but
> we would need someone todo the work to add that to Fedora before
> we can consider making this mandatory.

So does the MinGW build work at all without jansson, considering its
functionality is fairly reduced in scope compared to the Unix builds?

Either way, at least Arch seems to have a MinGW package for it:

  https://aur.archlinux.org/packages/mingw-w64-jansson/

so it's probably just a matter of sitting down, coming up with a
suitable .spec and getting it into Fedora.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCHv4 13/15] build: switch --with-qemu default from yes to check

2018-07-20 Thread Daniel P . Berrangé
On Fri, Jul 20, 2018 at 12:24:50PM +0200, Ján Tomko wrote:
> On Thu, Jul 19, 2018 at 07:38:15PM -0400, John Ferlan wrote:
> > 
> > 
> > On 07/18/2018 10:44 AM, Ján Tomko wrote:
> > > Unless explicitly requested, enable the QEMU driver
> > > only if the Jansson library is present.
> > > 
> > > Signed-off-by: Ján Tomko 
> > > ---
> > >  m4/virt-driver-qemu.m4 | 6 +-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > 
> > 
> > Perhaps it's obvious for someone else, but I think there some sort of
> > dependency missing.
> > Starting with this patch I found that my with-qemu
> > "went away".
> > 
> > I have:
> > 
> > jansson.x86_64   2.11-1.fc28
> > @fedora
> > 
> > 
> > But not:
> > 
> > jansson-devel x86_64 2.11-1.fc28  fedora
> >  15 k
> > 
> > If I explicitly add --with-jansson onto the command line, then I get:
> > 
> > checking for JANSSON... no
> > configure: error: You must install the jansson >= 2.5 pkg-config module
> > to compile libvirt
> > error: configure failed
> > 
> > If I then install jansson-devel, the build succeeds.
> > 
> > Honestly I think we need to be much more in your face in this instance -
> > something isn't quite right and it eventually leads to some really
> > strange results because nothing in/for qemu is built, but you are left
> > with old build bits in your tree.  Eventually something fails.
> > 
> 
> There was not that much discussion about it:
> https://www.redhat.com/archives/libvir-list/2018-May/msg01321.html
> 
> I do not oppose reverting this bit and failing by default if we don't
> have a JSON library (as Andrea mentioned, more drivers might possibly
> require a working JSON implementation).

We use virjson.h throughout src/util and src/rpc so that code is
not even driver specific, however, there are not currently mingw
pacakges for jansson in Fedora at least. Maybe it will work, but
we would need someone todo the work to add that to Fedora before
we can consider making this mandatory.

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] [PATCHv4 13/15] build: switch --with-qemu default from yes to check

2018-07-20 Thread Ján Tomko

On Thu, Jul 19, 2018 at 07:38:15PM -0400, John Ferlan wrote:



On 07/18/2018 10:44 AM, Ján Tomko wrote:

Unless explicitly requested, enable the QEMU driver
only if the Jansson library is present.

Signed-off-by: Ján Tomko 
---
 m4/virt-driver-qemu.m4 | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)



Perhaps it's obvious for someone else, but I think there some sort of
dependency missing.
Starting with this patch I found that my with-qemu
"went away".

I have:

jansson.x86_64   2.11-1.fc28
@fedora


But not:

jansson-devel x86_64 2.11-1.fc28  fedora
 15 k

If I explicitly add --with-jansson onto the command line, then I get:

checking for JANSSON... no
configure: error: You must install the jansson >= 2.5 pkg-config module
to compile libvirt
error: configure failed

If I then install jansson-devel, the build succeeds.

Honestly I think we need to be much more in your face in this instance -
something isn't quite right and it eventually leads to some really
strange results because nothing in/for qemu is built, but you are left
with old build bits in your tree.  Eventually something fails.



There was not that much discussion about it:
https://www.redhat.com/archives/libvir-list/2018-May/msg01321.html

I do not oppose reverting this bit and failing by default if we don't
have a JSON library (as Andrea mentioned, more drivers might possibly
require a working JSON implementation).

However after dropping old QEMU support, building QEMU driver without
JSON makes no sense. (There was some logic in virt-yajl.m4 that tried
to figure out whether lack of yajl should be fatal based on the version
of the QEMU executable present on the system).


This whole build/config system is a generally a mystery to me, so I have
zero suggestions.  Ironically if I had to build from source, I'd know
from libvirt.spec that jansson-devel was required, although there's no

= version check so that probably should be fixed.


Right, the specfile still has:
%define min_rhel 6



Suffice to say digging into the config.log trying to figure why one's
QEMU is disabled is not an enjoyable or easy experience.

Yeah, yeah, we're developers we're supposed to be smart, we get what we
get... I'll bet some qemu devel will hit this some day and wonder how to
actually build libvirt with qemu because it's not obvious and it "used
to be" a default=yes.



I expected that people surprised by the lack of QEMU driver would pass
--with-qemu, which should have the proper error.

Jano


John

Off to go drown the rest of my frustration ;-)



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

Re: [libvirt] [PATCHv4 13/15] build: switch --with-qemu default from yes to check

2018-07-19 Thread John Ferlan


On 07/18/2018 10:44 AM, Ján Tomko wrote:
> Unless explicitly requested, enable the QEMU driver
> only if the Jansson library is present.
> 
> Signed-off-by: Ján Tomko 
> ---
>  m4/virt-driver-qemu.m4 | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 

Perhaps it's obvious for someone else, but I think there some sort of
dependency missing. Starting with this patch I found that my with-qemu
"went away".

I have:

jansson.x86_64   2.11-1.fc28
@fedora


But not:

 jansson-devel x86_64 2.11-1.fc28  fedora
  15 k

If I explicitly add --with-jansson onto the command line, then I get:

checking for JANSSON... no
configure: error: You must install the jansson >= 2.5 pkg-config module
to compile libvirt
error: configure failed

If I then install jansson-devel, the build succeeds.

Honestly I think we need to be much more in your face in this instance -
something isn't quite right and it eventually leads to some really
strange results because nothing in/for qemu is built, but you are left
with old build bits in your tree.  Eventually something fails.

This whole build/config system is a generally a mystery to me, so I have
zero suggestions.  Ironically if I had to build from source, I'd know
from libvirt.spec that jansson-devel was required, although there's no
>= version check so that probably should be fixed.

Suffice to say digging into the config.log trying to figure why one's
QEMU is disabled is not an enjoyable or easy experience.

Yeah, yeah, we're developers we're supposed to be smart, we get what we
get... I'll bet some qemu devel will hit this some day and wonder how to
actually build libvirt with qemu because it's not obvious and it "used
to be" a default=yes.

John

Off to go drown the rest of my frustration ;-)

> diff --git a/m4/virt-driver-qemu.m4 b/m4/virt-driver-qemu.m4
> index 80e1d3ad46..ddb2834705 100644
> --- a/m4/virt-driver-qemu.m4
> +++ b/m4/virt-driver-qemu.m4
> @@ -18,7 +18,7 @@ dnl .
>  dnl
>  
>  AC_DEFUN([LIBVIRT_DRIVER_ARG_QEMU], [
> -  LIBVIRT_ARG_WITH_FEATURE([QEMU], [QEMU/KVM], [yes])
> +  LIBVIRT_ARG_WITH_FEATURE([QEMU], [QEMU/KVM], [check])
>LIBVIRT_ARG_WITH([QEMU_USER], [username to run QEMU system instance as],
> ['platform dependent'])
>LIBVIRT_ARG_WITH([QEMU_GROUP], [groupname to run QEMU system instance as],
> @@ -26,6 +26,10 @@ AC_DEFUN([LIBVIRT_DRIVER_ARG_QEMU], [
>  ])
>  
>  AC_DEFUN([LIBVIRT_DRIVER_CHECK_QEMU], [
> +  AC_REQUIRE([LIBVIRT_CHECK_JANSSON])
> +  if test "$with_qemu" = "check"; then
> +with_qemu=$with_jansson
> +  fi
>if test "$with_qemu" = "yes" ; then
>  AC_DEFINE_UNQUOTED([WITH_QEMU], 1, [whether QEMU driver is enabled])
>fi
> 

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

[libvirt] [PATCHv4 13/15] build: switch --with-qemu default from yes to check

2018-07-18 Thread Ján Tomko
Unless explicitly requested, enable the QEMU driver
only if the Jansson library is present.

Signed-off-by: Ján Tomko 
---
 m4/virt-driver-qemu.m4 | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/m4/virt-driver-qemu.m4 b/m4/virt-driver-qemu.m4
index 80e1d3ad46..ddb2834705 100644
--- a/m4/virt-driver-qemu.m4
+++ b/m4/virt-driver-qemu.m4
@@ -18,7 +18,7 @@ dnl .
 dnl
 
 AC_DEFUN([LIBVIRT_DRIVER_ARG_QEMU], [
-  LIBVIRT_ARG_WITH_FEATURE([QEMU], [QEMU/KVM], [yes])
+  LIBVIRT_ARG_WITH_FEATURE([QEMU], [QEMU/KVM], [check])
   LIBVIRT_ARG_WITH([QEMU_USER], [username to run QEMU system instance as],
['platform dependent'])
   LIBVIRT_ARG_WITH([QEMU_GROUP], [groupname to run QEMU system instance as],
@@ -26,6 +26,10 @@ AC_DEFUN([LIBVIRT_DRIVER_ARG_QEMU], [
 ])
 
 AC_DEFUN([LIBVIRT_DRIVER_CHECK_QEMU], [
+  AC_REQUIRE([LIBVIRT_CHECK_JANSSON])
+  if test "$with_qemu" = "check"; then
+with_qemu=$with_jansson
+  fi
   if test "$with_qemu" = "yes" ; then
 AC_DEFINE_UNQUOTED([WITH_QEMU], 1, [whether QEMU driver is enabled])
   fi
-- 
2.16.1

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