From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:46893) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ra7lT-0004Ws-Qn for qemu-devel@nongnu.org; Mon, 12 Dec 2011 10:26:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ra7lS-0002aI-Go for qemu-devel@nongnu.org; Mon, 12 Dec 2011 10:26:03 -0500 Received: from e4.ny.us.ibm.com ([32.97.182.144]:40211) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ra7lS-0002Zv-EA for qemu-devel@nongnu.org; Mon, 12 Dec 2011 10:26:02 -0500 Received: from /spool/local by e4.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 12 Dec 2011 10:25:59 -0500 Received: from d01av04.pok.ibm.com (d01av04.pok.ibm.com [9.56.224.64]) by d01relay05.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id pBCFMKaa240870 for ; Mon, 12 Dec 2011 10:22:20 -0500 Received: from d01av04.pok.ibm.com (loopback [127.0.0.1]) by d01av04.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id pBCFME6d031883 for ; Mon, 12 Dec 2011 10:22:19 -0500 From: "Aneesh Kumar K.V" In-Reply-To: <20111212120833.GA22654@stefanha-thinkpad.localdomain> References: <1323101930-27163-1-git-send-email-mohan@in.ibm.com> <1323101930-27163-5-git-send-email-mohan@in.ibm.com> <20111208183114.GC20998@stefanha-thinkpad.localdomain> <201112092212.20343.mohan@in.ibm.com> <20111212120833.GA22654@stefanha-thinkpad.localdomain> Date: Mon, 12 Dec 2011 20:51:58 +0530 Message-ID: <87ehw9hl2h.fsf@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [PATCH V4 04/13] hw/9pfs: File system helper process for qemu 9p proxy FS List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi , "M. Mohan Kumar" Cc: "M. Mohan Kumar" , qemu-devel@nongnu.org On Mon, 12 Dec 2011 12:08:33 +0000, Stefan Hajnoczi wrote: > On Fri, Dec 09, 2011 at 10:12:17PM +0530, M. Mohan Kumar wrote: > > On Friday, December 09, 2011 12:01:14 AM Stefan Hajnoczi wrote: > > > On Mon, Dec 05, 2011 at 09:48:41PM +0530, M. Mohan Kumar wrote: > > > > +static int read_request(int sockfd, struct iovec *iovec, ProxyHeader > > > > *header) +{ > > > > + int retval; > > > > + > > > > + /* > > > > + * read the request header. > > > > + */ > > > > + iovec->iov_len = 0; > > > > + retval = socket_read(sockfd, iovec->iov_base, PROXY_HDR_SZ); > > > > + if (retval < 0) { > > > > + return retval; > > > > + } > > > > + iovec->iov_len = PROXY_HDR_SZ; > > > > + retval = proxy_unmarshal(iovec, 0, "dd", &header->type, > > > > &header->size); + if (retval < 0) { > > > > + return retval; > > > > + } > > > > + /* > > > > + * We can't process message.size > PROXY_MAX_IO_SZ, read the > > > > complete + * message from the socket and ignore it. This ensures > > > > that + * we can correctly handle the next request. We also return + > > > > * ENOBUFS as error to indicate we ran out of buffer space. + */ > > > > + if (header->size > PROXY_MAX_IO_SZ) { > > > > + int count, size; > > > > + size = header->size; > > > > + while (size > 0) { > > > > + count = MIN(PROXY_MAX_IO_SZ, size); > > > > + count = socket_read(sockfd, iovec->iov_base + PROXY_HDR_SZ, > > > > count); + if (count < 0) { > > > > + return count; > > > > + } > > > > + size -= count; > > > > + } > > > > > > I'm not sure recovery attempts are worthwhile here. The client is > > > buggy, perhaps just refuse further work. > > > > But whats the issue in trying to recover in this case? > > This recovery procedure is not robust because it does not always work. > In fact it only works in the case where the header->size field was > out-of-range but accurate. That's not a likely case since the QEMU-side > code that you are writing should handle this. > > If the nature of the invalid request is different, either a broken or > malicious client which does not send a valid header->size then we're > stuck in this special-case recovery trying to gobble bytes and we never > log an error. > > A real recovery would be something like disconnecting and > re-establishing the connection between QEMU and the helper. This would > allow us to get back to a clean state in all cases. > Since we are not having any state in the proxy helper, returning ENOBUFS should be similar to the above right ? One of the reason to try to recover as much as possible, is to make sure the guest can umount the file system properly. That is if we hit these error condition due to a bug in proxy FS driver is qemu, we want to make sure we return some valid error, which will atleast enable the guest/client to do an umount. -aneesh