[Qemu-devel] block/nfs: Fine grained runtime options in nfs

[Qemu-devel] block/nfs: Fine grained runtime options in nfs

[Ashijeet Acharya]

Hi all,

I was working on trying to add blockdev-add compatibility for the nfs
block driver but before that runtime options need to be separated into
various options rather than just a simple "filename" option.

I have added the following until now:
a) host
b) port (not sure about this one, do we just use a default port number?)
c) export
d) path (path to the file)

I have matched these with the URI but still let me know if i have
missed anyone :)

Now, in order to parse the uri for different runtime options, I have
made two new functions nfs_parse_filename() and nfs_parse_uri() which
is pretty similar to the way how other network block drivers do it.

Currently we parse the uri in a nfs_client_open() function which takes
'const char *filename' as one of its parameters but I dont think
that's the right way anymore because we pass 'qemu_opt_get(opts,
"filename")' which is invalid due to no runtime option named
"filename" available anymore. Right?

While parsing uri we check for the query parameters inside a 'for
loop', so I have moved that too inside

nfs_parse_uri(const char *filename, QDict *options, Error **errp)

but the problem is there is no struct NFSClient parameter here, so I
cannot fill up its important fields while parsing the query
parameters. I cannot do the same inside nfs_client_open() because I no
longer parse the uri over there.....OR CAN I? A completely different
approach will work too :)

I can attach a pastebin link containing a raw patch if you want to get
a clear view but I am afraid it doesn't compile at the moment due to
the problems mentioned above.

Any help will be appreciated.

Thanks for reading
Ashijeet

Re: [Qemu-devel] [Qemu-block] block/nfs: Fine grained runtime options in nfs

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1873 bytes --]

On Mon, Oct 10, 2016 at 10:39:30AM +0530, Ashijeet Acharya wrote:
> Hi all,
> 
> I was working on trying to add blockdev-add compatibility for the nfs
> block driver but before that runtime options need to be separated into
> various options rather than just a simple "filename" option.
> 
> I have added the following until now:
> a) host
> b) port (not sure about this one, do we just use a default port number?)
> c) export
> d) path (path to the file)
> 
> I have matched these with the URI but still let me know if i have
> missed anyone :)
> 
> Now, in order to parse the uri for different runtime options, I have
> made two new functions nfs_parse_filename() and nfs_parse_uri() which
> is pretty similar to the way how other network block drivers do it.
> 
> Currently we parse the uri in a nfs_client_open() function which takes
> 'const char *filename' as one of its parameters but I dont think
> that's the right way anymore because we pass 'qemu_opt_get(opts,
> "filename")' which is invalid due to no runtime option named
> "filename" available anymore. Right?
> 
> While parsing uri we check for the query parameters inside a 'for
> loop', so I have moved that too inside
> 
> nfs_parse_uri(const char *filename, QDict *options, Error **errp)
> 
> but the problem is there is no struct NFSClient parameter here, so I
> cannot fill up its important fields while parsing the query
> parameters. I cannot do the same inside nfs_client_open() because I no
> longer parse the uri over there.....OR CAN I? A completely different
> approach will work too :)
> 
> I can attach a pastebin link containing a raw patch if you want to get
> a clear view but I am afraid it doesn't compile at the moment due to
> the problems mentioned above.

Please post the code and annotate the relevant places where you are
stuck.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [Qemu-block] block/nfs: Fine grained runtime options in nfs

[Ashijeet Acharya]

On Fri, Oct 14, 2016 at 9:16 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Mon, Oct 10, 2016 at 10:39:30AM +0530, Ashijeet Acharya wrote:
>> Hi all,
>>
>> I was working on trying to add blockdev-add compatibility for the nfs
>> block driver but before that runtime options need to be separated into
>> various options rather than just a simple "filename" option.
>>
>> I have added the following until now:
>> a) host
>> b) port (not sure about this one, do we just use a default port number?)
>> c) export
>> d) path (path to the file)
>>
>> I have matched these with the URI but still let me know if i have
>> missed anyone :)
>>
>> Now, in order to parse the uri for different runtime options, I have
>> made two new functions nfs_parse_filename() and nfs_parse_uri() which
>> is pretty similar to the way how other network block drivers do it.
>>
>> Currently we parse the uri in a nfs_client_open() function which takes
>> 'const char *filename' as one of its parameters but I dont think
>> that's the right way anymore because we pass 'qemu_opt_get(opts,
>> "filename")' which is invalid due to no runtime option named
>> "filename" available anymore. Right?
>>
>> While parsing uri we check for the query parameters inside a 'for
>> loop', so I have moved that too inside
>>
>> nfs_parse_uri(const char *filename, QDict *options, Error **errp)
>>
>> but the problem is there is no struct NFSClient parameter here, so I
>> cannot fill up its important fields while parsing the query
>> parameters. I cannot do the same inside nfs_client_open() because I no
>> longer parse the uri over there.....OR CAN I? A completely different
>> approach will work too :)
>>
>> I can attach a pastebin link containing a raw patch if you want to get
>> a clear view but I am afraid it doesn't compile at the moment due to
>> the problems mentioned above.
>
> Please post the code and annotate the relevant places where you are
> stuck.

I have solved the issues I was facing earlier (thanks to Max!).
One more relatively easy question though, will we include @port as an
option in runtime_opts while converting NFS to use several
runtime_opts? The reason I ask this because the uri syntax for NFS in
QEMU looks like this:

       nfs://<host>/<export>/<filename>[?param=value[&param2=value2[&...]]]

At the moment my runtime_opts looks like this:

static QemuOptsList runtime_opts = {
    .name = "nfs",
    .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
    .desc = {
        {
            .name = "host",
            .type = QEMU_OPT_STRING,
            .help = "Host to connect to",
        },
        {
            .name = "export",
            .type = QEMU_OPT_STRING,
            .help = "Name of the NFS export to open",
        },
        {
            .name = "path",
            .type = QEMU_OPT_STRING,
            .help = "Path of the image on the host",
        },
        {
            .name = "uid",
            .type = QEMU_OPT_NUMBER,
            .help = "UID value to use when talking to the server",
        },
        {
            .name = "gid",
            .type = QEMU_OPT_NUMBER,
            .help = "GID value to use when talking to the server",
        },
        {
            .name = "tcp-syncnt",
            .type = QEMU_OPT_NUMBER,
            .help = "Number of SYNs to send during the session establish",
        },
        {
            .name = "readahead",
            .type = QEMU_OPT_NUMBER,
            .help = "Set the readahead size in bytes",
        },
        {
            .name = "pagecache",
            .type = QEMU_OPT_NUMBER,
            .help = "Set the pagecache size in bytes",
        },
        {
            .name = "debug",
            .type = QEMU_OPT_NUMBER,
            .help = "Set the NFS debug level (max 2)",
        },
        { /* end of list */ }
    },
};

Any comment on including several query params of the uri in
runtime_opts will be helpful too.

Ashijeet

> Stefan

Re: [Qemu-devel] [Qemu-block] block/nfs: Fine grained runtime options in nfs

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 563 bytes --]

On 10/17/2016 01:00 PM, Ashijeet Acharya wrote:

> One more relatively easy question though, will we include @port as an
> option in runtime_opts while converting NFS to use several
> runtime_opts? The reason I ask this because the uri syntax for NFS in
> QEMU looks like this:
> 
>        nfs://<host>/<export>/<filename>[?param=value[&param2=value2[&...]]]

It's actually nfs://<host>[:port]/...

so the URI syntax already supports port.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [Qemu-block] block/nfs: Fine grained runtime options in nfs

[Ashijeet Acharya]

On Tue, Oct 18, 2016 at 12:59 AM, Eric Blake <eblake@redhat.com> wrote:
> On 10/17/2016 01:00 PM, Ashijeet Acharya wrote:
>
>> One more relatively easy question though, will we include @port as an
>> option in runtime_opts while converting NFS to use several
>> runtime_opts? The reason I ask this because the uri syntax for NFS in
>> QEMU looks like this:
>>
>>        nfs://<host>/<export>/<filename>[?param=value[&param2=value2[&...]]]
>
> It's actually nfs://<host>[:port]/...
>
> so the URI syntax already supports port.

But the commit message which added support for NFS had the uri which I
mentioned above and the code for NFS does not make use of 'port'
anywhere either, which is why I am a bit confused.

Ashijeet
>
> --
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>

Re: [Qemu-devel] [Qemu-block] block/nfs: Fine grained runtime options in nfs

[Peter Lieven]

Am 17.10.2016 um 21:34 schrieb Ashijeet Acharya:
> On Tue, Oct 18, 2016 at 12:59 AM, Eric Blake <eblake@redhat.com> wrote:
>> On 10/17/2016 01:00 PM, Ashijeet Acharya wrote:
>>
>>> One more relatively easy question though, will we include @port as an
>>> option in runtime_opts while converting NFS to use several
>>> runtime_opts? The reason I ask this because the uri syntax for NFS in
>>> QEMU looks like this:
>>>
>>>         nfs://<host>/<export>/<filename>[?param=value[&param2=value2[&...]]]
>> It's actually nfs://<host>[:port]/...
>>
>> so the URI syntax already supports port.
> But the commit message which added support for NFS had the uri which I
> mentioned above and the code for NFS does not make use of 'port'
> anywhere either, which is why I am a bit confused.

Hi Aschijeet,

don't worry there is no port number when connecting to an NFS server.
The portmapper always listens on port 111. So theoretically we could
specifiy a port in the URL but it is ignored.

BR,
Peter

Re: [Qemu-devel] [Qemu-block] block/nfs: Fine grained runtime options in nfs

[Ashijeet Acharya]

On Tue, Oct 18, 2016 at 4:11 PM, Peter Lieven <pl@kamp.de> wrote:
> Am 17.10.2016 um 21:34 schrieb Ashijeet Acharya:
>>
>> On Tue, Oct 18, 2016 at 12:59 AM, Eric Blake <eblake@redhat.com> wrote:
>>>
>>> On 10/17/2016 01:00 PM, Ashijeet Acharya wrote:
>>>
>>>> One more relatively easy question though, will we include @port as an
>>>> option in runtime_opts while converting NFS to use several
>>>> runtime_opts? The reason I ask this because the uri syntax for NFS in
>>>> QEMU looks like this:
>>>>
>>>>
>>>> nfs://<host>/<export>/<filename>[?param=value[&param2=value2[&...]]]
>>>
>>> It's actually nfs://<host>[:port]/...
>>>
>>> so the URI syntax already supports port.
>>
>> But the commit message which added support for NFS had the uri which I
>> mentioned above and the code for NFS does not make use of 'port'
>> anywhere either, which is why I am a bit confused.
>
>
> Hi Aschijeet,
>
> don't worry there is no port number when connecting to an NFS server.
> The portmapper always listens on port 111. So theoretically we could
> specifiy a port in the URL but it is ignored.

So that means I will be including 'port' in runtime_opts and then just
ignoring any value that comes through it?

Ashijeet
>
> BR,
> Peter

Re: [Qemu-devel] [Qemu-block] block/nfs: Fine grained runtime options in nfs

[Kevin Wolf]

Am 18.10.2016 um 14:46 hat Ashijeet Acharya geschrieben:
> On Tue, Oct 18, 2016 at 4:11 PM, Peter Lieven <pl@kamp.de> wrote:
> > Am 17.10.2016 um 21:34 schrieb Ashijeet Acharya:
> >>
> >> On Tue, Oct 18, 2016 at 12:59 AM, Eric Blake <eblake@redhat.com> wrote:
> >>>
> >>> On 10/17/2016 01:00 PM, Ashijeet Acharya wrote:
> >>>
> >>>> One more relatively easy question though, will we include @port as an
> >>>> option in runtime_opts while converting NFS to use several
> >>>> runtime_opts? The reason I ask this because the uri syntax for NFS in
> >>>> QEMU looks like this:
> >>>>
> >>>>
> >>>> nfs://<host>/<export>/<filename>[?param=value[&param2=value2[&...]]]
> >>>
> >>> It's actually nfs://<host>[:port]/...
> >>>
> >>> so the URI syntax already supports port.
> >>
> >> But the commit message which added support for NFS had the uri which I
> >> mentioned above and the code for NFS does not make use of 'port'
> >> anywhere either, which is why I am a bit confused.
> >
> >
> > Hi Aschijeet,
> >
> > don't worry there is no port number when connecting to an NFS server.
> > The portmapper always listens on port 111. So theoretically we could
> > specifiy a port in the URL but it is ignored.
> 
> So that means I will be including 'port' in runtime_opts and then just
> ignoring any value that comes through it?

No, if there is nothing to configure there, leave it out. Adding an
option that doesn't do anything is not very useful.

Kevin

Re: [Qemu-devel] [Qemu-block] block/nfs: Fine grained runtime options in nfs

[Ashijeet Acharya]

On Tue, Oct 18, 2016 at 6:34 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 18.10.2016 um 14:46 hat Ashijeet Acharya geschrieben:
>> On Tue, Oct 18, 2016 at 4:11 PM, Peter Lieven <pl@kamp.de> wrote:
>> > Am 17.10.2016 um 21:34 schrieb Ashijeet Acharya:
>> >>
>> >> On Tue, Oct 18, 2016 at 12:59 AM, Eric Blake <eblake@redhat.com> wrote:
>> >>>
>> >>> On 10/17/2016 01:00 PM, Ashijeet Acharya wrote:
>> >>>
>> >>>> One more relatively easy question though, will we include @port as an
>> >>>> option in runtime_opts while converting NFS to use several
>> >>>> runtime_opts? The reason I ask this because the uri syntax for NFS in
>> >>>> QEMU looks like this:
>> >>>>
>> >>>>
>> >>>> nfs://<host>/<export>/<filename>[?param=value[&param2=value2[&...]]]
>> >>>
>> >>> It's actually nfs://<host>[:port]/...
>> >>>
>> >>> so the URI syntax already supports port.
>> >>
>> >> But the commit message which added support for NFS had the uri which I
>> >> mentioned above and the code for NFS does not make use of 'port'
>> >> anywhere either, which is why I am a bit confused.
>> >
>> >
>> > Hi Aschijeet,
>> >
>> > don't worry there is no port number when connecting to an NFS server.
>> > The portmapper always listens on port 111. So theoretically we could
>> > specifiy a port in the URL but it is ignored.
>>
>> So that means I will be including 'port' in runtime_opts and then just
>> ignoring any value that comes through it?
>
> No, if there is nothing to configure there, leave it out. Adding an
> option that doesn't do anything is not very useful.
>
Okay, understood.

So this is what my runtime_opts look like now:

static QemuOptsList runtime_opts = {
    .name = "nfs",
    .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
    .desc = {
        {
            .name = "host",
            .type = QEMU_OPT_STRING,
            .help = "Host to connect to",
        },
        {
            .name = "export",
            .type = QEMU_OPT_STRING,
            .help = "Name of the NFS export to open",
        },
        {
            .name = "path",
            .type = QEMU_OPT_STRING,
            .help = "Path of the image on the host",
        },
        {
            .name = "uid",
            .type = QEMU_OPT_NUMBER,
            .help = "UID value to use when talking to the server",
        },
        {
            .name = "gid",
            .type = QEMU_OPT_NUMBER,
            .help = "GID value to use when talking to the server",
        },
        {
            .name = "tcp-syncnt",
            .type = QEMU_OPT_NUMBER,
            .help = "Number of SYNs to send during the session establish",
        },
        {
            .name = "readahead",
            .type = QEMU_OPT_NUMBER,
            .help = "Set the readahead size in bytes",
        },
        {
            .name = "pagecache",
            .type = QEMU_OPT_NUMBER,
            .help = "Set the pagecache size in bytes",
        },
        {
            .name = "debug",
            .type = QEMU_OPT_NUMBER,
            .help = "Set the NFS debug level (max 2)",
        },
        { /* end of list */ }
    },
};

Please check if I have missed out on anything.
I have successfully converted NFS block driver to use this set of
runtime opts which I think is the required condition to add
blockdev-add compatibility later. Also, since I do not have 'port' as
a runtime option, I can directly add blockdev-add compatibility after
this through qapi/block-core.json and will not have to go through the
tricky method we are implementing for NBD and SSH as there will be no
use of InetSocketAddress. Right?

Ashijeet
> Kevin

Re: [Qemu-devel] [Qemu-block] block/nfs: Fine grained runtime options in nfs

[Kevin Wolf]

Am 18.10.2016 um 15:14 hat Ashijeet Acharya geschrieben:
> On Tue, Oct 18, 2016 at 6:34 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> > Am 18.10.2016 um 14:46 hat Ashijeet Acharya geschrieben:
> >> On Tue, Oct 18, 2016 at 4:11 PM, Peter Lieven <pl@kamp.de> wrote:
> >> > Am 17.10.2016 um 21:34 schrieb Ashijeet Acharya:
> >> >>
> >> >> On Tue, Oct 18, 2016 at 12:59 AM, Eric Blake <eblake@redhat.com> wrote:
> >> >>>
> >> >>> On 10/17/2016 01:00 PM, Ashijeet Acharya wrote:
> >> >>>
> >> >>>> One more relatively easy question though, will we include @port as an
> >> >>>> option in runtime_opts while converting NFS to use several
> >> >>>> runtime_opts? The reason I ask this because the uri syntax for NFS in
> >> >>>> QEMU looks like this:
> >> >>>>
> >> >>>>
> >> >>>> nfs://<host>/<export>/<filename>[?param=value[&param2=value2[&...]]]
> >> >>>
> >> >>> It's actually nfs://<host>[:port]/...
> >> >>>
> >> >>> so the URI syntax already supports port.
> >> >>
> >> >> But the commit message which added support for NFS had the uri which I
> >> >> mentioned above and the code for NFS does not make use of 'port'
> >> >> anywhere either, which is why I am a bit confused.
> >> >
> >> >
> >> > Hi Aschijeet,
> >> >
> >> > don't worry there is no port number when connecting to an NFS server.
> >> > The portmapper always listens on port 111. So theoretically we could
> >> > specifiy a port in the URL but it is ignored.
> >>
> >> So that means I will be including 'port' in runtime_opts and then just
> >> ignoring any value that comes through it?
> >
> > No, if there is nothing to configure there, leave it out. Adding an
> > option that doesn't do anything is not very useful.
> >
> Okay, understood.
> 
> So this is what my runtime_opts look like now:
> 
> static QemuOptsList runtime_opts = {
>     .name = "nfs",
>     .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
>     .desc = {
>         {
>             .name = "host",
>             .type = QEMU_OPT_STRING,
>             .help = "Host to connect to",
>         },
>         {
>             .name = "export",
>             .type = QEMU_OPT_STRING,
>             .help = "Name of the NFS export to open",
>         },
>         {
>             .name = "path",
>             .type = QEMU_OPT_STRING,
>             .help = "Path of the image on the host",
>         },

Does libnfs actually have separate export and path?

If I understand correctly, we currently split the URL at the last / in
the string. So is export the part before that and path the part after
it?

If so, does this mean that you can't currently access an image file in a
subdirectory of an NFS mount and adding the explicit options will fix
this?

>         {
>             .name = "uid",
>             .type = QEMU_OPT_NUMBER,
>             .help = "UID value to use when talking to the server",
>         },
>         {
>             .name = "gid",
>             .type = QEMU_OPT_NUMBER,
>             .help = "GID value to use when talking to the server",
>         },
>         {
>             .name = "tcp-syncnt",
>             .type = QEMU_OPT_NUMBER,
>             .help = "Number of SYNs to send during the session establish",
>         },
>         {
>             .name = "readahead",
>             .type = QEMU_OPT_NUMBER,
>             .help = "Set the readahead size in bytes",
>         },
>         {
>             .name = "pagecache",
>             .type = QEMU_OPT_NUMBER,
>             .help = "Set the pagecache size in bytes",
>         },
>         {
>             .name = "debug",
>             .type = QEMU_OPT_NUMBER,
>             .help = "Set the NFS debug level (max 2)",
>         },
>         { /* end of list */ }
>     },
> };
> 
> Please check if I have missed out on anything.

I don't see anything, but then I'm not an expert on NFS either. I hope
Peter can take a look.

Though maybe it's easier to see in the context of the full patch.

> I have successfully converted NFS block driver to use this set of
> runtime opts which I think is the required condition to add
> blockdev-add compatibility later. Also, since I do not have 'port' as
> a runtime option, I can directly add blockdev-add compatibility after
> this through qapi/block-core.json and will not have to go through the
> tricky method we are implementing for NBD and SSH as there will be no
> use of InetSocketAddress. Right?

Yes, InetSocketAddress is what makes things a bit tricky, and it doesn't
seem to be useful with the API we get from libnfs, so just directly
taking a host name should be okay. Then this one should be easier than
SSH.

Eric, do you agree, or do you think we should take into account that
libnfs might be extended one day to work on any socket?

Kevin

Re: [Qemu-devel] [Qemu-block] block/nfs: Fine grained runtime options in nfs

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1585 bytes --]

On 10/18/2016 08:33 AM, Kevin Wolf wrote:

>> I have successfully converted NFS block driver to use this set of
>> runtime opts which I think is the required condition to add
>> blockdev-add compatibility later. Also, since I do not have 'port' as
>> a runtime option, I can directly add blockdev-add compatibility after
>> this through qapi/block-core.json and will not have to go through the
>> tricky method we are implementing for NBD and SSH as there will be no
>> use of InetSocketAddress. Right?
> 
> Yes, InetSocketAddress is what makes things a bit tricky, and it doesn't
> seem to be useful with the API we get from libnfs, so just directly
> taking a host name should be okay. Then this one should be easier than
> SSH.
> 
> Eric, do you agree, or do you think we should take into account that
> libnfs might be extended one day to work on any socket?

Ideally, we want the valid JSON for ssh to be a subset of the valid JSON
for either InetSocketAddress, or for a flat counterpart (what we did for
gluster).  I kind of like the flat counterpart idea.  Yes, that probably
means we need to create a new QAPI type (comparable to the existing
types, but omitting port), rather than being able to reuse one; but as
long as the parameters are spelled the same, backwards-compatibility
states that we can later add fields, and that any two structs with
identical fields can be merged into one struct without breaking
backwards compatibility.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [Qemu-block] block/nfs: Fine grained runtime options in nfs

[Ashijeet Acharya]

On Tue, Oct 18, 2016 at 7:03 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 18.10.2016 um 15:14 hat Ashijeet Acharya geschrieben:
>> On Tue, Oct 18, 2016 at 6:34 PM, Kevin Wolf <kwolf@redhat.com> wrote:
>> > Am 18.10.2016 um 14:46 hat Ashijeet Acharya geschrieben:
>> >> On Tue, Oct 18, 2016 at 4:11 PM, Peter Lieven <pl@kamp.de> wrote:
>> >> > Am 17.10.2016 um 21:34 schrieb Ashijeet Acharya:
>> >> >>
>> >> >> On Tue, Oct 18, 2016 at 12:59 AM, Eric Blake <eblake@redhat.com> wrote:
>> >> >>>
>> >> >>> On 10/17/2016 01:00 PM, Ashijeet Acharya wrote:
>> >> >>>
>> >> >>>> One more relatively easy question though, will we include @port as an
>> >> >>>> option in runtime_opts while converting NFS to use several
>> >> >>>> runtime_opts? The reason I ask this because the uri syntax for NFS in
>> >> >>>> QEMU looks like this:
>> >> >>>>
>> >> >>>>
>> >> >>>> nfs://<host>/<export>/<filename>[?param=value[&param2=value2[&...]]]
>> >> >>>
>> >> >>> It's actually nfs://<host>[:port]/...
>> >> >>>
>> >> >>> so the URI syntax already supports port.
>> >> >>
>> >> >> But the commit message which added support for NFS had the uri which I
>> >> >> mentioned above and the code for NFS does not make use of 'port'
>> >> >> anywhere either, which is why I am a bit confused.
>> >> >
>> >> >
>> >> > Hi Aschijeet,
>> >> >
>> >> > don't worry there is no port number when connecting to an NFS server.
>> >> > The portmapper always listens on port 111. So theoretically we could
>> >> > specifiy a port in the URL but it is ignored.
>> >>
>> >> So that means I will be including 'port' in runtime_opts and then just
>> >> ignoring any value that comes through it?
>> >
>> > No, if there is nothing to configure there, leave it out. Adding an
>> > option that doesn't do anything is not very useful.
>> >
>> Okay, understood.
>>
>> So this is what my runtime_opts look like now:
>>
>> static QemuOptsList runtime_opts = {
>>     .name = "nfs",
>>     .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
>>     .desc = {
>>         {
>>             .name = "host",
>>             .type = QEMU_OPT_STRING,
>>             .help = "Host to connect to",
>>         },
>>         {
>>             .name = "export",
>>             .type = QEMU_OPT_STRING,
>>             .help = "Name of the NFS export to open",
>>         },
>>         {
>>             .name = "path",
>>             .type = QEMU_OPT_STRING,
>>             .help = "Path of the image on the host",
>>         },
>
> Does libnfs actually have separate export and path?
>
> If I understand correctly, we currently split the URL at the last / in
> the string. So is export the part before that and path the part after
> it?

At the moment, I don't see any piece of code in NFS BLOCK DRIVER which
does that to extract export from the URL. We actually do this at the
moment:

    strp = strrchr(uri->path, '/');

which I think is just to extract the name of the file from the URL and
has nothing to do with export.
Although NBD and gluster seem to extract export from the URL like this:

    p += strspn(p, "/");

which actually splits the URL at the first appearance of "/", doesn't
it? And then does after checking for p[0]:

    qdict_put(options, "export", qstring_from_str(p));

which puts the value of key export in qdict as the part of the URL
which occurs after the first appearance of "/", right?

>
> If so, does this mean that you can't currently access an image file in a
> subdirectory of an NFS mount and adding the explicit options will fix
> this?

I am not sure of that :-/

Ashijeet
>
>>         {
>>             .name = "uid",
>>             .type = QEMU_OPT_NUMBER,
>>             .help = "UID value to use when talking to the server",
>>         },
>>         {
>>             .name = "gid",
>>             .type = QEMU_OPT_NUMBER,
>>             .help = "GID value to use when talking to the server",
>>         },
>>         {
>>             .name = "tcp-syncnt",
>>             .type = QEMU_OPT_NUMBER,
>>             .help = "Number of SYNs to send during the session establish",
>>         },
>>         {
>>             .name = "readahead",
>>             .type = QEMU_OPT_NUMBER,
>>             .help = "Set the readahead size in bytes",
>>         },
>>         {
>>             .name = "pagecache",
>>             .type = QEMU_OPT_NUMBER,
>>             .help = "Set the pagecache size in bytes",
>>         },
>>         {
>>             .name = "debug",
>>             .type = QEMU_OPT_NUMBER,
>>             .help = "Set the NFS debug level (max 2)",
>>         },
>>         { /* end of list */ }
>>     },
>> };
>>
>> Please check if I have missed out on anything.
>
> I don't see anything, but then I'm not an expert on NFS either. I hope
> Peter can take a look.
>
> Though maybe it's easier to see in the context of the full patch.
>
>> I have successfully converted NFS block driver to use this set of
>> runtime opts which I think is the required condition to add
>> blockdev-add compatibility later. Also, since I do not have 'port' as
>> a runtime option, I can directly add blockdev-add compatibility after
>> this through qapi/block-core.json and will not have to go through the
>> tricky method we are implementing for NBD and SSH as there will be no
>> use of InetSocketAddress. Right?
>
> Yes, InetSocketAddress is what makes things a bit tricky, and it doesn't
> seem to be useful with the API we get from libnfs, so just directly
> taking a host name should be okay. Then this one should be easier than
> SSH.
>
> Eric, do you agree, or do you think we should take into account that
> libnfs might be extended one day to work on any socket?
>
> Kevin

Re: [Qemu-devel] [Qemu-block] block/nfs: Fine grained runtime options in nfs

[Ashijeet Acharya]

On Tue, Oct 18, 2016 at 9:43 PM, Ashijeet Acharya
<ashijeetacharya@gmail.com> wrote:
> On Tue, Oct 18, 2016 at 7:03 PM, Kevin Wolf <kwolf@redhat.com> wrote:
>> Am 18.10.2016 um 15:14 hat Ashijeet Acharya geschrieben:
>>> On Tue, Oct 18, 2016 at 6:34 PM, Kevin Wolf <kwolf@redhat.com> wrote:
>>> > Am 18.10.2016 um 14:46 hat Ashijeet Acharya geschrieben:
>>> >> On Tue, Oct 18, 2016 at 4:11 PM, Peter Lieven <pl@kamp.de> wrote:
>>> >> > Am 17.10.2016 um 21:34 schrieb Ashijeet Acharya:
>>> >> >>
>>> >> >> On Tue, Oct 18, 2016 at 12:59 AM, Eric Blake <eblake@redhat.com> wrote:
>>> >> >>>
>>> >> >>> On 10/17/2016 01:00 PM, Ashijeet Acharya wrote:
>>> >> >>>
>>> >> >>>> One more relatively easy question though, will we include @port as an
>>> >> >>>> option in runtime_opts while converting NFS to use several
>>> >> >>>> runtime_opts? The reason I ask this because the uri syntax for NFS in
>>> >> >>>> QEMU looks like this:
>>> >> >>>>
>>> >> >>>>
>>> >> >>>> nfs://<host>/<export>/<filename>[?param=value[&param2=value2[&...]]]
>>> >> >>>
>>> >> >>> It's actually nfs://<host>[:port]/...
>>> >> >>>
>>> >> >>> so the URI syntax already supports port.
>>> >> >>
>>> >> >> But the commit message which added support for NFS had the uri which I
>>> >> >> mentioned above and the code for NFS does not make use of 'port'
>>> >> >> anywhere either, which is why I am a bit confused.
>>> >> >
>>> >> >
>>> >> > Hi Aschijeet,
>>> >> >
>>> >> > don't worry there is no port number when connecting to an NFS server.
>>> >> > The portmapper always listens on port 111. So theoretically we could
>>> >> > specifiy a port in the URL but it is ignored.
>>> >>
>>> >> So that means I will be including 'port' in runtime_opts and then just
>>> >> ignoring any value that comes through it?
>>> >
>>> > No, if there is nothing to configure there, leave it out. Adding an
>>> > option that doesn't do anything is not very useful.
>>> >
>>> Okay, understood.
>>>
>>> So this is what my runtime_opts look like now:
>>>
>>> static QemuOptsList runtime_opts = {
>>>     .name = "nfs",
>>>     .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
>>>     .desc = {
>>>         {
>>>             .name = "host",
>>>             .type = QEMU_OPT_STRING,
>>>             .help = "Host to connect to",
>>>         },
>>>         {
>>>             .name = "export",
>>>             .type = QEMU_OPT_STRING,
>>>             .help = "Name of the NFS export to open",
>>>         },
>>>         {
>>>             .name = "path",
>>>             .type = QEMU_OPT_STRING,
>>>             .help = "Path of the image on the host",
>>>         },
>>
>> Does libnfs actually have separate export and path?
>>
>> If I understand correctly, we currently split the URL at the last / in
>> the string. So is export the part before that and path the part after
>> it?
>
> At the moment, I don't see any piece of code in NFS BLOCK DRIVER which
> does that to extract export from the URL. We actually do this at the
> moment:
>
>     strp = strrchr(uri->path, '/');
>
> which I think is just to extract the name of the file from the URL and
> has nothing to do with export.
> Although NBD and gluster seem to extract export from the URL like this:
>
>     p += strspn(p, "/");
>
> which actually splits the URL at the first appearance of "/", doesn't
> it? And then does after checking for p[0]:
>
>     qdict_put(options, "export", qstring_from_str(p));
>
> which puts the value of key export in qdict as the part of the URL
> which occurs after the first appearance of "/", right?
>
>>
>> If so, does this mean that you can't currently access an image file in a
>> subdirectory of an NFS mount and adding the explicit options will fix
>> this?
>
> I am not sure of that :-/
>
But either way I think I will have to drop export and was a silly
mistake to include it, since it is extracted from path one way or
another.

Ashijeet