diff mbox series

buildsys: Move rdma libs to per object

Message ID 20170907084230.26493-1-famz@redhat.com
State New
Headers show
Series buildsys: Move rdma libs to per object | expand

Commit Message

Fam Zheng Sept. 7, 2017, 8:42 a.m. UTC
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 configure               | 2 +-
 migration/Makefile.objs | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

Comments

Dr. David Alan Gilbert Sept. 7, 2017, 9:37 a.m. UTC | #1
* Fam Zheng (famz@redhat.com) wrote:
> Signed-off-by: Fam Zheng <famz@redhat.com>

OK, I've not actually got a preference as to whether it's
per-object or not - I don't really see any advantage.

But since it doesn't look like it'll break it;

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  configure               | 2 +-
>  migration/Makefile.objs | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index fb7e34a901..470b8a085b 100755
> --- a/configure
> +++ b/configure
> @@ -2818,7 +2818,6 @@ EOF
>    rdma_libs="-lrdmacm -libverbs"
>    if compile_prog "" "$rdma_libs" ; then
>      rdma="yes"
> -    libs_softmmu="$libs_softmmu $rdma_libs"
>    else
>      if test "$rdma" = "yes" ; then
>          error_exit \
> @@ -6035,6 +6034,7 @@ echo "CONFIG_TRACE_FILE=$trace_file" >> $config_host_mak
>  
>  if test "$rdma" = "yes" ; then
>    echo "CONFIG_RDMA=y" >> $config_host_mak
> +  echo "RDMA_LIBS=$rdma_libs" >> $config_host_mak
>  fi
>  
>  if test "$have_rtnetlink" = "yes" ; then
> diff --git a/migration/Makefile.objs b/migration/Makefile.objs
> index 1c7770da46..99e038024d 100644
> --- a/migration/Makefile.objs
> +++ b/migration/Makefile.objs
> @@ -11,3 +11,4 @@ common-obj-$(CONFIG_RDMA) += rdma.o
>  
>  common-obj-$(CONFIG_LIVE_BLOCK_MIGRATION) += block.o
>  
> +rdma.o-libs := $(RDMA_LIBS)
> -- 
> 2.13.5
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Peter Xu Sept. 7, 2017, 9:46 a.m. UTC | #2
On Thu, Sep 07, 2017 at 04:42:30PM +0800, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <famz@redhat.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

> ---
>  configure               | 2 +-
>  migration/Makefile.objs | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index fb7e34a901..470b8a085b 100755
> --- a/configure
> +++ b/configure
> @@ -2818,7 +2818,6 @@ EOF
>    rdma_libs="-lrdmacm -libverbs"
>    if compile_prog "" "$rdma_libs" ; then
>      rdma="yes"
> -    libs_softmmu="$libs_softmmu $rdma_libs"
>    else
>      if test "$rdma" = "yes" ; then
>          error_exit \
> @@ -6035,6 +6034,7 @@ echo "CONFIG_TRACE_FILE=$trace_file" >> $config_host_mak
>  
>  if test "$rdma" = "yes" ; then
>    echo "CONFIG_RDMA=y" >> $config_host_mak
> +  echo "RDMA_LIBS=$rdma_libs" >> $config_host_mak
>  fi
>  
>  if test "$have_rtnetlink" = "yes" ; then
> diff --git a/migration/Makefile.objs b/migration/Makefile.objs
> index 1c7770da46..99e038024d 100644
> --- a/migration/Makefile.objs
> +++ b/migration/Makefile.objs
> @@ -11,3 +11,4 @@ common-obj-$(CONFIG_RDMA) += rdma.o
>  
>  common-obj-$(CONFIG_LIVE_BLOCK_MIGRATION) += block.o
>  
> +rdma.o-libs := $(RDMA_LIBS)
> -- 
> 2.13.5
>
Fam Zheng Sept. 7, 2017, 11:37 a.m. UTC | #3
On Thu, 09/07 10:37, Dr. David Alan Gilbert wrote:
> * Fam Zheng (famz@redhat.com) wrote:
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> 
> OK, I've not actually got a preference as to whether it's
> per-object or not - I don't really see any advantage.

Thanks for the review.  You're right this probably doesn't make a difference
except for a bit more consistency, until we want to make rdma a module (as in
--enable-modules) like the ones in block layer. The -libs and -cflags variables
were initially added just for that.

While we are talking about it, is there any reason why that will not be a good
idea?  There are other libraries used by QEMU outside block layer that are
overdue to be converted to modules, like ui (gtk, sdl, etc.), rdma seems to be a
candidate too.

Fam
Dr. David Alan Gilbert Sept. 7, 2017, 11:54 a.m. UTC | #4
* Fam Zheng (famz@redhat.com) wrote:
> On Thu, 09/07 10:37, Dr. David Alan Gilbert wrote:
> > * Fam Zheng (famz@redhat.com) wrote:
> > > Signed-off-by: Fam Zheng <famz@redhat.com>
> > 
> > OK, I've not actually got a preference as to whether it's
> > per-object or not - I don't really see any advantage.
> 
> Thanks for the review.  You're right this probably doesn't make a difference
> except for a bit more consistency, until we want to make rdma a module (as in
> --enable-modules) like the ones in block layer. The -libs and -cflags variables
> were initially added just for that.
> 
> While we are talking about it, is there any reason why that will not be a good
> idea?  There are other libraries used by QEMU outside block layer that are
> overdue to be converted to modules, like ui (gtk, sdl, etc.), rdma seems to be a
> candidate too.

I don't think we've ever tried to make the migration code modular;
it probably wouldn't be impossible to split it out - it's wired
into a few places but it should be doable.

Dave

> Fam
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Juan Quintela Sept. 7, 2017, 11:55 a.m. UTC | #5
Fam Zheng <famz@redhat.com> wrote:
> Signed-off-by: Fam Zheng <famz@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>


> ---
>  configure               | 2 +-
>  migration/Makefile.objs | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index fb7e34a901..470b8a085b 100755
> --- a/configure
> +++ b/configure
> @@ -2818,7 +2818,6 @@ EOF
>    rdma_libs="-lrdmacm -libverbs"
>    if compile_prog "" "$rdma_libs" ; then
>      rdma="yes"
> -    libs_softmmu="$libs_softmmu $rdma_libs"
>    else
>      if test "$rdma" = "yes" ; then
>          error_exit \
> @@ -6035,6 +6034,7 @@ echo "CONFIG_TRACE_FILE=$trace_file" >> $config_host_mak
>  
>  if test "$rdma" = "yes" ; then
>    echo "CONFIG_RDMA=y" >> $config_host_mak
> +  echo "RDMA_LIBS=$rdma_libs" >> $config_host_mak
>  fi
>  
>  if test "$have_rtnetlink" = "yes" ; then
> diff --git a/migration/Makefile.objs b/migration/Makefile.objs
> index 1c7770da46..99e038024d 100644
> --- a/migration/Makefile.objs
> +++ b/migration/Makefile.objs
> @@ -11,3 +11,4 @@ common-obj-$(CONFIG_RDMA) += rdma.o
>  
>  common-obj-$(CONFIG_LIVE_BLOCK_MIGRATION) += block.o
>  
> +rdma.o-libs := $(RDMA_LIBS)
Daniel P. Berrangé Sept. 7, 2017, 12:12 p.m. UTC | #6
On Thu, Sep 07, 2017 at 07:37:42PM +0800, Fam Zheng wrote:
> On Thu, 09/07 10:37, Dr. David Alan Gilbert wrote:
> > * Fam Zheng (famz@redhat.com) wrote:
> > > Signed-off-by: Fam Zheng <famz@redhat.com>
> > 
> > OK, I've not actually got a preference as to whether it's
> > per-object or not - I don't really see any advantage.
> 
> Thanks for the review.  You're right this probably doesn't make a difference
> except for a bit more consistency, until we want to make rdma a module (as in
> --enable-modules) like the ones in block layer. The -libs and -cflags variables
> were initially added just for that.
> 
> While we are talking about it, is there any reason why that will not be a good
> idea?  There are other libraries used by QEMU outside block layer that are
> overdue to be converted to modules, like ui (gtk, sdl, etc.), rdma seems to be a
> candidate too.

Since we have per-module flags, it makes sense to use them whereever it is
reasonable todo so. The global flags should only be needed for things which
are truely globally used, which is (almost) only glib2.

Regards,
Daniel
Fam Zheng Sept. 8, 2017, 9:21 a.m. UTC | #7
On Thu, 09/07 16:42, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <famz@redhat.com>

I will include this in a coming pull request. Thanks.

Fam
diff mbox series

Patch

diff --git a/configure b/configure
index fb7e34a901..470b8a085b 100755
--- a/configure
+++ b/configure
@@ -2818,7 +2818,6 @@  EOF
   rdma_libs="-lrdmacm -libverbs"
   if compile_prog "" "$rdma_libs" ; then
     rdma="yes"
-    libs_softmmu="$libs_softmmu $rdma_libs"
   else
     if test "$rdma" = "yes" ; then
         error_exit \
@@ -6035,6 +6034,7 @@  echo "CONFIG_TRACE_FILE=$trace_file" >> $config_host_mak
 
 if test "$rdma" = "yes" ; then
   echo "CONFIG_RDMA=y" >> $config_host_mak
+  echo "RDMA_LIBS=$rdma_libs" >> $config_host_mak
 fi
 
 if test "$have_rtnetlink" = "yes" ; then
diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index 1c7770da46..99e038024d 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -11,3 +11,4 @@  common-obj-$(CONFIG_RDMA) += rdma.o
 
 common-obj-$(CONFIG_LIVE_BLOCK_MIGRATION) += block.o
 
+rdma.o-libs := $(RDMA_LIBS)