Bug 1206587 - Replace contrib/uuid by a libglusterfs wrapper that uses the uuid implementation from the OS
Summary: Replace contrib/uuid by a libglusterfs wrapper that uses the uuid implementat...
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: GlusterFS
Classification: Community
Component: build
Version: mainline
Hardware: Unspecified
OS: Unspecified
low
low
Target Milestone: ---
Assignee: Niels de Vos
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks: 1217176
TreeView+ depends on / blocked
 
Reported: 2015-03-27 13:23 UTC by Niels de Vos
Modified: 2016-06-16 12:45 UTC (History)
2 users (show)

Fixed In Version: glusterfs-3.8rc2
Clone Of:
: 1217176 (view as bug list)
Environment:
Last Closed: 2016-06-16 12:45:56 UTC
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Embargoed:


Attachments (Terms of Use)

Description Niels de Vos 2015-03-27 13:23:38 UTC
Different OS's have different APIs for using UUID's. At the moment the Gluster sources contain contrib/uuid which provide the Linux API for OS's that provide an other interface.

It is much cleaner to use the API provided by the OS and drop (or at least reduce) the usage of contrib/uuid. This would affect libglusterfs/src/common-utils.{c,h}.

From Manu:

Here is OpenGroup flavor (NetBSD, MacOS X, etc...):
int32_t uuid_compare(const uuid_t *, const uuid_t *, uint32_t *);
void    uuid_create(uuid_t *, uint32_t *);
void    uuid_create_nil(uuid_t *, uint32_t *);
int32_t uuid_equal(const uuid_t *, const uuid_t *, uint32_t *);
void    uuid_from_string(const char *, uuid_t *, uint32_t *);
uint16_t uuid_hash(const uuid_t *, uint32_t *);
int32_t uuid_is_nil(const uuid_t *, uint32_t *);
void    uuid_to_string(const uuid_t *, char **, uint32_t *);
void    uuid_enc_le(void *, const uuid_t *);
void    uuid_dec_le(const void *, uuid_t *);
void    uuid_enc_be(void *, const uuid_t *);
void    uuid_dec_be(const void *, uuid_t *);

Here is Linux API, used by glusterfs, and available in contrib/uuid:
void uuid_clear(uuid_t uu);
int uuid_compare(const uuid_t uu1, const uuid_t uu2);
void uuid_copy(uuid_t dst, const uuid_t src);
void uuid_generate(uuid_t out);
void uuid_generate_random(uuid_t out);
void uuid_generate_time(uuid_t out);
int uuid_is_null(const uuid_t uu);
int uuid_parse(const char *in, uuid_t uu);
void uuid_unparse(const uuid_t uu, char *out);
void uuid_unparse_lower(const uuid_t uu, char *out);
void uuid_unparse_upper(const uuid_t uu, char *out);
time_t uuid_time(const uuid_t uu, struct timeval *ret_tv);
int uuid_type(const uuid_t uu);
int uuid_variant(const uuid_t uu);

Comment 1 Anand Avati 2015-03-27 16:39:12 UTC
REVIEW: http://review.gluster.org/10017 (Avoid conflict between contrib/uuid and system uuid) posted (#5) for review on master by Emmanuel Dreyfus (manu)

Comment 2 Anand Avati 2015-03-29 04:06:37 UTC
REVIEW: http://review.gluster.org/10017 (Avoid conflict between contrib/uuid and system uuid) posted (#6) for review on master by Emmanuel Dreyfus (manu)

Comment 3 Anand Avati 2015-03-30 14:37:31 UTC
REVIEW: http://review.gluster.org/10017 (Avoid conflict between contrib/uuid and system uuid) posted (#7) for review on master by Emmanuel Dreyfus (manu)

Comment 4 Anand Avati 2015-04-01 14:18:21 UTC
REVIEW: http://review.gluster.org/10017 (Avoid conflict between contrib/uuid and system uuid) posted (#8) for review on master by Emmanuel Dreyfus (manu)

Comment 5 Anand Avati 2015-04-02 13:52:22 UTC
REVIEW: http://review.gluster.org/10017 (Avoid conflict between contrib/uuid and system uuid) posted (#9) for review on master by Emmanuel Dreyfus (manu)

Comment 6 Anand Avati 2015-04-04 17:48:41 UTC
COMMIT: http://review.gluster.org/10017 committed in master by Vijay Bellur (vbellur) 
------
commit 28397cae4102ac3f08576ebaf071ad92683097e8
Author: Emmanuel Dreyfus <manu>
Date:   Thu Apr 2 15:51:30 2015 +0200

    Avoid conflict between contrib/uuid and system uuid
    
    glusterfs relies on Linux uuid implementation, which
    API is incompatible with most other systems's uuid. As
    a result, libglusterfs has to embed contrib/uuid,
    which is the Linux implementation, on non Linux systems.
    This implementation is incompatible with systtem's
    built in, but the symbols have the same names.
    
    Usually this is not a problem because when we link
    with -lglusterfs, libc's symbols are trumped. However
    there is a problem when a program not linked with
    -lglusterfs will dlopen() glusterfs component. In
    such a case, libc's uuid implementation is already
    loaded in the calling program, and it will be used
    instead of libglusterfs's implementation, causing
    crashes.
    
    A possible workaround is to use pre-load libglusterfs
    in the calling program (using LD_PRELOAD on NetBSD for
    instance), but such a mechanism is not portable, nor
    is it flexible. A much better approach is to rename
    libglusterfs's uuid_* functions to gf_uuid_* to avoid
    any possible conflict. This is what this change attempts.
    
    BUG: 1206587
    Change-Id: I9ccd3e13afed1c7fc18508e92c7beb0f5d49f31a
    Signed-off-by: Emmanuel Dreyfus <manu>
    Reviewed-on: http://review.gluster.org/10017
    Tested-by: Gluster Build System <jenkins.com>
    Reviewed-by: Niels de Vos <ndevos>

Comment 7 Anand Avati 2015-04-06 05:52:00 UTC
REVIEW: http://review.gluster.org/10129 (build: make contrib/uuid dependency optional) posted (#1) for review on master by Niels de Vos (ndevos)

Comment 8 Anand Avati 2015-04-07 19:34:53 UTC
REVIEW: http://review.gluster.org/10129 (build: make contrib/uuid dependency optional) posted (#2) for review on master by Niels de Vos (ndevos)

Comment 9 Anand Avati 2015-04-08 04:08:32 UTC
REVIEW: http://review.gluster.org/10129 (build: make contrib/uuid dependency optional) posted (#3) for review on master by Niels de Vos (ndevos)

Comment 10 Anand Avati 2015-04-10 11:41:44 UTC
COMMIT: http://review.gluster.org/10129 committed in master by Vijay Bellur (vbellur) 
------
commit 6eb27480b6559103e4437facd7aecbcd373479c9
Author: Niels de Vos <ndevos>
Date:   Fri Apr 3 18:14:13 2015 +0200

    build: make contrib/uuid dependency optional
    
    On Linux systems we should use the libuuid from the distribution and not
    bundle and statically link the contrib/uuid/ bits.
    
    libglusterfs/src/compat-uuid.h has been introduced and should become an
    abstraction layer for different UUID APIs. Non-Linux operating systems
    should implement their compatibility layer there.
    
    Once all operating systems have an implementation in compat-uuid.h, we
    can remove contrib/uuid/ from the repository completely.
    
    Change-Id: I345e5357644be2521685e00358bb8c83c4ea0577
    BUG: 1206587
    Signed-off-by: Niels de Vos <ndevos>
    Reviewed-on: http://review.gluster.org/10129
    Tested-by: Gluster Build System <jenkins.com>
    Reviewed-by: Vijay Bellur <vbellur>

Comment 11 Kaleb KEITHLEY 2015-04-29 17:21:18 UTC
Building master and release-3.7 on Debian Jessie fails:

...
Making all in heal
Making all in src
  CC       glfs-heal.o
  CCLD     glfsheal
/usr/bin/ld: glfs-heal.o: undefined reference to symbol 'uuid_clear@@UUID_1.0'
//lib/x86_64-linux-gnu/libuuid.so.1: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status
Makefile:503: recipe for target 'glfsheal' failed
make[3]: *** [glfsheal] Error 1


$ diff -u heal/src/Makefile.am.orig heal/src/Makefile.am
--- heal/src/Makefile.am.orig   2015-04-29 13:12:39.580000000 -0400
+++ heal/src/Makefile.am        2015-04-29 13:12:50.012000000 -0400
@@ -8,7 +8,7 @@
                $(top_builddir)/api/src/libgfapi.la \
                $(GF_GLUSTERFS_LIBS) $(XML_LIBS) $(GFAPI_LIBS)
 
-glfsheal_LDFLAGS = $(GF_LDFLAGS)
+glfsheal_LDFLAGS = $(GF_LDFLAGS) $(UUID_LIBS)
 
 AM_CPPFLAGS = $(GF_CPPFLAGS) \
        -I$(top_srcdir)/xlators/lib/src\

Comment 12 Anand Avati 2015-04-29 19:06:17 UTC
REVIEW: http://review.gluster.org/10453 (build: glfsheal should link against $(UUID_LIBS)) posted (#1) for review on master by Niels de Vos (ndevos)

Comment 13 Anand Avati 2015-04-30 06:26:18 UTC
COMMIT: http://review.gluster.org/10453 committed in master by Niels de Vos (ndevos) 
------
commit 0d3759fa8b4c0ba1c2d747ab96b9fd2442488635
Author: Niels de Vos <ndevos>
Date:   Wed Apr 29 21:03:39 2015 +0200

    build: glfsheal should link against $(UUID_LIBS)
    
    Without this, building fails on Debian Jessie:
    
        /usr/bin/ld: glfs-heal.o: undefined reference to symbol 'uuid_clear@@UUID_1.0'
        /lib/x86_64-linux-gnu/libuuid.so.1: error adding symbols: DSO missing from command line
        collect2: error: ld returned 1 exit status
        Makefile:503: recipe for target 'glfsheal' failed
    
    While at it, clean some of the unused defines too.
    
    BUG: 1206587
    Change-Id: I2e513b2a806e6241542e60007d720cca16dabb06
    Signed-off-by: Niels de Vos <ndevos>
    Reviewed-on: http://review.gluster.org/10453
    Tested-by: Gluster Build System <jenkins.com>
    Reviewed-by: Kaleb KEITHLEY <kkeithle>

Comment 14 Niels de Vos 2015-05-14 17:29:24 UTC
This bug is getting closed because a release has been made available that should address the reported issue. In case the problem is still not fixed with glusterfs-3.7.0, please open a new bug report.

glusterfs-3.7.0 has been announced on the Gluster mailinglists [1], packages for several distributions should become available in the near future. Keep an eye on the Gluster Users mailinglist [2] and the update infrastructure for your distribution.

[1] http://thread.gmane.org/gmane.comp.file-systems.gluster.devel/10939
[2] http://thread.gmane.org/gmane.comp.file-systems.gluster.user

Comment 15 Niels de Vos 2015-05-14 17:35:55 UTC
This bug is getting closed because a release has been made available that should address the reported issue. In case the problem is still not fixed with glusterfs-3.7.0, please open a new bug report.

glusterfs-3.7.0 has been announced on the Gluster mailinglists [1], packages for several distributions should become available in the near future. Keep an eye on the Gluster Users mailinglist [2] and the update infrastructure for your distribution.

[1] http://thread.gmane.org/gmane.comp.file-systems.gluster.devel/10939
[2] http://thread.gmane.org/gmane.comp.file-systems.gluster.user

Comment 16 Niels de Vos 2015-05-14 17:38:16 UTC
This bug is getting closed because a release has been made available that should address the reported issue. In case the problem is still not fixed with glusterfs-3.7.0, please open a new bug report.

glusterfs-3.7.0 has been announced on the Gluster mailinglists [1], packages for several distributions should become available in the near future. Keep an eye on the Gluster Users mailinglist [2] and the update infrastructure for your distribution.

[1] http://thread.gmane.org/gmane.comp.file-systems.gluster.devel/10939
[2] http://thread.gmane.org/gmane.comp.file-systems.gluster.user

Comment 17 Niels de Vos 2015-05-14 17:46:38 UTC
This bug is getting closed because a release has been made available that should address the reported issue. In case the problem is still not fixed with glusterfs-3.7.0, please open a new bug report.

glusterfs-3.7.0 has been announced on the Gluster mailinglists [1], packages for several distributions should become available in the near future. Keep an eye on the Gluster Users mailinglist [2] and the update infrastructure for your distribution.

[1] http://thread.gmane.org/gmane.comp.file-systems.gluster.devel/10939
[2] http://thread.gmane.org/gmane.comp.file-systems.gluster.user

Comment 18 Niels de Vos 2016-06-16 12:45:56 UTC
This bug is getting closed because a release has been made available that should address the reported issue. In case the problem is still not fixed with glusterfs-3.8.0, please open a new bug report.

glusterfs-3.8.0 has been announced on the Gluster mailinglists [1], packages for several distributions should become available in the near future. Keep an eye on the Gluster Users mailinglist [2] and the update infrastructure for your distribution.

[1] http://blog.gluster.org/2016/06/glusterfs-3-8-released/
[2] http://thread.gmane.org/gmane.comp.file-systems.gluster.user


Note You need to log in before you can comment on or make changes to this bug.