[dpdk-dev] mk: Make XEN_PMD build in combined library mode

Message ID 1449059922-10976-1-git-send-email-christian.ehrhardt@canonical.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Christian Ehrhardt Dec. 2, 2015, 12:38 p.m. UTC
  Building RTE_LIBRTE_PMD_XENVIRT was broken when RTE_BUILD_COMBINE_LIBS was
enabled (http://dpdk.org/ml/archives/dev/2015-November/028660.html).
Now the underlying issue is rather simple, the xen code needs libxenstore.
But rte.app.mk so far only considered that when RTE_BUILD_COMBINE_LIBS was
disabled.
While it is correct to create the DPDK sublib linking only in the
RTE_BUILD_COMBINE_LIBS=n case, the libxenstore should be added to the linked
libs in any case if RTE_LIBRTE_PMD_XENVIRT is enabled.

Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
---

[diffstat]
 rte.app.mk |    2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

[diff]
  

Comments

Thomas Monjalon Dec. 2, 2015, 11:14 p.m. UTC | #1
2015-12-02 13:38, Christian Ehrhardt:
> Building RTE_LIBRTE_PMD_XENVIRT was broken when RTE_BUILD_COMBINE_LIBS was
> enabled (http://dpdk.org/ml/archives/dev/2015-November/028660.html).
> Now the underlying issue is rather simple, the xen code needs libxenstore.
> But rte.app.mk so far only considered that when RTE_BUILD_COMBINE_LIBS was
> disabled.
> While it is correct to create the DPDK sublib linking only in the
> RTE_BUILD_COMBINE_LIBS=n case, the libxenstore should be added to the linked
> libs in any case if RTE_LIBRTE_PMD_XENVIRT is enabled.
[...]
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -113,6 +113,7 @@ endif # ! CONFIG_RTE_BUILD_SHARED_LIBS
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_BNX2X_PMD)      += -lz
>  
>  _LDLIBS-y += --start-group
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT)    += -lxenstore

Why is it moved after --start-group and not above as other dependencies?

>  ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n)
  
Christian Ehrhardt Dec. 3, 2015, 6:56 a.m. UTC | #2
Hi Thomas,
it was already after the --start-group before my patch, only guarded
by the "ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n)"
I've had no opinion on being inside/outside the group.

I just wanted to move it out of the "ifeq" which was not appropriate
for the -lxenstore.
I didn't want to cause any other semantic change, so I kept it inside the group.

But you are right, looking at it without just focusing on the ifeq it
makes more sense to move it out of the group as well.
I'll resubmit a slightly modified version.


Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd


On Thu, Dec 3, 2015 at 12:14 AM, Thomas Monjalon
<thomas.monjalon@6wind.com> wrote:
> 2015-12-02 13:38, Christian Ehrhardt:
>> Building RTE_LIBRTE_PMD_XENVIRT was broken when RTE_BUILD_COMBINE_LIBS was
>> enabled (http://dpdk.org/ml/archives/dev/2015-November/028660.html).
>> Now the underlying issue is rather simple, the xen code needs libxenstore.
>> But rte.app.mk so far only considered that when RTE_BUILD_COMBINE_LIBS was
>> disabled.
>> While it is correct to create the DPDK sublib linking only in the
>> RTE_BUILD_COMBINE_LIBS=n case, the libxenstore should be added to the linked
>> libs in any case if RTE_LIBRTE_PMD_XENVIRT is enabled.
> [...]
>> --- a/mk/rte.app.mk
>> +++ b/mk/rte.app.mk
>> @@ -113,6 +113,7 @@ endif # ! CONFIG_RTE_BUILD_SHARED_LIBS
>>  _LDLIBS-$(CONFIG_RTE_LIBRTE_BNX2X_PMD)      += -lz
>>
>>  _LDLIBS-y += --start-group
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT)    += -lxenstore
>
> Why is it moved after --start-group and not above as other dependencies?
>
>>  ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n)
>
  

Patch

diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index 85a680d..f003187 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -113,6 +113,7 @@  endif # ! CONFIG_RTE_BUILD_SHARED_LIBS
 _LDLIBS-$(CONFIG_RTE_LIBRTE_BNX2X_PMD)      += -lz
 
 _LDLIBS-y += --start-group
+_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT)    += -lxenstore
 
 ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n)
 
@@ -130,7 +131,6 @@  _LDLIBS-$(CONFIG_RTE_LIBRTE_CFGFILE)        += -lrte_cfgfile
 _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_BOND)       += -lrte_pmd_bond
 
 _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT)    += -lrte_pmd_xenvirt
-_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT)    += -lxenstore
 
 ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n)
 # plugins (link only if static libraries)