qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Peter Lieven
Subject: Re: [Qemu-devel] [PATCHv5] block: add native support for NFS
Date: Fri, 10 Jan 2014 16:05:46 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0

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:

    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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]