[Qemu-devel] [PATCH] buildsys: Move rdma libs to per object

Fam Zheng posted 1 patch 8 years, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170907084230.26493-1-famz@redhat.com
Test checkpatch passed
Test docker passed
Test s390x passed
configure               | 2 +-
migration/Makefile.objs | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
[Qemu-devel] [PATCH] buildsys: Move rdma libs to per object
Posted by Fam Zheng 8 years, 5 months ago
Signed-off-by: Fam Zheng <famz@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


Re: [Qemu-devel] [PATCH] buildsys: Move rdma libs to per object
Posted by Dr. David Alan Gilbert 8 years, 5 months ago
* 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

Re: [Qemu-devel] [PATCH] buildsys: Move rdma libs to per object
Posted by Fam Zheng 8 years, 5 months ago
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

Re: [Qemu-devel] [PATCH] buildsys: Move rdma libs to per object
Posted by Dr. David Alan Gilbert 8 years, 5 months ago
* 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

Re: [Qemu-devel] [PATCH] buildsys: Move rdma libs to per object
Posted by Daniel P. Berrange 8 years, 5 months ago
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
-- 
|: 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 :|

Re: [Qemu-devel] [PATCH] buildsys: Move rdma libs to per object
Posted by Peter Xu 8 years, 5 months ago
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
> 

-- 
Peter Xu

Re: [Qemu-devel] [PATCH] buildsys: Move rdma libs to per object
Posted by Juan Quintela 8 years, 5 months ago
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)

Re: [Qemu-devel] [PATCH] buildsys: Move rdma libs to per object
Posted by Fam Zheng 8 years, 5 months ago
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