From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=33838 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PxES1-0006ek-77 for qemu-devel@nongnu.org; Wed, 09 Mar 2011 03:09:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PxERt-0001aV-BJ for qemu-devel@nongnu.org; Wed, 09 Mar 2011 03:08:50 -0500 Received: from e23smtp09.au.ibm.com ([202.81.31.142]:33450) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PxERs-0001Xe-Q9 for qemu-devel@nongnu.org; Wed, 09 Mar 2011 03:08:49 -0500 Received: from d23relay03.au.ibm.com (d23relay03.au.ibm.com [202.81.31.245]) by e23smtp09.au.ibm.com (8.14.4/8.13.1) with ESMTP id p2988aHx015336 for ; Wed, 9 Mar 2011 19:08:36 +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 p2988Z2b1458394 for ; Wed, 9 Mar 2011 19:08:36 +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 p2988Zo4013803 for ; Wed, 9 Mar 2011 19:08:35 +1100 From: "M. Mohan Kumar" Date: Wed, 9 Mar 2011 13:38:31 +0530 References: <1299230756-1644-1-git-send-email-mohan@in.ibm.com> <1299230756-1644-8-git-send-email-mohan@in.ibm.com> In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201103091338.32071.mohan@in.ibm.com> Subject: [Qemu-devel] Re: [V7 PATCH 7/9] virtio-9p: Support for creating special files List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: qemu-devel@nongnu.org On Friday 04 March 2011 4:36:35 pm Stefan Hajnoczi wrote: > On Fri, Mar 4, 2011 at 9:25 AM, M. Mohan Kumar wrote: > > +static int chroot_do_create_special(V9fsFileObjectRequest *request) > > +{ > > + int cur_uid, cur_gid; > > + int retval = -1; > > + > > + cur_uid = geteuid(); > > + cur_gid = getegid(); > > + > > + if (setfsuid(request->data.uid) < 0) { > > + return -errno; > > + } > > + if (setfsgid(request->data.gid) < 0) { > > + retval = -errno; > > + goto unset_uid; > > + } > > + > > + switch (request->data.type) { > > + case T_MKDIR: > > + retval = mkdir(request->path.path, request->data.mode); > > + break; > > + case T_SYMLINK: > > + retval = symlink(request->path.old_path, request->path.path); > > + break; > > + case T_LINK: > > + retval = link(request->path.old_path, request->path.path); > > + break; > > + default: > > + retval = mknod(request->path.path, request->data.mode, > > + request->data.dev); > > + break; > > + } > > + > > + if (retval < 0) { > > + retval = -errno; > > + } > > + setfsgid(cur_gid); > > +unset_uid: > > + setfsuid(cur_uid); > > + return retval; > > +} > > It would be nice to take this one step further and move file create > and open here too. The prototype we need is: > > static int chroot_handle_request(V9fsFileObjectRequest *request, int *fd) > { > *fd = -1; > > It returns 0 on success or -errno and *fd >= 0 if a file descriptor > was opened and -1 otherwise. > > This function becomes the main request processing function called from > v9fs_chroot() and the switch statement there can be eliminated. > We don't need setfsgid, setfsuid for normal open. Also I think having separate function based on the functionality helps better code readability. > Sending the response back to QEMU then gets a cleaned up prototype: > chroot_sendfd(int chroot_sock, int result, int fd) where result is 0 > on success or -errno and fd >= 0 if present or -1 if not. > > > +int v9fs_create_special(FsContext *fs_ctx, V9fsFileObjectRequest > > *request) +{ > > + int retval, sock_error; > > + qemu_mutex_lock(&fs_ctx->chroot_mutex); > > + if (fs_ctx->chroot_ioerror) { > > + retval = -EIO; > > + goto unlock; > > + } > > + if (v9fs_write_request(fs_ctx->chroot_socket, request) < 0) { > > + fs_ctx->chroot_ioerror = 1; > > + retval = -EIO; > > + goto unlock; > > + } > > + retval = v9fs_receivefd(fs_ctx->chroot_socket, &sock_error); > > + if (retval < 0 && sock_error) { > > + fs_ctx->chroot_ioerror = 1; > > + } > > +unlock: > > + qemu_mutex_unlock(&fs_ctx->chroot_mutex); > > + return retval; > > +} > > This function is a duplicate of v9fs_request(). Can't there be just > one function? > Yeah, I will make it as a single function. ---- M. Mohan Kumar