[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCHv5] block: add native support for NFS
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCHv5] block: add native support for NFS |
Date: |
Fri, 10 Jan 2014 16:46:14 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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 <address@hidden> 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
- Re: [Qemu-devel] [PATCHv5] block: add native support for NFS, (continued)
- Re: [Qemu-devel] [PATCHv5] block: add native support for NFS, Peter Lieven, 2014/01/03
- Re: [Qemu-devel] [PATCHv5] block: add native support for NFS, Kevin Wolf, 2014/01/09
- Re: [Qemu-devel] [PATCHv5] block: add native support for NFS, Peter Lieven, 2014/01/09
- Re: [Qemu-devel] [PATCHv5] block: add native support for NFS, Kevin Wolf, 2014/01/10
- Re: [Qemu-devel] [PATCHv5] block: add native support for NFS, Peter Lieven, 2014/01/10
- Re: [Qemu-devel] [PATCHv5] block: add native support for NFS, Paolo Bonzini, 2014/01/10
- Re: [Qemu-devel] [PATCHv5] block: add native support for NFS, ronnie sahlberg, 2014/01/10
- Re: [Qemu-devel] [PATCHv5] block: add native support for NFS, Peter Lieven, 2014/01/10
- Re: [Qemu-devel] [PATCHv5] block: add native support for NFS,
Kevin Wolf <=
- Re: [Qemu-devel] [PATCHv5] block: add native support for NFS, Peter Lieven, 2014/01/10
- Re: [Qemu-devel] [PATCHv5] block: add native support for NFS, ronnie sahlberg, 2014/01/10
- Re: [Qemu-devel] [PATCHv5] block: add native support for NFS, Paolo Bonzini, 2014/01/10
- Re: [Qemu-devel] [PATCHv5] block: add native support for NFS, Peter Lieven, 2014/01/10
- Re: [Qemu-devel] [PATCHv5] block: add native support for NFS, Paolo Bonzini, 2014/01/10
- Re: [Qemu-devel] [PATCHv5] block: add native support for NFS, Peter Lieven, 2014/01/10