On (14/10/14 17:49), Sumit Bose wrote:
On Tue, Oct 14, 2014 at 05:36:00PM +0200, Lukas Slebodnik wrote:
> On (14/10/14 17:20), Sumit Bose wrote:
> >On Tue, Oct 14, 2014 at 04:30:15PM +0200, Lukas Slebodnik wrote:
> >> On (14/10/14 11:09), Sumit Bose wrote:
> >> >Hi,
> >> >
> >> >this patch adds a new interface to the nss responder which is similar
to
> >> >SSS_NSS_GETSIDBYNAME but can return more information about the given
> >> >object than just the SID and which is not available with the POSIX
> >> >interface as well. As mentioned in the commit messages the main use
case
> >> >is the extdom plugin on the FreeIPA server.
> >> >
> >> >bye,
> >> >Sumit
> >>
> >> >From f8500f00224e781b03ebab0ab396565e4c034489 Mon Sep 17 00:00:00 2001
> >> >From: Sumit Bose <sbose(a)redhat.com>
> >> >Date: Thu, 9 Oct 2014 21:05:34 +0200
> >> >Subject: [PATCH 2/2] sss_nss_idmap: add sss_nss_getorigbyname()
> >> >
> >> >This patch adds an interface to the new SSS_NSS_GETORIGBYNAME request
of
> >> >the nss responder to libsss_nss_idmap.
> >> >
> >> >The main use case for this new call is to replace
sss_nss_getsidbyname()
> >> >in the extdom plugin on the FreeIPA server to get more information
about
> >> >the given object than just the SID which is not available with the
> >> >default POSIX interfaces.
> >> >---
> >> > src/sss_client/idmap/sss_nss_idmap.c | 113
+++++++++++++++++++++++++++++
> >> > src/sss_client/idmap/sss_nss_idmap.exports | 2 +
> >> > src/sss_client/idmap/sss_nss_idmap.h | 32 ++++++++
> >> > src/tests/cmocka/sss_nss_idmap-tests.c | 26 ++++++-
> >> > 4 files changed, 172 insertions(+), 1 deletion(-)
> >> >
> >> //snip
> >>
> >> >diff --git a/src/sss_client/idmap/sss_nss_idmap.exports
b/src/sss_client/idmap/sss_nss_idmap.exports
> >> >index 7b8488b..ba340ca 100644
> >> >--- a/src/sss_client/idmap/sss_nss_idmap.exports
> >> >+++ b/src/sss_client/idmap/sss_nss_idmap.exports
> >> >@@ -7,6 +7,8 @@ SSS_NSS_IDMAP_0.0.1 {
> >> > sss_nss_getsidbyid;
> >> > sss_nss_getnamebysid;
> >> > sss_nss_getidbysid;
> >> >+ sss_nss_getorigbyname;
> >> >+ sss_nss_free_kv;
> >>
> >> It works but it is not the right solution. The purpose of version symbol
file
> >> is to help linker distinguish between two version of libraries with
backward
> >> compatible changes. (just new functions were added).
> >> Without version symbol file, linker cannot detect differences due to the
same
> >> soname and it need to be solved in downstream packaging.
> >>
> >> sh$ objdump -p .libs/libsss_nss_idmap.so | grep SONAME
> >> SONAME libsss_nss_idmap.so.0
> >>
> >> The right approach is to add new version as in
> >> ./src/sss_client/libwbclient/wbclient.exports
> >> The new version will extend previous one.
> >>
> >> When I created version symbol files, I decided to use full version of
library
> >> generated from version-info. It could be any number, but it is easier
> >> to backport changes if version is not related to sssd version.
> >>
> >> Provisioned version with attached patch:
> >> sh$ objdump -p .libs/libsss_nss_idmap.so | grep -A5 "Version
definitions"Version definitions:
> >> 1 0x01 0x0b2b7380 libsss_nss_idmap.so.0
> >> 2 0x00 0x0597c1a1 SSS_NSS_IDMAP_0.0.1
> >> 3 0x00 0x0597c2a0 SSS_NSS_IDMAP_0.1.0
> >> SSS_NSS_IDMAP_0.0.1
> >>
> >> Version symbol file add version to each function:
> >> sh$ objdump -T .libs/libsss_nss_idmap.so | grep SSS_NSS_IDMAP
> >> 0000000000000000 g DO *ABS* 0000000000000000 SSS_NSS_IDMAP_0.0.1
SSS_NSS_IDMAP_0.0.1
> >> 0000000000001930 g DF .text 0000000000000052 SSS_NSS_IDMAP_0.1.0
sss_nss_getorigbyname
> >> 0000000000001420 g DF .text 0000000000000052 SSS_NSS_IDMAP_0.1.0
sss_nss_free_kv
> >> 0000000000001830 g DF .text 000000000000003f SSS_NSS_IDMAP_0.0.1
sss_nss_getsidbyid
> >> 0000000000001870 g DF .text 0000000000000052 SSS_NSS_IDMAP_0.0.1
sss_nss_getnamebysid
> >> 0000000000000000 g DO *ABS* 0000000000000000 SSS_NSS_IDMAP_0.1.0
SSS_NSS_IDMAP_0.1.0
> >> 00000000000017d0 g DF .text 0000000000000052 SSS_NSS_IDMAP_0.0.1
sss_nss_getsidbyname
> >> 00000000000018d0 g DF .text 0000000000000058 SSS_NSS_IDMAP_0.0.1
sss_nss_getidbysid
> >>
> >> So linker will be able to distinguish between two libraries with the same
> >> soname and automatically requires necessary versions.
> >>
> >> > # everything else is local
> >> > local:
> >>
> >> and version-info should be bumped as well.
> >
> >Thank you Lukas I really forgot to update the version. While adding the
> >new calls to the exports file I was thinking that I can fix the version
> >later when I know that everything is working as expected.
> >
> >Would you mind to send your changes as a proper patch to this thread so
> >that the attributions are correct?
> >
> It is part of review. I don't want to have another patch. I just wanted to
> simplify your job. Feel free to squash changes into your patches.
Thank you. New version attached.
bye,
Sumit
ACK
LS