diff mbox

[RFC,v1,1/2] gluster: Use pkg-config to configure GlusterFS block driver

Message ID 20130712065854.GJ32532@in.ibm.com
State New
Headers show

Commit Message

Bharata B Rao July 12, 2013, 6:58 a.m. UTC
gluster: Use pkg-config to configure GlusterFS block driver

Use pkg-config to determine the version and library dependency
for GlusterFS block driver.

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
 configure |   20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

Comments

Stefan Hajnoczi July 16, 2013, 6:02 a.m. UTC | #1
On Fri, Jul 12, 2013 at 12:28:54PM +0530, Bharata B Rao wrote:
> gluster: Use pkg-config to configure GlusterFS block driver

The commit message is duplicated in the commit description.

> Use pkg-config to determine the version and library dependency
> for GlusterFS block driver.
> 
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> ---
>  configure |   20 +++++++-------------
>  1 file changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/configure b/configure
> index cb0f870..76adcb1 100755
> --- a/configure
> +++ b/configure
> @@ -2566,23 +2566,17 @@ fi
>  ##########################################
>  # glusterfs probe
>  if test "$glusterfs" != "no" ; then
> -  cat > $TMPC <<EOF
> -#include <glusterfs/api/glfs.h>
> -int main(void) {
> -    (void) glfs_new("volume");
> -    return 0;
> -}
> -EOF
> -  glusterfs_libs="-lgfapi -lgfrpc -lgfxdr"
> -  if compile_prog "" "$glusterfs_libs" ; then
> -    glusterfs=yes
> -    libs_tools="$glusterfs_libs $libs_tools"
> -    libs_softmmu="$glusterfs_libs $libs_softmmu"
> +  if $pkg_config --atleast-version=3 glusterfs-api >/dev/null 2>&1; then

gfapi was added in Gluster 3.4.  Is --atleast-version=3 sufficient?
Anand Avati July 16, 2013, 6:04 a.m. UTC | #2
On 7/15/13 11:02 PM, Stefan Hajnoczi wrote:
> On Fri, Jul 12, 2013 at 12:28:54PM +0530, Bharata B Rao wrote:
>> gluster: Use pkg-config to configure GlusterFS block driver
>
> The commit message is duplicated in the commit description.
>
>> Use pkg-config to determine the version and library dependency
>> for GlusterFS block driver.
>>
>> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
>> ---
>>   configure |   20 +++++++-------------
>>   1 file changed, 7 insertions(+), 13 deletions(-)
>>
>> diff --git a/configure b/configure
>> index cb0f870..76adcb1 100755
>> --- a/configure
>> +++ b/configure
>> @@ -2566,23 +2566,17 @@ fi
>>   ##########################################
>>   # glusterfs probe
>>   if test "$glusterfs" != "no" ; then
>> -  cat > $TMPC <<EOF
>> -#include <glusterfs/api/glfs.h>
>> -int main(void) {
>> -    (void) glfs_new("volume");
>> -    return 0;
>> -}
>> -EOF
>> -  glusterfs_libs="-lgfapi -lgfrpc -lgfxdr"
>> -  if compile_prog "" "$glusterfs_libs" ; then
>> -    glusterfs=yes
>> -    libs_tools="$glusterfs_libs $libs_tools"
>> -    libs_softmmu="$glusterfs_libs $libs_softmmu"
>> +  if $pkg_config --atleast-version=3 glusterfs-api >/dev/null 2>&1; then
>
> gfapi was added in Gluster 3.4.  Is --atleast-version=3 sufficient?
>

--atleast-version=3 is the api version (not release version). 3 is 
sufficient for basic qemu integration. 5 and above has fallocate/discard.

Avati
Kevin Wolf July 16, 2013, 8:25 a.m. UTC | #3
Am 12.07.2013 um 08:58 hat Bharata B Rao geschrieben:
> gluster: Use pkg-config to configure GlusterFS block driver
> 
> Use pkg-config to determine the version and library dependency
> for GlusterFS block driver.
> 
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> ---
>  configure |   20 +++++++-------------
>  1 file changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/configure b/configure
> index cb0f870..76adcb1 100755
> --- a/configure
> +++ b/configure
> @@ -2566,23 +2566,17 @@ fi
>  ##########################################
>  # glusterfs probe
>  if test "$glusterfs" != "no" ; then
> -  cat > $TMPC <<EOF
> -#include <glusterfs/api/glfs.h>
> -int main(void) {
> -    (void) glfs_new("volume");
> -    return 0;
> -}
> -EOF
> -  glusterfs_libs="-lgfapi -lgfrpc -lgfxdr"
> -  if compile_prog "" "$glusterfs_libs" ; then
> -    glusterfs=yes
> -    libs_tools="$glusterfs_libs $libs_tools"
> -    libs_softmmu="$glusterfs_libs $libs_softmmu"
> +  if $pkg_config --atleast-version=3 glusterfs-api >/dev/null 2>&1; then
> +    glusterfs="yes"
> +    glusterfs_cflags=`$pkg_config --cflags glusterfs-api 2>/dev/null`
> +    glusterfs_libs=`$pkg_config --libs glusterfs-api 2>/dev/null`
> +    CFLAGS="$CFLAGS $glusterfs_cflags"
> +    LIBS="$LIBS $glusterfs_libs"

Why do you switch from libs_tools/libs_softmmu to LIBS? If I understand
it right, this means that you now link the library to linux-user targets
as well, but I can't see how they need it.

Kevin
Bharata B Rao July 16, 2013, 11:34 a.m. UTC | #4
On Tue, Jul 16, 2013 at 10:25:29AM +0200, Kevin Wolf wrote:
> > -  glusterfs_libs="-lgfapi -lgfrpc -lgfxdr"
> > -  if compile_prog "" "$glusterfs_libs" ; then
> > -    glusterfs=yes
> > -    libs_tools="$glusterfs_libs $libs_tools"
> > -    libs_softmmu="$glusterfs_libs $libs_softmmu"
> > +  if $pkg_config --atleast-version=3 glusterfs-api >/dev/null 2>&1; then
> > +    glusterfs="yes"
> > +    glusterfs_cflags=`$pkg_config --cflags glusterfs-api 2>/dev/null`
> > +    glusterfs_libs=`$pkg_config --libs glusterfs-api 2>/dev/null`
> > +    CFLAGS="$CFLAGS $glusterfs_cflags"
> > +    LIBS="$LIBS $glusterfs_libs"
> 
> Why do you switch from libs_tools/libs_softmmu to LIBS? If I understand
> it right, this means that you now link the library to linux-user targets
> as well, but I can't see how they need it.

Copied from some other backend and didn't realize that I ended up considering
linux-user target also. Will change this and send v2.

Regards,
Bharata.
Daniel P. Berrangé July 23, 2013, 11:57 a.m. UTC | #5
On Fri, Jul 12, 2013 at 12:28:54PM +0530, Bharata B Rao wrote:
> gluster: Use pkg-config to configure GlusterFS block driver
> 
> Use pkg-config to determine the version and library dependency
> for GlusterFS block driver.
> 
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> ---
>  configure |   20 +++++++-------------
>  1 file changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/configure b/configure
> index cb0f870..76adcb1 100755
> --- a/configure
> +++ b/configure
> @@ -2566,23 +2566,17 @@ fi
>  ##########################################
>  # glusterfs probe
>  if test "$glusterfs" != "no" ; then
> -  cat > $TMPC <<EOF
> -#include <glusterfs/api/glfs.h>
> -int main(void) {
> -    (void) glfs_new("volume");
> -    return 0;
> -}
> -EOF
> -  glusterfs_libs="-lgfapi -lgfrpc -lgfxdr"
> -  if compile_prog "" "$glusterfs_libs" ; then
> -    glusterfs=yes
> -    libs_tools="$glusterfs_libs $libs_tools"
> -    libs_softmmu="$glusterfs_libs $libs_softmmu"
> +  if $pkg_config --atleast-version=3 glusterfs-api >/dev/null 2>&1; then
> +    glusterfs="yes"
> +    glusterfs_cflags=`$pkg_config --cflags glusterfs-api 2>/dev/null`
> +    glusterfs_libs=`$pkg_config --libs glusterfs-api 2>/dev/null`
> +    CFLAGS="$CFLAGS $glusterfs_cflags"
> +    LIBS="$LIBS $glusterfs_libs"

The glusterfs v 3.4 RPMs in Fedora do not include any pkg-config files.
So with this change now in GIT, QEMU no longer detects support for
glusterfs even though it is present.

Has the min required glusterfs been increased to a new 3.5 version
which does include pkg-config support ?  If not, then I think this
patch needs to be reverted, so that it does a non-pkg-config based
check for glusterfs.

Regards,
Daniel
Anand Avati July 23, 2013, 12:02 p.m. UTC | #6
On 7/23/13 4:57 AM, Daniel P. Berrange wrote:
> On Fri, Jul 12, 2013 at 12:28:54PM +0530, Bharata B Rao wrote:
>> gluster: Use pkg-config to configure GlusterFS block driver
>>
>> Use pkg-config to determine the version and library dependency
>> for GlusterFS block driver.
>>
>> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
>> ---
>>   configure |   20 +++++++-------------
>>   1 file changed, 7 insertions(+), 13 deletions(-)
>>
>> diff --git a/configure b/configure
>> index cb0f870..76adcb1 100755
>> --- a/configure
>> +++ b/configure
>> @@ -2566,23 +2566,17 @@ fi
>>   ##########################################
>>   # glusterfs probe
>>   if test "$glusterfs" != "no" ; then
>> -  cat > $TMPC <<EOF
>> -#include <glusterfs/api/glfs.h>
>> -int main(void) {
>> -    (void) glfs_new("volume");
>> -    return 0;
>> -}
>> -EOF
>> -  glusterfs_libs="-lgfapi -lgfrpc -lgfxdr"
>> -  if compile_prog "" "$glusterfs_libs" ; then
>> -    glusterfs=yes
>> -    libs_tools="$glusterfs_libs $libs_tools"
>> -    libs_softmmu="$glusterfs_libs $libs_softmmu"
>> +  if $pkg_config --atleast-version=3 glusterfs-api >/dev/null 2>&1; then
>> +    glusterfs="yes"
>> +    glusterfs_cflags=`$pkg_config --cflags glusterfs-api 2>/dev/null`
>> +    glusterfs_libs=`$pkg_config --libs glusterfs-api 2>/dev/null`
>> +    CFLAGS="$CFLAGS $glusterfs_cflags"
>> +    LIBS="$LIBS $glusterfs_libs"
>
> The glusterfs v 3.4 RPMs in Fedora do not include any pkg-config files.
> So with this change now in GIT, QEMU no longer detects support for
> glusterfs even though it is present.
>
> Has the min required glusterfs been increased to a new 3.5 version
> which does include pkg-config support ?  If not, then I think this
> patch needs to be reverted, so that it does a non-pkg-config based
> check for glusterfs.
>
> Regards,
> Daniel
>

Copying Kaleb.

We should just include the pkg-config file in the Fedora RPM for 
glusterfs if it already isn't.

Avati
Daniel P. Berrangé July 23, 2013, 12:03 p.m. UTC | #7
On Tue, Jul 23, 2013 at 05:02:20AM -0700, Anand Avati wrote:
> On 7/23/13 4:57 AM, Daniel P. Berrange wrote:
> >On Fri, Jul 12, 2013 at 12:28:54PM +0530, Bharata B Rao wrote:
> >>gluster: Use pkg-config to configure GlusterFS block driver
> >>
> >>Use pkg-config to determine the version and library dependency
> >>for GlusterFS block driver.
> >>
> >>Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> >>---
> >>  configure |   20 +++++++-------------
> >>  1 file changed, 7 insertions(+), 13 deletions(-)
> >>
> >>diff --git a/configure b/configure
> >>index cb0f870..76adcb1 100755
> >>--- a/configure
> >>+++ b/configure
> >>@@ -2566,23 +2566,17 @@ fi
> >>  ##########################################
> >>  # glusterfs probe
> >>  if test "$glusterfs" != "no" ; then
> >>-  cat > $TMPC <<EOF
> >>-#include <glusterfs/api/glfs.h>
> >>-int main(void) {
> >>-    (void) glfs_new("volume");
> >>-    return 0;
> >>-}
> >>-EOF
> >>-  glusterfs_libs="-lgfapi -lgfrpc -lgfxdr"
> >>-  if compile_prog "" "$glusterfs_libs" ; then
> >>-    glusterfs=yes
> >>-    libs_tools="$glusterfs_libs $libs_tools"
> >>-    libs_softmmu="$glusterfs_libs $libs_softmmu"
> >>+  if $pkg_config --atleast-version=3 glusterfs-api >/dev/null 2>&1; then
> >>+    glusterfs="yes"
> >>+    glusterfs_cflags=`$pkg_config --cflags glusterfs-api 2>/dev/null`
> >>+    glusterfs_libs=`$pkg_config --libs glusterfs-api 2>/dev/null`
> >>+    CFLAGS="$CFLAGS $glusterfs_cflags"
> >>+    LIBS="$LIBS $glusterfs_libs"
> >
> >The glusterfs v 3.4 RPMs in Fedora do not include any pkg-config files.
> >So with this change now in GIT, QEMU no longer detects support for
> >glusterfs even though it is present.
> >
> >Has the min required glusterfs been increased to a new 3.5 version
> >which does include pkg-config support ?  If not, then I think this
> >patch needs to be reverted, so that it does a non-pkg-config based
> >check for glusterfs.
> >
> >Regards,
> >Daniel
> >
> 
> Copying Kaleb.
> 
> We should just include the pkg-config file in the Fedora RPM for
> glusterfs if it already isn't.

That doesn't help anyone trying to build QEMU with gluster support
on all the existing released distros which lack the pkg-config files.

If you really want a pkg-config file check for glusterfs in QEMU,
then it must at least fallback to probing the non-pkg-config way
to support existing deployed distros.

Daniel
Kaleb KEITHLEY July 23, 2013, 12:07 p.m. UTC | #8
On 07/23/2013 05:32 PM, Anand Avati wrote:
> On 7/23/13 4:57 AM, Daniel P. Berrange wrote:
>> On Fri, Jul 12, 2013 at 12:28:54PM +0530, Bharata B Rao wrote:
>>> gluster: Use pkg-config to configure GlusterFS block driver
>>>
>>> Use pkg-config to determine the version and library dependency
>>> for GlusterFS block driver.
>>>
>>> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
>>> ---
>>>   configure |   20 +++++++-------------
>>>   1 file changed, 7 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/configure b/configure
>>> index cb0f870..76adcb1 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -2566,23 +2566,17 @@ fi
>>>   ##########################################
>>>   # glusterfs probe
>>>   if test "$glusterfs" != "no" ; then
>>> -  cat > $TMPC <<EOF
>>> -#include <glusterfs/api/glfs.h>
>>> -int main(void) {
>>> -    (void) glfs_new("volume");
>>> -    return 0;
>>> -}
>>> -EOF
>>> -  glusterfs_libs="-lgfapi -lgfrpc -lgfxdr"
>>> -  if compile_prog "" "$glusterfs_libs" ; then
>>> -    glusterfs=yes
>>> -    libs_tools="$glusterfs_libs $libs_tools"
>>> -    libs_softmmu="$glusterfs_libs $libs_softmmu"
>>> +  if $pkg_config --atleast-version=3 glusterfs-api >/dev/null 2>&1;
>>> then
>>> +    glusterfs="yes"
>>> +    glusterfs_cflags=`$pkg_config --cflags glusterfs-api 2>/dev/null`
>>> +    glusterfs_libs=`$pkg_config --libs glusterfs-api 2>/dev/null`
>>> +    CFLAGS="$CFLAGS $glusterfs_cflags"
>>> +    LIBS="$LIBS $glusterfs_libs"
>>
>> The glusterfs v 3.4 RPMs in Fedora do not include any pkg-config files.
>> So with this change now in GIT, QEMU no longer detects support for
>> glusterfs even though it is present.
>>
>> Has the min required glusterfs been increased to a new 3.5 version
>> which does include pkg-config support ?  If not, then I think this
>> patch needs to be reverted, so that it does a non-pkg-config based
>> check for glusterfs.
>>
>> Regards,
>> Daniel
>>
>
> Copying Kaleb.
>
> We should just include the pkg-config file in the Fedora RPM for
> glusterfs if it already isn't.

It's in the glusterfs-api-devel rpm:

% rpm -ql glusterfs-api-devel
/usr/include/glusterfs/api/glfs.h
/usr/lib64/libgfapi.so
/usr/lib64/pkgconfig/glusterfs-api.pc

--

Kaleb
Daniel P. Berrangé July 23, 2013, 12:14 p.m. UTC | #9
On Tue, Jul 23, 2013 at 05:37:54PM +0530, Kaleb KEITHLEY wrote:
> On 07/23/2013 05:32 PM, Anand Avati wrote:
> >On 7/23/13 4:57 AM, Daniel P. Berrange wrote:
> >>On Fri, Jul 12, 2013 at 12:28:54PM +0530, Bharata B Rao wrote:
> >>>gluster: Use pkg-config to configure GlusterFS block driver
> >>>
> >>>Use pkg-config to determine the version and library dependency
> >>>for GlusterFS block driver.
> >>>
> >>>Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> >>>---
> >>>  configure |   20 +++++++-------------
> >>>  1 file changed, 7 insertions(+), 13 deletions(-)
> >>>
> >>>diff --git a/configure b/configure
> >>>index cb0f870..76adcb1 100755
> >>>--- a/configure
> >>>+++ b/configure
> >>>@@ -2566,23 +2566,17 @@ fi
> >>>  ##########################################
> >>>  # glusterfs probe
> >>>  if test "$glusterfs" != "no" ; then
> >>>-  cat > $TMPC <<EOF
> >>>-#include <glusterfs/api/glfs.h>
> >>>-int main(void) {
> >>>-    (void) glfs_new("volume");
> >>>-    return 0;
> >>>-}
> >>>-EOF
> >>>-  glusterfs_libs="-lgfapi -lgfrpc -lgfxdr"
> >>>-  if compile_prog "" "$glusterfs_libs" ; then
> >>>-    glusterfs=yes
> >>>-    libs_tools="$glusterfs_libs $libs_tools"
> >>>-    libs_softmmu="$glusterfs_libs $libs_softmmu"
> >>>+  if $pkg_config --atleast-version=3 glusterfs-api >/dev/null 2>&1;
> >>>then
> >>>+    glusterfs="yes"
> >>>+    glusterfs_cflags=`$pkg_config --cflags glusterfs-api 2>/dev/null`
> >>>+    glusterfs_libs=`$pkg_config --libs glusterfs-api 2>/dev/null`
> >>>+    CFLAGS="$CFLAGS $glusterfs_cflags"
> >>>+    LIBS="$LIBS $glusterfs_libs"
> >>
> >>The glusterfs v 3.4 RPMs in Fedora do not include any pkg-config files.
> >>So with this change now in GIT, QEMU no longer detects support for
> >>glusterfs even though it is present.
> >>
> >>Has the min required glusterfs been increased to a new 3.5 version
> >>which does include pkg-config support ?  If not, then I think this
> >>patch needs to be reverted, so that it does a non-pkg-config based
> >>check for glusterfs.
> >>
> >>Regards,
> >>Daniel
> >>
> >
> >Copying Kaleb.
> >
> >We should just include the pkg-config file in the Fedora RPM for
> >glusterfs if it already isn't.
> 
> It's in the glusterfs-api-devel rpm:
> 
> % rpm -ql glusterfs-api-devel
> /usr/include/glusterfs/api/glfs.h
> /usr/lib64/libgfapi.so
> /usr/lib64/pkgconfig/glusterfs-api.pc

Oooh, not the main glusterfs-devel RPM. Ok, ignore my earlier message

Daniel
diff mbox

Patch

diff --git a/configure b/configure
index cb0f870..76adcb1 100755
--- a/configure
+++ b/configure
@@ -2566,23 +2566,17 @@  fi
 ##########################################
 # glusterfs probe
 if test "$glusterfs" != "no" ; then
-  cat > $TMPC <<EOF
-#include <glusterfs/api/glfs.h>
-int main(void) {
-    (void) glfs_new("volume");
-    return 0;
-}
-EOF
-  glusterfs_libs="-lgfapi -lgfrpc -lgfxdr"
-  if compile_prog "" "$glusterfs_libs" ; then
-    glusterfs=yes
-    libs_tools="$glusterfs_libs $libs_tools"
-    libs_softmmu="$glusterfs_libs $libs_softmmu"
+  if $pkg_config --atleast-version=3 glusterfs-api >/dev/null 2>&1; then
+    glusterfs="yes"
+    glusterfs_cflags=`$pkg_config --cflags glusterfs-api 2>/dev/null`
+    glusterfs_libs=`$pkg_config --libs glusterfs-api 2>/dev/null`
+    CFLAGS="$CFLAGS $glusterfs_cflags"
+    LIBS="$LIBS $glusterfs_libs"
   else
     if test "$glusterfs" = "yes" ; then
       feature_not_found "GlusterFS backend support"
     fi
-    glusterfs=no
+    glusterfs="no"
   fi
 fi