From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:56567) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SwmUB-00059J-2F for qemu-devel@nongnu.org; Wed, 01 Aug 2012 23:54:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SwmU9-0002yq-Tu for qemu-devel@nongnu.org; Wed, 01 Aug 2012 23:54:06 -0400 Received: from e23smtp02.au.ibm.com ([202.81.31.144]:46439) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SwmU8-0002yQ-VP for qemu-devel@nongnu.org; Wed, 01 Aug 2012 23:54:05 -0400 Received: from /spool/local by e23smtp02.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 2 Aug 2012 13:53:32 +1000 Received: from d23av01.au.ibm.com (d23av01.au.ibm.com [9.190.234.96]) by d23relay05.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q723jS0V26476604 for ; Thu, 2 Aug 2012 13:45:29 +1000 Received: from d23av01.au.ibm.com (loopback [127.0.0.1]) by d23av01.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q723rs7j017596 for ; Thu, 2 Aug 2012 13:53:55 +1000 Date: Thu, 2 Aug 2012 09:25:26 +0530 From: Bharata B Rao Message-ID: <20120802035526.GG21697@in.ibm.com> References: <20120801141414.GD21697@in.ibm.com> <20120801141625.GF21697@in.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH v4 2/2] block: Support GlusterFS as a QEMU block backend Reply-To: bharata@linux.vnet.ibm.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: Amar Tumballi , Vijay Bellur , Anand Avati , qemu-devel@nongnu.org, Stefan Hajnoczi On Wed, Aug 01, 2012 at 06:35:22PM +0000, Blue Swirl wrote: > > + > > + if (!transport) { > > + uri->transport = strdup("socket"); > > g_strdup Sorry about that, pitfalls of developing the parsing code out of line :( > > +static int qemu_gluster_parseuri(GlusterURI *uri, const char *filename) > > +{ > > + char *token, *saveptr; > > + char *p, *r; > > + int ret = -EINVAL; > > + > > + p = r = g_strdup(filename); > > Why? - Are you asking why use 2 variables ? I need them because I loose p and need r to free the string. - Or are you asking why strdup ? That's because filename is const char * and I need to modify the filename when parsing. > > +static void gluster_finish_aiocb(struct glfs_fd *fd, ssize_t ret, void *arg) > > +{ > > + GlusterAIOCB *acb = (GlusterAIOCB *)arg; > > + BDRVGlusterState *s = acb->common.bs->opaque; > > + > > + acb->ret = ret; > > + if (qemu_gluster_send_pipe(s, acb) < 0) { > > + error_report("Could not complete read/write/flush from gluster"); > > + abort(); > > Aborting is a bit drastic, it would be nice to save and exit gracefully. I am not sure if there is an easy way to recover sanely and exit from this kind of error. Here the non-QEMU thread (gluster thread) failed to notify the QEMU thread on the read side of the pipe about the IO completion. So essentially bdrv_read or bdrv_write will never complete if this error happens. Do you have any suggestions on how to exit gracefully here ? > > +static QEMUOptionParameter qemu_gluster_create_options[] = { > > 'const'? Hmm no precedence of const usage for identical scenario in other block drivers in QEMU. > > > + { > > + .name = BLOCK_OPT_SIZE, > > + .type = OPT_SIZE, > > + .help = "Virtual disk size" > > + }, > > + { NULL } > > +}; > > + > > +static BlockDriver bdrv_gluster = { > > 'const'? Again dodn't see the precedence for this. Thanks for your review. Regards, Bharata.