From patchwork Tue Feb 8 12:17:42 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mohan Kumar M X-Patchwork-Id: 82304 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 6A92FB70CD for ; Tue, 8 Feb 2011 23:20:26 +1100 (EST) Received: from localhost ([127.0.0.1]:46847 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PmmYR-0005Q8-7a for incoming@patchwork.ozlabs.org; Tue, 08 Feb 2011 07:20:23 -0500 Received: from [140.186.70.92] (port=50403 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PmmW1-0004Yk-Mp for qemu-devel@nongnu.org; Tue, 08 Feb 2011 07:17:55 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PmmVz-0001eC-W4 for qemu-devel@nongnu.org; Tue, 08 Feb 2011 07:17:53 -0500 Received: from e23smtp07.au.ibm.com ([202.81.31.140]:45720) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PmmVz-0001d1-7Z for qemu-devel@nongnu.org; Tue, 08 Feb 2011 07:17:51 -0500 Received: from d23relay03.au.ibm.com (d23relay03.au.ibm.com [202.81.31.245]) by e23smtp07.au.ibm.com (8.14.4/8.13.1) with ESMTP id p18CHkiR015449 for ; Tue, 8 Feb 2011 23:17:46 +1100 Received: from d23av04.au.ibm.com (d23av04.au.ibm.com [9.190.235.139]) by d23relay03.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p18CHkoT1859632 for ; Tue, 8 Feb 2011 23:17:46 +1100 Received: from d23av04.au.ibm.com (loopback [127.0.0.1]) by d23av04.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p18CHjb9026311 for ; Tue, 8 Feb 2011 23:17:46 +1100 Received: from explorer.ibm.com ([9.124.35.46]) by d23av04.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with ESMTP id p18CHihs026301 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 8 Feb 2011 23:17:45 +1100 From: "M. Mohan Kumar" To: "Daniel P. Berrange" Subject: Re: [Qemu-devel] [V4 PATCH 2/8] Provide chroot environment server side interfaces Date: Tue, 8 Feb 2011 17:47:42 +0530 User-Agent: KMail/1.13.5 (Linux/2.6.35.10-74.fc14.i686.PAE; KDE/4.5.4; i686; ; ) References: <1296537693-16406-1-git-send-email-mohan@in.ibm.com> <1296537939-16649-1-git-send-email-mohan@in.ibm.com> <20110201103222.GC1725@redhat.com> In-Reply-To: <20110201103222.GC1725@redhat.com> MIME-Version: 1.0 Message-Id: <201102081747.42543.mohan@in.ibm.com> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6, seldom 2.4 (older, 4) X-Received-From: 202.81.31.140 Cc: blauwirbel@gmail.com, stefanha@gmail.com, qemu-devel@nongnu.org, Anthony Liguori X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Hi, Is it okay to fork the chroot process in the fsdev parameter parsing time? Can we ask other initialization routines to not create threads till chroot process is forked? Following code snippet forks the chroot process during fsdev parameter parsing to avoid the problems associated with forking in multi thread environment. return 0; On Tuesday 01 February 2011 4:02:22 pm Daniel P. Berrange wrote: > On Tue, Feb 01, 2011 at 10:55:39AM +0530, M. Mohan Kumar wrote: > > Implement chroot server side interfaces like sending the file > > descriptor to qemu process, reading the object request from socket etc. > > Also add chroot main function and other helper routines. > > > > Signed-off-by: M. Mohan Kumar > > --- > > > > Makefile.objs | 1 + > > hw/9pfs/virtio-9p-chroot.c | 212 > > ++++++++++++++++++++++++++++++++++++++++++++ hw/9pfs/virtio-9p-chroot.h > > | 41 +++++++++ > > hw/9pfs/virtio-9p.c | 33 +++++++ > > hw/file-op-9p.h | 3 + > > 5 files changed, 290 insertions(+), 0 deletions(-) > > create mode 100644 hw/9pfs/virtio-9p-chroot.c > > create mode 100644 hw/9pfs/virtio-9p-chroot.h > > > > diff --git a/Makefile.objs b/Makefile.objs > > index bc0344c..3007b6d 100644 > > --- a/Makefile.objs > > +++ b/Makefile.objs > > +/* > > + * Fork a process and chroot into the share path. Communication > > + * between qemu process and chroot process happens via socket > > + * All file descriptors (including stdout and stderr) are closed > > + * except one socket descriptor (which is used for communicating > > + * between qemu process and chroot process) > > + */ > > +int v9fs_chroot(FsContext *fs_ctx) > > +{ > > + int fd_pair[2], chroot_sock, error; > > + V9fsFileObjectRequest request; > > + pid_t pid; > > + uint64_t code; > > + FdInfo fd_info; > > + > > + if (socketpair(PF_UNIX, SOCK_STREAM, 0, fd_pair) < 0) { > > + error_report("socketpair %s", strerror(errno)); > > + return -1; > > + } > > + > > + pid = fork(); > > + if (pid < 0) { > > + error_report("fork %s", strerror(errno)); > > + return -1; > > + } > > + if (pid != 0) { > > + fs_ctx->chroot_socket = fd_pair[0]; > > + close(fd_pair[1]); > > + return 0; > > + } > > + > > + close(fd_pair[0]); > > + chroot_sock = fd_pair[1]; > > + if (chroot(fs_ctx->fs_root) < 0) { > > + code = CHROOT_ERROR << 32 | errno; > > + error = qemu_write_full(chroot_sock, &code, sizeof(code)); > > + _exit(1); > > + } > > + > > + error = chroot_daemonize(chroot_sock); > > + if (error) { > > + code = SETSID_ERROR << 32 | error; > > + error = qemu_write_full(chroot_sock, &code, sizeof(code)); > > + _exit(1); > > + } > > + > > + /* > > + * Write 0 to chroot socket to indicate chroot process creation is > > + * successful > > + */ > > + code = 0; > > + if (qemu_write_full(chroot_sock, &code, sizeof(code)) > > + != sizeof(code)) { > > + _exit(1); > > + } > > + /* get the request from the socket */ > > + while (1) { > > + memset(&fd_info, 0, sizeof(fd_info)); > > + if (chroot_read_request(chroot_sock, &request) == EIO) { > > + fd_info.fi_fd = 0; > > + fd_info.fi_error = EIO; > > + fd_info.fi_flags = FI_SOCKERR; > > + chroot_sendfd(chroot_sock, &fd_info); > > + continue; > > + } > > + qemu_free((void *)request.path.path); > > + if (request.data.oldpath_len) { > > + qemu_free((void *)request.path.old_path); > > + } > > + } > > +} > > There is a subtle problem with using fork() in a multi-threaded > program that I was recently made aware of in libvirt. In short > if you have a multi-threaded program that calls fork(), then > the child process must only use POSIX functions that are > declared 'async signal safe', until the child calls exec() or > exit(). In particular any malloc()/free() related functions > are *not* async signal safe. > > http://pubs.opengroup.org/onlinepubs/9699919799/functions/fork.html > > "If a multi-threaded process calls fork(), the new process shall contain > a replica of the calling thread and its entire address space, possibly > including the states of mutexes and other resources. Consequently, to > avoid errors, the child process may only execute async-signal-safe > operations until such time as one of the exec functions is called." > > One example problem scenario. Thread 1 is currently doing a > malloc() and the malloc() impl is holding a mutex. Thread 2 > now does a fork(), and in the child process calls malloc(). > The child process will deadlock / hang forever because there > is nothing which will ever release the malloc() mutex that > was originally held by Thread 1. See also this thread which > brought the problem to my attention: > > http://lists.gnu.org/archive/html/coreutils/2011-01/msg00085.html > > Regards, > Daniel ---- M. Mohan Kumar diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c index 0b33290..2dad688 100644 --- a/fsdev/qemu-fsdev.c +++ b/fsdev/qemu-fsdev.c @@ -17,6 +17,8 @@ #include "osdep.h" #include "qemu-common.h" #include "qemu-config.h" +#include "qemu_socket.h" +#include "hw/9pfs/virtio-9p-chroot.h" static QTAILQ_HEAD(FsTypeEntry_head, FsTypeListEntry) fstype_entries = QTAILQ_HEAD_INITIALIZER(fstype_entries); @@ -72,6 +74,34 @@ int qemu_fsdev_add(QemuOpts *opts) fsle->fse.security_model = qemu_strdup(sec_model); fsle->fse.ops = FsTypes[i].ops; + if (!strcmp(fsle->fse.security_model, "passthrough")) { + uint64_t code; + if (v9fs_chroot(&fsle->fse) < 0) { + exit(1); + } + + /* + * Chroot process sends 0 to indicate chroot process creation is + * successful + */ + if (read(fsle->fse.chroot_socket, &code, sizeof(code)) != sizeof(code)) { + fprintf(stderr, "chroot process creation failed"); + exit(1); + } + if (code != 0) { + switch (code >> 32) { + case CHROOT_ERROR: + fprintf(stderr, "chroot system call failed: %s", + strerror(code & 0xFFFFFFFF)); + break; + case SETSID_ERROR: + fprintf(stderr, "setsid failed: %s", strerror(code & 0xFFFFFFFF)); + break; + } + exit(1); + } + } + QTAILQ_INSERT_TAIL(&fstype_entries, fsle, next);