Re: [Qemu-block] [PATCH v6 0/2] allow blockdev-add for NFS

[Peter Lieven]

Am 31.10.2016 um 18:20 schrieb Kevin Wolf:
> Am 31.10.2016 um 16:05 hat Ashijeet Acharya geschrieben:
> > Previously posted series patches:
> > v5: https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg07580.html
> > v4: https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg07449.html
> > v3: https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg06903.html
> > v2: https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg05844.html
> > v1: https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg04487.html
> >
> > This series adds blockdev-add support for NFS block driver.
> Thanks, fixed as commented on patch 1 and applied.

Hi,

it seems this series breaks passing options via URI.

1) in nfs_parse_uri

parse_uint_full(qp->p[i].value, NULL, 0)

segfaults, as the routine wants to set *NULL = val.

Program received signal SIGSEGV, Segmentation fault.
parse_uint (s=0x555555d0ead0 "131072", address@hidden, address@hidden, 
address@hidden) at util/cutils.c:475
475        *value = val;
(gdb) bt full
#0  parse_uint (s=0x555555d0ead0 "131072", address@hidden, address@hidden, 
address@hidden) at util/cutils.c:475
        r = 0
        endp = 0x555555d0ead6 ""
        val = 131072
#1  0x0000555555655ff2 in parse_uint_full (s=<optimized out>, address@hidden, 
address@hidden) at util/cutils.c:499
        endp = 0x555555d093f0 "\004"
        r = <optimized out>
#2  0x0000555555603425 in nfs_parse_uri (filename=<optimized out>, 
options=0x555555d093f0, errp=0x7fffffffd980) at block/nfs.c:116
        uri = 0x555555d0e920
        qp = 0x555555d0ea30
        ret = -22
        i = 0
        __func__ = "nfs_parse_uri"
#3  0x000055555559c7bb in bdrv_fill_options (errp=0x7fffffffd968, flags=0x7fffffffd95c, 
filename=<optimized out>, options=<synthetischer Zeiger>) at block.c:1278
        drvname = <optimized out>
        protocol = <optimized out>
        local_err = 0x0
        parse_filename = true
        drv = 0x5555558e3140 <bdrv_nfs>


2) all parameters that have a different names in options and qdict
e.g. readahead-size vs. readahead cannot be passed via URI.

$ qemu-img convert -p 
nfs://172.21.200.61/templates/VC_debian8-20170116.qcow2,linux\?readahead=131072 
iscsi://172.21.200.56:3260/iqn.2001-05.com.equallogic:0-8a0906-69d384e0a-aa3004e55e15878d-XXX/0

qemu-img: Could not open 
'nfs://172.21.200.61/vcore-dev-cdrom/templates/VC_debian8-20170116.qcow2,linux?readahead=131072':
 Block protocol 'nfs' doesn't support the option 'readahead-size'

Please let me know if the below fix would be correct:

address@hidden:~/git/qemu$ git diff
diff --git a/block/nfs.c b/block/nfs.c
index a564340..0ff8268 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -108,30 +108,31 @@ static int nfs_parse_uri(const char *filename, QDict 
*options, Error **errp)
     qdict_put(options, "path", qstring_from_str(uri->path));

     for (i = 0; i < qp->n; i++) {
+        unsigned long long val;
         if (!qp->p[i].value) {
             error_setg(errp, "Value for NFS parameter expected: %s",
                        qp->p[i].name);
             goto out;
         }
-        if (parse_uint_full(qp->p[i].value, NULL, 0)) {
+        if (parse_uint_full(qp->p[i].value, &val, 0)) {
             error_setg(errp, "Illegal value for NFS parameter: %s",
                        qp->p[i].name);
             goto out;
         }
         if (!strcmp(qp->p[i].name, "uid")) {
-            qdict_put(options, "user",
+            qdict_put(options, "uid",
                       qstring_from_str(qp->p[i].value));
         } else if (!strcmp(qp->p[i].name, "gid")) {
-            qdict_put(options, "group",
+            qdict_put(options, "gid",
                       qstring_from_str(qp->p[i].value));
         } else if (!strcmp(qp->p[i].name, "tcp-syncnt")) {
-            qdict_put(options, "tcp-syn-count",
+            qdict_put(options, "tcp-syncnt",
                       qstring_from_str(qp->p[i].value));
         } else if (!strcmp(qp->p[i].name, "readahead")) {
-            qdict_put(options, "readahead-size",
+            qdict_put(options, "readahead",
                       qstring_from_str(qp->p[i].value));
         } else if (!strcmp(qp->p[i].name, "pagecache")) {
-            qdict_put(options, "page-cache-size",
+            qdict_put(options, "pagecache",
                       qstring_from_str(qp->p[i].value));
         } else if (!strcmp(qp->p[i].name, "debug")) {
             qdict_put(options, "debug",


Thanks,
Peter