From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=58604 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PmqJZ-0001Qm-28 for qemu-devel@nongnu.org; Tue, 08 Feb 2011 11:21:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PmqJX-0006g8-3o for qemu-devel@nongnu.org; Tue, 08 Feb 2011 11:21:16 -0500 Received: from e28smtp02.in.ibm.com ([122.248.162.2]:38340) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PmqJW-0006ey-F4 for qemu-devel@nongnu.org; Tue, 08 Feb 2011 11:21:15 -0500 Received: from d28relay03.in.ibm.com (d28relay03.in.ibm.com [9.184.220.60]) by e28smtp02.in.ibm.com (8.14.4/8.13.1) with ESMTP id p18GL9Ew021485 for ; Tue, 8 Feb 2011 21:51:09 +0530 Received: from d28av02.in.ibm.com (d28av02.in.ibm.com [9.184.220.64]) by d28relay03.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p18GL8Xp3797194 for ; Tue, 8 Feb 2011 21:51:08 +0530 Received: from d28av02.in.ibm.com (loopback [127.0.0.1]) by d28av02.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p18GL8oh023392 for ; Wed, 9 Feb 2011 03:21:08 +1100 From: "M. Mohan Kumar" Date: Tue, 8 Feb 2011 21:51:07 +0530 References: <1296537693-16406-1-git-send-email-mohan@in.ibm.com> <1296537992-16687-1-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: <201102082151.07677.mohan@in.ibm.com> Subject: [Qemu-devel] Re: [V4 PATCH 3/8] Add client side interfaces for chroot environment 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 Wednesday 02 February 2011 3:24:16 pm Stefan Hajnoczi wrote: > On Tue, Feb 1, 2011 at 5:26 AM, M. Mohan Kumar wrote: > > +/* Receive file descriptor and error status from chroot process */ > > +static int v9fs_receivefd(int sockfd, int *error) > > The return value and int *error overlap in functionality. Would it be > possible to have only one mechanism for returning errors? > v9fs_receivefd function returns 'fd' on success and returns EIO (and error as EIO) if there was a socket error and -1 on other errors. We need to return -EIO and error as EIO to differentiate between generic EIO error and socket failure. > *error = 0 is never done so a caller that passes an uninitialized > local variable gets back junk when the function succeeds. It would be > safer to clear it at the start of this function. I will update the patch. > > Inconsistent use of errno constants and -1: > return -EIO; > return -1; /* == -EPERM, probably not what you wanted */ -1 denotes failure > > How about getting rid of int *error and returning the -errno? If > if_error is set then return -fd_info.error. Do you mean return fd on success and -errno on failure? In this case how to differentiate between actual EIO and EIO because of socket read/write failure? > > > +/* > > + * V9fsFileObjectRequest is written into the socket by QEMU process. > > + * Then this request is read by chroot process using read_request > > function + */ > > +static int v9fs_write_request(int sockfd, V9fsFileObjectRequest > > *request) +{ > > + int retval, length; > > + char *buff, *buffp; > > + > > + length = sizeof(request->data) + request->data.path_len + > > + request->data.oldpath_len; > > + > > + buff = qemu_malloc(length); > > + buffp = buff; > > + memcpy(buffp, &request->data, sizeof(request->data)); > > + buffp += sizeof(request->data); > > + memcpy(buffp, request->path.path, request->data.path_len); > > + buffp += request->data.path_len; > > + memcpy(buffp, request->path.old_path, request->data.oldpath_len); > > + > > + retval = qemu_write_full(sockfd, buff, length); > > qemu_free(buff); > > Also, weren't you doing the malloc() + single write() to avoid > interleaved write()? Is that still necessary, I thought a mutex was > introduced? It's probably worth adding a comment to explain why > you're doing the malloc + write. This is used to avoid multiple write system calls. Probably I can update with comments. ---- M. Mohan Kumar