From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53029) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W1ddI-00081J-Bf for qemu-devel@nongnu.org; Fri, 10 Jan 2014 10:04:32 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W1dd9-00021K-0o for qemu-devel@nongnu.org; Fri, 10 Jan 2014 10:04:23 -0500 Received: from mx.ipv6.kamp.de ([2a02:248:0:51::16]:51815 helo=mx01.kamp.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W1dd8-00020Q-MM for qemu-devel@nongnu.org; Fri, 10 Jan 2014 10:04:14 -0500 Message-ID: <52D00C4A.6010907@kamp.de> Date: Fri, 10 Jan 2014 16:05:46 +0100 From: Peter Lieven MIME-Version: 1.0 References: <1388062130-21126-1-git-send-email-pl@kamp.de> <20140109141322.GB2862@dhcp-200-207.str.redhat.com> <52CEC998.4040409@kamp.de> <20140110114042.GB4276@dhcp-200-207.str.redhat.com> <52CFE396.7070802@kamp.de> <52CFE7C9.9060805@redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCHv5] block: add native support for NFS List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: ronnie sahlberg , Paolo Bonzini Cc: Kevin Wolf , qemu-devel , Orit Wasserman , Stefan Hajnoczi , "famz@redhat.com" On 10.01.2014 15:49, ronnie sahlberg wrote: > On Fri, Jan 10, 2014 at 4:30 AM, Paolo Bonzini wrote: >> Il 10/01/2014 13:12, Peter Lieven ha scritto: >>> Then I shall convert everything to a qapi schema whereby the current >>> design of libnfs is designed to work with plain URLs. >> No, no one is asking you to do this. URLs are fine, but I agree with >> Kevin that parsing them in QEMU is better. >> >> Also because the QEMU parser is known to be based on RFCs and good code >> from libxml2. For example, the iSCSI URL parser, when introduced, >> didn't even have percent-escape parsing, causing libvirt to fail with >> old libiscsi (and actually not that old too: IIRC libiscsi 1.7 will >> still fail). Unless the libnfs parser is as good as libxml2's, I think >> there's value in using the QEMU URI parser. > > I think that is fair enough. > > The arguments we are talking about are the type of arguments that only > affect the interface between libnfs and the nfs server itself > and is not strictly all that interesting to the application that links > to libnfs. > > Since parsing a URL does require a fair amount of code, a hundred > lines or more, it is a bit annoying having to re-implement the parsing > code for every single small utility. For example nfs-ls nfs-cp > nfs-cp or for the parsing, that is still done, in the sg-utils > patch. > For a lot of these small and semi-trivial applications we don't really > care all that much about what the options are but we might care a lot > about making it easier to use libnfs and to avoid having to write a > parser each time. > > For those use cases, I definitely think that having a built in > function to parse a url, and automatically update the nfs context with > connection related tweaks is a good thing. It eliminates the need to > re-implement the parsing functions in every single trivial > application. > > > For QEMU and libvirt things may be different. These are non-trivial > applications and may have needs to be able to control the settings > explicitely in the QEMU code. > That is still possible to do. All the url arguments so far tweak > arguments that can also be controlled through explicit existing APIs. > So for QEMU, there are functions available in libnfs now that will > automatically update the nfs context with things like UID/GID to use > when talking to the server, passed via the URL and QEMU can use them. > On the other hand, if QEMU rather wants to parse the URL itself > and make calls into the libnfs API to tweak these settings directly > from the QEMU codebase, that is also possible. > > > For example: nfs://1.2.3.4/path/file?uid=10&gid=10 > When parsing these using the libnfs functions, the parsing functions > will automatically update the nfs context so that it will use these > values when it fills in the rpc header to send to the server. > But if you want to parse the url yourself, you can do that too, by > just calling nfs_set_auth(nfs, libnfs_authunix_create(..., 10, 10, > ... Proposal: I revert the URL parsing code to v4 of the patch: URI *uri; char *file = NULL, *strp = NULL; uri = uri_parse(filename); if (!uri) { error_setg(errp, "Invalid URL specified."); goto fail; } strp = strrchr(uri->path, '/'); if (strp == NULL) { error_setg(errp, "Invalid URL specified."); goto fail; } file = g_strdup(strp); *strp = 0; And then pipe all the URL params (in QueryParams) through a (to be defined public) function in libnfs nfs_set_context_args(struct nfs_context *nfs, char *arg, char *val); And we leave all the QemuOptsList and qapi-schema.json stuff for a later version when we touch all the other protocols. Peter