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

[Peter Lieven <pl@kamp.de>]

Am 10.01.2014 16:46, schrieb Kevin Wolf:
> 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.
Ronnie, I would map to the following functions. Especially for uid,gid because
we would have to add all that specific what to do on windows and what to do if
a user specifies only a uid and no gid stuff again:

uid => rpc_set_uid
gid => rpc_set_gid
tcp-syncnt => rpc_set_tcp_syncnt
autoreconnect => rpc_{set,unset}_autoreconnect

Ronnie, can you also give a short advise on Kevin's question about short reads.
I think they can happen if we read beyond past EOF or not?

>
>> 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.
Thanks. Kevin, can you please give an advice how to proceed with the qemu-iotests.

Peter