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); ^ cc1: all warnings being treated as errors make[3]: *** [liblxc_so-lxclock.o] Error 1 make[3]: Leaving directory `/tmp/android-build-scripts/lxc/src/lxc' make[2]: *** [all-recursive] Error 1 make[2]: Leaving directory `/tmp/android-build-scripts/lxc/src' make[1]: *** [all] Error 2 make[1]: Leaving directory `/tmp/android-build-scripts/lxc/src' make: *** [all-recursive] Error 1
It looks like pthread_atfork doesn't exist in bionic. On Wed, Jan 01, 2014 at 01:52:33PM -0600, Serge Hallyn wrote: > Quoting Andrey Mazo ([email protected]): > > Signed-off-by: Andrey Mazo <[email protected]> > > --- > > src/lxc/Makefile.am | 3 ++- > > src/lxc/attach.c | 2 -- > > src/lxc/bdev.c | 13 ------------- > > src/lxc/lxccontainer.c | 4 ---- > > src/lxc/lxclock.c | 23 ++++++++++++++++++++++- > > src/lxc/monitor.c | 1 - > > 6 files changed, 24 insertions(+), 22 deletions(-) > > > > diff --git a/src/lxc/Makefile.am b/src/lxc/Makefile.am > > index 74b38e2..f18d6e1 100644 > > --- a/src/lxc/Makefile.am > > +++ b/src/lxc/Makefile.am > > @@ -135,9 +135,10 @@ AM_CFLAGS += -DHAVE_SECCOMP > > liblxc_so_SOURCES += seccomp.c > > endif > > > > -liblxc_so_CFLAGS = -fPIC -DPIC $(AM_CFLAGS) > > +liblxc_so_CFLAGS = -fPIC -DPIC $(AM_CFLAGS) -pthread > > > > liblxc_so_LDFLAGS = \ > > + -pthread \ > > -shared \ > > -Wl,-soname,liblxc.so.$(firstword $(subst ., ,$(VERSION))) > > > > diff --git a/src/lxc/attach.c b/src/lxc/attach.c > > index 6749617..252e172 100644 > > --- a/src/lxc/attach.c > > +++ b/src/lxc/attach.c > > @@ -497,7 +497,6 @@ char *lxc_attach_getpwshell(uid_t uid) > > NULL > > }; > > > > - process_unlock(); // we're no longer sharing > > close(pipes[0]); > > > > /* we want to capture stdout */ > > @@ -790,7 +789,6 @@ int lxc_attach(const char* name, const char* lxcpath, > > lxc_attach_exec_t exec_fun > > return -1; > > } > > > > - process_unlock(); // we're no longer sharing > > /* first subprocess begins here, we close the socket that is for the > > * initial thread > > */ > > diff --git a/src/lxc/bdev.c b/src/lxc/bdev.c > > index b737eff..ec0d0ee 100644 > > --- a/src/lxc/bdev.c > > +++ b/src/lxc/bdev.c > > @@ -72,7 +72,6 @@ static int do_rsync(const char *src, const char *dest) > > if (pid > 0) > > return wait_for_pid(pid); > > > > - process_unlock(); // we're no longer sharing > > l = strlen(src) + 2; > > s = malloc(l); > > if (!s) > > @@ -197,7 +196,6 @@ static int do_mkfs(const char *path, const char *fstype) > > if (pid > 0) > > return wait_for_pid(pid); > > > > - process_unlock(); // we're no longer sharing > > // If the file is not a block device, we don't want mkfs to ask > > // us about whether to proceed. > > close(0); > > @@ -282,7 +280,6 @@ static int detect_fs(struct bdev *bdev, char *type, int > > len) > > return ret; > > } > > > > - process_unlock(); // we're no longer sharing > > if (unshare(CLONE_NEWNS) < 0) > > exit(1); > > > > @@ -574,7 +571,6 @@ static int zfs_clone(const char *opath, const char > > *npath, const char *oname, > > if (!pid) { > > char dev[MAXPATHLEN]; > > > > - process_unlock(); // we're no longer sharing > > ret = snprintf(dev, MAXPATHLEN, "%s/%s", zfsroot, > > nname); > > if (ret < 0 || ret >= MAXPATHLEN) > > exit(1); > > @@ -598,7 +594,6 @@ static int zfs_clone(const char *opath, const char > > *npath, const char *oname, > > if ((pid = fork()) < 0) > > return -1; > > if (!pid) { > > - process_unlock(); // we're no longer sharing > > execlp("zfs", "zfs", "destroy", path1, NULL); > > exit(1); > > } > > @@ -609,7 +604,6 @@ static int zfs_clone(const char *opath, const char > > *npath, const char *oname, > > if ((pid = fork()) < 0) > > return -1; > > if (!pid) { > > - process_unlock(); // we're no longer sharing > > execlp("zfs", "zfs", "snapshot", path1, NULL); > > exit(1); > > } > > @@ -620,7 +614,6 @@ static int zfs_clone(const char *opath, const char > > *npath, const char *oname, > > if ((pid = fork()) < 0) > > return -1; > > if (!pid) { > > - process_unlock(); // we're no longer sharing > > execlp("zfs", "zfs", "clone", option, path1, path2, > > NULL); > > exit(1); > > } > > @@ -671,7 +664,6 @@ static int zfs_destroy(struct bdev *orig) > > if (pid) > > return wait_for_pid(pid); > > > > - process_unlock(); // we're no longer sharing > > if (!zfs_list_entry(orig->src, output, MAXPATHLEN)) { > > ERROR("Error: zfs entry for %s not found", orig->src); > > return -1; > > @@ -716,7 +708,6 @@ static int zfs_create(struct bdev *bdev, const char > > *dest, const char *n, > > if (pid) > > return wait_for_pid(pid); > > > > - process_unlock(); // we're no longer sharing > > char dev[MAXPATHLEN]; > > ret = snprintf(dev, MAXPATHLEN, "%s/%s", zfsroot, n); > > if (ret < 0 || ret >= MAXPATHLEN) > > @@ -861,7 +852,6 @@ static int do_lvm_create(const char *path, unsigned > > long size, const char *thinp > > if (pid > 0) > > return wait_for_pid(pid); > > > > - process_unlock(); // we're no longer sharing > > // lvcreate default size is in M, not bytes. > > ret = snprintf(sz, 24, "%lu", size/1000000); > > if (ret < 0 || ret >= 24) > > @@ -921,7 +911,6 @@ static int lvm_snapshot(const char *orig, const char > > *path, unsigned long size) > > if (pid > 0) > > return wait_for_pid(pid); > > > > - process_unlock(); // we're no longer sharing > > // lvcreate default size is in M, not bytes. > > ret = snprintf(sz, 24, "%lu", size/1000000); > > if (ret < 0 || ret >= 24) > > @@ -1055,7 +1044,6 @@ static int lvm_destroy(struct bdev *orig) > > if ((pid = fork()) < 0) > > return -1; > > if (!pid) { > > - process_unlock(); // we're no longer sharing > > execlp("lvremove", "lvremove", "-f", orig->src, NULL); > > exit(1); > > } > > @@ -2091,7 +2079,6 @@ struct bdev *bdev_copy(const char *src, const char > > *oldname, const char *cname, > > return new; > > } > > > > - process_unlock(); // we're no longer sharing > > if (unshare(CLONE_NEWNS) < 0) { > > SYSERROR("unshare CLONE_NEWNS"); > > exit(1); > > diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c > > index e1d004f..cf71f28 100644 > > --- a/src/lxc/lxccontainer.c > > +++ b/src/lxc/lxccontainer.c > > @@ -599,7 +599,6 @@ static bool lxcapi_start(struct lxc_container *c, int > > useinit, char * const argv > > if (pid != 0) > > return wait_on_daemonized_start(c, pid); > > > > - process_unlock(); // we're no longer sharing > > /* second fork to be reparented by init */ > > pid = fork(); > > if (pid < 0) { > > @@ -837,7 +836,6 @@ static bool create_run_template(struct lxc_container > > *c, char *tpath, bool quiet > > char **newargv; > > struct lxc_conf *conf = c->lxc_conf; > > > > - process_unlock(); // we're no longer sharing > > if (quiet) { > > close(0); > > close(1); > > @@ -1234,7 +1232,6 @@ static bool lxcapi_create(struct lxc_container *c, > > const char *t, > > if (pid == 0) { // child > > struct bdev *bdev = NULL; > > > > - process_unlock(); // we're no longer sharing > > if (!(bdev = do_bdev_create(c, bdevtype, specs))) { > > ERROR("Error creating backing store type %s for %s", > > bdevtype ? bdevtype : "(none)", c->name); > > @@ -2326,7 +2323,6 @@ static int clone_update_rootfs(struct lxc_container > > *c0, > > if (pid > 0) > > return wait_for_pid(pid); > > > > - process_unlock(); // we're no longer sharing > > bdev = bdev_init(c->lxc_conf->rootfs.path, c->lxc_conf->rootfs.mount, > > NULL); > > if (!bdev) > > exit(1); > > diff --git a/src/lxc/lxclock.c b/src/lxc/lxclock.c > > index 28691b0..36c4bd3 100644 > > --- a/src/lxc/lxclock.c > > +++ b/src/lxc/lxclock.c > > @@ -87,7 +87,7 @@ void unlock_mutex(pthread_mutex_t *l) > > int ret; > > > > if ((ret = pthread_mutex_unlock(l)) != 0) { > > - fprintf(stderr, "pthread_mutex_lock returned:%d %s", ret, > > strerror(ret)); > > + fprintf(stderr, "pthread_mutex_unlock returned:%d %s", ret, > > strerror(ret)); > > dump_stacktrace(); > > exit(1); > > } > > @@ -315,6 +315,21 @@ void process_unlock(void) > > unlock_mutex(&thread_mutex); > > } > > > > +/* One thread can do fork() while another one is holding a mutex. > > + * There is only one thread in child just after the fork(), so noone will > > ever release that mutex. > > + * We setup a "child" fork handler to unlock the mutex just after the > > fork(). > > + * For several mutex types, unlocking an unlocked mutex can lead to > > undefined behavior. > > Thanks for pointing this out. I have to say I don't like that we have > to lock this... though it should be safe with our current usage (and > I see Caglar has verified it :) > > An alternative would be to have a unlock_if_locked() for each lock, > and do pthread_atfork(NULL, NULL, process_unlock_if_locked()). > There would be no risk of a race since the child is single-threaded. > > But this is ok for now. > > Acked-by: Serge E. Hallyn <[email protected]> > > (Note I'm acking, but not applying until tomorrow) > > > + * One way to deal with it is to setup "prepare" fork handler > > + * to lock the mutex before fork() and both "parent" and "child" fork > > handlers > > + * to unlock the mutex. > > + * This forbids doing fork() while explicitly holding the lock. > > + */ > > +__attribute__((constructor)) > > +static void process_lock_setup_atfork(void) > > +{ > > + pthread_atfork(process_lock, process_unlock, process_unlock); > > +} > > + > > /* Protects static const values inside the lxc_global_config_value funtion > > */ > > void static_lock(void) > > { > > @@ -326,6 +341,12 @@ void static_unlock(void) > > unlock_mutex(&static_mutex); > > } > > > > +__attribute__((constructor)) > > +static void static_lock_setup_atfork(void) > > +{ > > + pthread_atfork(static_lock, static_unlock, static_unlock); > > +} > > + > > int container_mem_lock(struct lxc_container *c) > > { > > return lxclock(c->privlock, 0); > > diff --git a/src/lxc/monitor.c b/src/lxc/monitor.c > > index 7e0a713..0b6a065 100644 > > --- a/src/lxc/monitor.c > > +++ b/src/lxc/monitor.c > > @@ -300,7 +300,6 @@ int lxc_monitord_spawn(const char *lxcpath) > > return 0; > > } > > > > - process_unlock(); // we're no longer sharing > > if (pipe(pipefd) < 0) { > > SYSERROR("failed to create pipe"); > > exit(EXIT_FAILURE); > > -- > > 1.8.4.5 > > > > _______________________________________________ > > lxc-devel mailing list > > [email protected] > > http://lists.linuxcontainers.org/listinfo/lxc-devel > _______________________________________________ > 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
