diff mbox

block/nfs: use per-object vars and make it modular

Message ID 1399813620-26668-1-git-send-email-mjt@msgid.tls.msk.ru
State New
Headers show

Commit Message

Michael Tokarev May 11, 2014, 1:07 p.m. UTC
nfs block module uses libnfs and uses pkg-config to determine
its build information.  Somehow it used only --libs, not --cflags,
and added those libs into global $LIBS, instead of using per-object
variable.

Use both --libs and --cflags, use them as per-object variable,
and finally make block/nfs.o to be modular.

Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 block/Makefile.objs |    2 ++
 configure           |    6 ++++--
 2 files changed, 6 insertions(+), 2 deletions(-)

Comments

Peter Lieven May 13, 2014, 1:04 p.m. UTC | #1
On 11.05.2014 15:07, Michael Tokarev wrote:
> nfs block module uses libnfs and uses pkg-config to determine
> its build information.  Somehow it used only --libs, not --cflags,
> and added those libs into global $LIBS, instead of using per-object
> variable.

the missing cflags stuff was due to a bug in the libnfs.pc file.

https://github.com/sahlberg/libnfs/commit/d47c989d886e5398143d43d3b6d25fdf7210cb11

as there where no special flags I dropped it. this was before we realized
that we had to depend on a newer libnfs version anyway. thanks for spotting this.

>
> Use both --libs and --cflags, use them as per-object variable,
> and finally make block/nfs.o to be modular.

can you explain the modular stuff?

Thanks,
Peter

>
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> ---
>   block/Makefile.objs |    2 ++
>   configure           |    6 ++++--
>   2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/block/Makefile.objs b/block/Makefile.objs
> index fd88c03..38ddc0e 100644
> --- a/block/Makefile.objs
> +++ b/block/Makefile.objs
> @@ -35,5 +35,7 @@ gluster.o-cflags   := $(GLUSTERFS_CFLAGS)
>   gluster.o-libs     := $(GLUSTERFS_LIBS)
>   ssh.o-cflags       := $(LIBSSH2_CFLAGS)
>   ssh.o-libs         := $(LIBSSH2_LIBS)
> +nfs.o-cflags       := $(LIBNFS_CFLAGS)
> +nfs.o-libs         := $(LIBNFS_LIBS)
>   qcow.o-libs        := -lz
>   linux-aio.o-libs   := -laio
> diff --git a/configure b/configure
> index ac2fa15..3bc91f6 100755
> --- a/configure
> +++ b/configure
> @@ -3928,7 +3928,7 @@ if test "$libnfs" != "no" ; then
>     if $pkg_config --atleast-version=1.9.3 libnfs; then
>       libnfs="yes"
>       libnfs_libs=$($pkg_config --libs libnfs)
> -    LIBS="$LIBS $libnfs_libs"
> +    libnfs_cflags=$($pkg_config --cflags libnfs)
>     else
>       if test "$libnfs" = "yes" ; then
>         feature_not_found "libnfs"
> @@ -4534,7 +4534,9 @@ if test "$libiscsi" = "yes" ; then
>   fi
>   
>   if test "$libnfs" = "yes" ; then
> -  echo "CONFIG_LIBNFS=y" >> $config_host_mak
> +  echo "CONFIG_LIBNFS=m" >> $config_host_mak
> +  echo "LIBNFS_CFLAGS=$libnfs_cflags" >> $config_host_mak
> +  echo "LIBNFS_LIBS=$libnfs_libs" >> $config_host_mak
>   fi
>   
>   if test "$seccomp" = "yes"; then
Fam Zheng May 13, 2014, 1:19 p.m. UTC | #2
On Tue, 05/13 15:04, Peter Lieven wrote:
> On 11.05.2014 15:07, Michael Tokarev wrote:
> >nfs block module uses libnfs and uses pkg-config to determine
> >its build information.  Somehow it used only --libs, not --cflags,
> >and added those libs into global $LIBS, instead of using per-object
> >variable.
> 
> the missing cflags stuff was due to a bug in the libnfs.pc file.
> 
> https://github.com/sahlberg/libnfs/commit/d47c989d886e5398143d43d3b6d25fdf7210cb11
> 
> as there where no special flags I dropped it. this was before we realized
> that we had to depend on a newer libnfs version anyway. thanks for spotting this.
> 
> >
> >Use both --libs and --cflags, use them as per-object variable,
> >and finally make block/nfs.o to be modular.
> 
> can you explain the modular stuff?

As what is possible with iscsi, gluster and curl now, with this change nfs.o is
possible to be built to a separate block-nfs.so, if configured as
--enable-modules, and loaded on program start.

Fam
Peter Lieven May 13, 2014, 1:35 p.m. UTC | #3
On 13.05.2014 15:19, Fam Zheng wrote:
> On Tue, 05/13 15:04, Peter Lieven wrote:
>> On 11.05.2014 15:07, Michael Tokarev wrote:
>>> nfs block module uses libnfs and uses pkg-config to determine
>>> its build information.  Somehow it used only --libs, not --cflags,
>>> and added those libs into global $LIBS, instead of using per-object
>>> variable.
>> the missing cflags stuff was due to a bug in the libnfs.pc file.
>>
>> https://github.com/sahlberg/libnfs/commit/d47c989d886e5398143d43d3b6d25fdf7210cb11
>>
>> as there where no special flags I dropped it. this was before we realized
>> that we had to depend on a newer libnfs version anyway. thanks for spotting this.
>>
>>> Use both --libs and --cflags, use them as per-object variable,
>>> and finally make block/nfs.o to be modular.
>> can you explain the modular stuff?
> As what is possible with iscsi, gluster and curl now, with this change nfs.o is
> possible to be built to a separate block-nfs.so, if configured as
> --enable-modules, and loaded on program start.

Aah okay. This is something new and not something I did wrong when the patch was submitted?

Peter
Fam Zheng May 13, 2014, 2:15 p.m. UTC | #4
On Tue, 05/13 15:35, Peter Lieven wrote:
> On 13.05.2014 15:19, Fam Zheng wrote:
> >On Tue, 05/13 15:04, Peter Lieven wrote:
> >>On 11.05.2014 15:07, Michael Tokarev wrote:
> >>>nfs block module uses libnfs and uses pkg-config to determine
> >>>its build information.  Somehow it used only --libs, not --cflags,
> >>>and added those libs into global $LIBS, instead of using per-object
> >>>variable.
> >>the missing cflags stuff was due to a bug in the libnfs.pc file.
> >>
> >>https://github.com/sahlberg/libnfs/commit/d47c989d886e5398143d43d3b6d25fdf7210cb11
> >>
> >>as there where no special flags I dropped it. this was before we realized
> >>that we had to depend on a newer libnfs version anyway. thanks for spotting this.
> >>
> >>>Use both --libs and --cflags, use them as per-object variable,
> >>>and finally make block/nfs.o to be modular.
> >>can you explain the modular stuff?
> >As what is possible with iscsi, gluster and curl now, with this change nfs.o is
> >possible to be built to a separate block-nfs.so, if configured as
> >--enable-modules, and loaded on program start.
> 
> Aah okay. This is something new and not something I did wrong when the patch was submitted?

Absolutely.

Fam
diff mbox

Patch

diff --git a/block/Makefile.objs b/block/Makefile.objs
index fd88c03..38ddc0e 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -35,5 +35,7 @@  gluster.o-cflags   := $(GLUSTERFS_CFLAGS)
 gluster.o-libs     := $(GLUSTERFS_LIBS)
 ssh.o-cflags       := $(LIBSSH2_CFLAGS)
 ssh.o-libs         := $(LIBSSH2_LIBS)
+nfs.o-cflags       := $(LIBNFS_CFLAGS)
+nfs.o-libs         := $(LIBNFS_LIBS)
 qcow.o-libs        := -lz
 linux-aio.o-libs   := -laio
diff --git a/configure b/configure
index ac2fa15..3bc91f6 100755
--- a/configure
+++ b/configure
@@ -3928,7 +3928,7 @@  if test "$libnfs" != "no" ; then
   if $pkg_config --atleast-version=1.9.3 libnfs; then
     libnfs="yes"
     libnfs_libs=$($pkg_config --libs libnfs)
-    LIBS="$LIBS $libnfs_libs"
+    libnfs_cflags=$($pkg_config --cflags libnfs)
   else
     if test "$libnfs" = "yes" ; then
       feature_not_found "libnfs"
@@ -4534,7 +4534,9 @@  if test "$libiscsi" = "yes" ; then
 fi
 
 if test "$libnfs" = "yes" ; then
-  echo "CONFIG_LIBNFS=y" >> $config_host_mak
+  echo "CONFIG_LIBNFS=m" >> $config_host_mak
+  echo "LIBNFS_CFLAGS=$libnfs_cflags" >> $config_host_mak
+  echo "LIBNFS_LIBS=$libnfs_libs" >> $config_host_mak
 fi
 
 if test "$seccomp" = "yes"; then