Re: [Qemu-devel] [PATCHv5] block: add native support for NFS

[Peter Lieven <pl@kamp.de>]

On 10.01.2014 15:49, ronnie sahlberg wrote:
> On Fri, Jan 10, 2014 at 4:30 AM, Paolo Bonzini <pbonzini@redhat.com> 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