From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:35002) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TH9Qw-00031n-98 for qemu-devel@nongnu.org; Thu, 27 Sep 2012 04:27:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TH9Qs-0007Q8-6G for qemu-devel@nongnu.org; Thu, 27 Sep 2012 04:26:58 -0400 Received: from e23smtp05.au.ibm.com ([202.81.31.147]:48210) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TH9Qr-0007Q2-KG for qemu-devel@nongnu.org; Thu, 27 Sep 2012 04:26:54 -0400 Received: from /spool/local by e23smtp05.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 27 Sep 2012 18:25:33 +1000 Received: from d23av01.au.ibm.com (d23av01.au.ibm.com [9.190.234.96]) by d23relay04.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q8R8HMrP17432718 for ; Thu, 27 Sep 2012 18:17:23 +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 q8R8Ql8P013387 for ; Thu, 27 Sep 2012 18:26:47 +1000 Date: Thu, 27 Sep 2012 13:58:46 +0530 From: Bharata B Rao Message-ID: <20120927082846.GC18285@in.ibm.com> References: <20120924091008.GJ18470@in.ibm.com> <20120924091340.GN18470@in.ibm.com> <5062D24F.7030304@redhat.com> <20120926161132.GA32195@in.ibm.com> <50632F6A.5020202@redhat.com> <20120927064132.GB18285@in.ibm.com> <5063FE06.3070304@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5063FE06.3070304@redhat.com> Subject: Re: [Qemu-devel] [PATCH v9 4/4] 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: Paolo Bonzini Cc: Kevin Wolf , Anthony Liguori , Anand Avati , Vijay Bellur , Stefan Hajnoczi , Harsh Bora , Amar Tumballi , qemu-devel@nongnu.org, "Richard W.M. Jones" , Blue Swirl , Avi Kivity , Daniel Veillard On Thu, Sep 27, 2012 at 09:19:34AM +0200, Paolo Bonzini wrote: > > > > For gluster://server/volname////path/to/image, the image is extracted as > > "//path/to/image". > > Should there be three /'s here? I assume it's just a typo. Yes it was a typo. > > I'm concerned that there is no documentation of what saveptr actually > points to. In many cases the strtok specification doesn't leave much > free room, but in the case you're testing here: > > >>> /* image */ > >>> if (!*saveptr) { > >>> return -EINVAL; > >>> } > > strtok_r may just as well leave saveptr = NULL for example. Haven't seen that, but yes can't depend on that I suppose. > > >>> gconf->image = g_strdup(saveptr); > >>> > >> > >> I would avoid strtok_r completely: > >> > >> char *p = path + strcpsn(path, "/"); > >> if (*p == '\0') { > >> return -EINVAL; > >> } > >> gconf->volname = g_strndup(path, p - path); > >> > >> p += strspn(p, "/"); > >> if (*p == '\0') { > >> return -EINVAL; > >> } > >> gconf->image = g_strdup(p); > > > > This isn't working because for a URI like > > gluster://server/volname/path/to/image, uri_parse() will give > > "/volname/path/to/image" in uri->path. I would have expected to see > > uri->path as "volname/path/to/image" (without 1st slash). > > Ok, that's easy enough to fix with an extra strspn, > > char *p = path + strpsn(path, "/"); > p += strcspn(p, "/"); Ok, this is how its looking finally... char *p, *q; p = q = path + strspn(path, "/"); p += strcspn(p, "/"); if (*p == '\0') { return -EINVAL; } gconf->volname = g_strndup(q, p - q); p += strspn(p, "/"); if (*p == '\0') { return -EINVAL; } gconf->image = g_strdup(p); > > > > Note that gluster is currently able to resolve image paths that don't > > have a preceding slash (like dir/a.img). But I guess we should support > > specifying complete image paths too (like /dir/a.img) > > How would the URIs look like? gluster://server/testvol//dir/a.img ? Isn't this a valid URI ? Here volname=testvol and image=/dir/a.img. If that's valid and needs to be supported, then the above strspn based parsing logic needs some rewrite. Regards, Bharata.