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

[Ashijeet Acharya]

On Tue, Oct 18, 2016 at 6:34 PM, Kevin Wolf <address@hidden> wrote:
> Am 18.10.2016 um 14:46 hat Ashijeet Acharya geschrieben:
>> On Tue, Oct 18, 2016 at 4:11 PM, Peter Lieven <address@hidden> wrote:
>> > Am 17.10.2016 um 21:34 schrieb Ashijeet Acharya:
>> >>
>> >> On Tue, Oct 18, 2016 at 12:59 AM, Eric Blake <address@hidden> 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