On Fri, Jan 03, 2014 at 06:22:11PM -0500, S.Çağlar Onur wrote: > Hi, > > On Fri, Jan 3, 2014 at 4:32 PM, Stéphane Graber <[email protected]> wrote: > > On Fri, Jan 03, 2014 at 04:22:51PM -0500, S.Çağlar Onur wrote: > >> Hi, > >> > >> On Fri, Jan 3, 2014 at 3:13 PM, Stéphane Graber <[email protected]> > >> wrote: > >> > On Thu, Jan 02, 2014 at 08:59:10AM -0600, Serge Hallyn wrote: > >> >> Quoting Stéphane Graber ([email protected]): > >> >> > On Wed, Jan 01, 2014 at 11:37:32PM -0600, Serge Hallyn wrote: > >> >> > > Quoting Stéphane Graber ([email protected]): > >> >> > > > This patch caused a build failure on Android: > >> >> > > > > >> >> > > > arm-linux-androideabi-gcc > >> >> > > > --sysroot=/tmp/android-build-scripts/android-ndk-r9b/platforms/android-9/arch-arm/ > >> >> > > > -DHAVE_CONFIG_H -I. -I../../src -fPIC -DPIC -I../../src > >> >> > > > -DLXCROOTFSMOUNT=\"/data/lxc/lxc/lib/lxc/rootfs\" > >> >> > > > -DLXCPATH=\"/data/lxc/lxc/var/lib/lxc\" > >> >> > > > -DLXC_GLOBAL_CONF=\"/data/lxc/lxc/etc/lxc/lxc.conf\" > >> >> > > > -DLXCINITDIR=\"/data/lxc/lxc/libexec\" > >> >> > > > -DLXCTEMPLATEDIR=\"/data/lxc/lxc/share/lxc/templates\" > >> >> > > > -DLOGPATH=\"/data/lxc/lxc/var/log/lxc\" > >> >> > > > -DLXC_DEFAULT_CONFIG=\"/data/lxc/lxc/etc/lxc/default.conf\" > >> >> > > > -DLXClxclock.c: In function 'process_lock_setup_atfork': > >> >> > > > lxclock.c:332:2: error: implicit declaration of function > >> >> > > > 'pthread_atfork' [-Werror=implicit-function-declaration] > >> >> > > > pthread_atfork(process_lock, process_unlock, process_unlock); > >> >> > > > >> >> > > Hm, according to > >> >> > > http://stackoverflow.com/questions/12370970/undefined-reference-to-pthread-atfork-while-i-was-trying-to-port-libpcsclite-t > >> >> > > perhaps all we need is to declare the fn before its use. Do you > >> >> > > have a local bionic compiler handy to test that on? > >> >> > > >> >> > Not having much luck with that... this just turned it into: > >> >> > > >> >> > liblxc.so: error: undefined reference to 'pthread_atfork' > >> >> > > >> >> > pthread on bionic is part of the C library so no extra linker flags > >> >> > should be needed to make this work... > >> >> > >> >> Hm. So we can either (a) keep looking for a way to make it work in > >> >> bionic, (b) give in and switch all uses of fork() to lxc_fork() to > >> >> do our own atfork hook, or (c) detect at configure.ac whether we have > >> >> pthread_atfork(), and if not then just don't do it. > >> > > >> > I think doing (c) would be the easiest here and would also mean that > >> > whenever pthread_atfork actually shows up in bionic (assuming the > >> > configure.ac detection code doesn't already detect it as present...), > >> > things will just work. > >> > > >> > It may be worth adding an entry to the configure.ac config overview part > >> > to indicate threading support so it's clear to anyone building on bionic > >> > that they shouldn't expect this to be working. > >> > >> What about something like [1] (after reverting > >> 64b1be2903078ef9e9ba3ffcbc30a4dc9bc5cc6c)? > >> > >> It adds the pthread_atfork check, removes static_lock/unlock, puts the > >> values array into TLS for non-bionic targets and lastly add a warning > >> for bionic users. > >> > >> [1] http://10ur.org/lock.patch > > > > Hmm, why use IS_BIONIC at some places and HAVE_PTHREAD_ATFORK at others? > > > > I'm not familar at all with pthread but assuming the two ifdefed chunk > > of code are interdependent, having both of them under the same condition > > would make more sense to me. > > > > I also would love to eventually get rid of IS_BIONIC/is_bionic and just > > rely on feature checks so I think it'd be best to have the warning > > issued if any of the pthread related checks fail (whatever the C library > > in use) and have lxclock.c check the feature flags (or introduce a new > > HAS_THREADS define if easier) instead of looking for bionic. > > I think new version at http://10ur.org/lock.patch addresses both yours > and Serge's comments (I hate gmail for making me upload stuff instead > of pasting inline). Let me know what you think so that I can send it > as a proper patch.
Looks good to me. Thanks. > > > -- > > Stéphane Graber > > Ubuntu developer > > http://www.ubuntu.com > > > > _______________________________________________ > > lxc-devel mailing list > > [email protected] > > http://lists.linuxcontainers.org/listinfo/lxc-devel > > > > > > -- > S.Çağlar Onur <[email protected]> > _______________________________________________ > lxc-devel mailing list > [email protected] > http://lists.linuxcontainers.org/listinfo/lxc-devel -- Stéphane Graber Ubuntu developer http://www.ubuntu.com
signature.asc
Description: Digital signature
_______________________________________________ lxc-devel mailing list [email protected] http://lists.linuxcontainers.org/listinfo/lxc-devel
