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

[Kevin Wolf <kwolf@redhat.com>]

Am 10.01.2014 um 16:05 hat Peter Lieven geschrieben:
> 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:
> [...]

Agreed.

> 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);

I wouldn't do that. We should use specific functions like Ronnie
suggested in his nfs_set_auth() example.

> And we leave all the
> 
> QemuOptsList
> 
> and qapi-schema.json stuff for a later version when we touch all the other protocols.

Okay, I'll take care of it. For the time being, please include the TODO
comment that the other network-based drivers have.

Kevin