[Qemu-devel] [PATCH V3] block/nfs: add support for setting debug level

[Qemu-devel] [PATCH V3] block/nfs: add support for setting debug level

[Peter Lieven]

upcoming libnfs versions will support logging debug messages. Add
support for it in qemu through a per-drive option.

Examples:
 qemu -drive if=virtio,file=nfs://...,file.debug=2
 qemu-img create -o debug=2 nfs://... 10G

Signed-off-by: Peter Lieven <pl@kamp.de>
---
v2->v3: use a per-drive option instead of a global one. [Stefan]
v1->v2: reworked patch to accept the debug level as a cmdline
        parameter instead of an URI parameter [Stefan]

 block/nfs.c          | 28 ++++++++++++++++++++++++----
 qapi/block-core.json | 20 ++++++++++++++++----
 2 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/block/nfs.c b/block/nfs.c
index c026ff6..72a4247 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -233,6 +233,11 @@ static QemuOptsList runtime_opts = {
             .type = QEMU_OPT_STRING,
             .help = "URL to the NFS file",
         },
+        {
+            .name = "debug",
+            .type = QEMU_OPT_NUMBER,
+            .help = "Set libnfs debug level (default 0 = no debug)",
+        },
         { /* end of list */ }
     },
 };
@@ -277,9 +282,9 @@ static void nfs_file_close(BlockDriverState *bs)
 }
 
 static int64_t nfs_client_open(NFSClient *client, const char *filename,
-                               int flags, Error **errp)
+                               int flags, QemuOpts *opts, Error **errp)
 {
-    int ret = -EINVAL, i;
+    int ret = -EINVAL, i, debug;
     struct stat st;
     URI *uri;
     QueryParams *qp = NULL;
@@ -343,6 +348,16 @@ static int64_t nfs_client_open(NFSClient *client, const char *filename,
         }
     }
 
+    debug = qemu_opt_get_number(opts, "debug", 0);
+    if (debug) {
+#ifdef LIBNFS_FEATURE_DEBUG
+        nfs_set_debug(client->context, debug);
+#else
+        error_report("NFS Warning: The linked version of libnfs does"
+                     " not support setting debug levels");
+#endif
+    }
+
     ret = nfs_mount(client->context, uri->server, uri->path);
     if (ret < 0) {
         error_setg(errp, "Failed to mount nfs share: %s",
@@ -405,7 +420,7 @@ static int nfs_file_open(BlockDriverState *bs, QDict *options, int flags,
     }
     ret = nfs_client_open(client, qemu_opt_get(opts, "filename"),
                           (flags & BDRV_O_RDWR) ? O_RDWR : O_RDONLY,
-                          errp);
+                          opts, errp);
     if (ret < 0) {
         goto out;
     }
@@ -425,6 +440,11 @@ static QemuOptsList nfs_create_opts = {
             .type = QEMU_OPT_SIZE,
             .help = "Virtual disk size"
         },
+        {
+            .name = "debug",
+            .type = QEMU_OPT_NUMBER,
+            .help = "Set libnfs debug level (default 0 = no debug)",
+        },
         { /* end of list */ }
     }
 };
@@ -441,7 +461,7 @@ static int nfs_file_create(const char *url, QemuOpts *opts, Error **errp)
     total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
                           BDRV_SECTOR_SIZE);
 
-    ret = nfs_client_open(client, url, O_CREAT, errp);
+    ret = nfs_client_open(client, url, O_CREAT, opts, errp);
     if (ret < 0) {
         goto out;
     }
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 7b2efb8..f43a1b1 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1381,9 +1381,9 @@
 { 'enum': 'BlockdevDriver',
   'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop',
             'dmg', 'file', 'ftp', 'ftps', 'host_cdrom', 'host_device',
-            'host_floppy', 'http', 'https', 'null-aio', 'null-co', 'parallels',
-            'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'tftp', 'vdi', 'vhdx',
-            'vmdk', 'vpc', 'vvfat' ] }
+            'host_floppy', 'http', 'https', 'nfs', 'null-aio', 'null-co',
+            'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'tftp',
+            'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] }
 
 ##
 # @BlockdevOptionsBase
@@ -1635,6 +1635,18 @@
             '*vport': 'int',
             '*segment': 'str' } }
 
+##
+# @BlockdevOptionsNFS
+#
+# Driver specific block device options for NFS.
+#
+# @debug:       #optional set libnfs debug level (default: 0 = disabled)
+#
+# Since: 2.4
+##
+{ 'struct': 'BlockdevOptionsNFS',
+  'base': 'BlockdevOptionsFile',
+  'data': { '*debug': 'int' } }
 
 ##
 # @BlkdebugEvent
@@ -1816,7 +1828,7 @@
       'https':      'BlockdevOptionsFile',
 # TODO iscsi: Wait for structured options
 # TODO nbd: Should take InetSocketAddress for 'host'?
-# TODO nfs: Wait for structured options
+      'nfs':        'BlockdevOptionsNFS',
       'null-aio':   'BlockdevOptionsNull',
       'null-co':    'BlockdevOptionsNull',
       'parallels':  'BlockdevOptionsGenericFormat',
-- 
1.9.1

Re: [Qemu-devel] [PATCH V3] block/nfs: add support for setting debug level

[Fam Zheng]

On Tue, 07/07 08:50, Peter Lieven wrote:
> upcoming libnfs versions will support logging debug messages. Add
> support for it in qemu through a per-drive option.
> 
> Examples:
>  qemu -drive if=virtio,file=nfs://...,file.debug=2
>  qemu-img create -o debug=2 nfs://... 10G
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>

Reviewed-by: Fam Zheng <famz@redhat.com>

Re: [Qemu-devel] [PATCH V3] block/nfs: add support for setting debug level

[Kevin Wolf]

Am 07.07.2015 um 08:50 hat Peter Lieven geschrieben:
> upcoming libnfs versions will support logging debug messages. Add
> support for it in qemu through a per-drive option.
> 
> Examples:
>  qemu -drive if=virtio,file=nfs://...,file.debug=2
>  qemu-img create -o debug=2 nfs://... 10G
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>

This patch does more than advertised in the commit message: It enables
NFS for blockdev-add, turning the options into a stable API.

'filename' is definitely not a good field name for an option taking an
NFS URL. What's more, a URL isn't really appropriate for QAPI either,
because it encodes structured information in a string rather than in
JSON objects. This part should work more like NBD, which has a
.bdrv_parse_filename implementation that splits a URL into individual
options.

But even with NBD, we weren't entirely sure if this is the right set of
options. The reason is that socket_optslist has a few more options than
just the hostname and port, and we would probably want to expose them in
blockdev-add. Not sure about NFS, it might be more limited there because
the connection is opened in libnfs.

Anyway, all of this isn't quite trivial to figure out, so I'm afraid we
need to revert the QAPI part before the 2.4 release.

Kevin



> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 7b2efb8..f43a1b1 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1381,9 +1381,9 @@
>  { 'enum': 'BlockdevDriver',
>    'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop',
>              'dmg', 'file', 'ftp', 'ftps', 'host_cdrom', 'host_device',
> -            'host_floppy', 'http', 'https', 'null-aio', 'null-co', 'parallels',
> -            'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'tftp', 'vdi', 'vhdx',
> -            'vmdk', 'vpc', 'vvfat' ] }
> +            'host_floppy', 'http', 'https', 'nfs', 'null-aio', 'null-co',
> +            'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'tftp',
> +            'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] }
>  
>  ##
>  # @BlockdevOptionsBase
> @@ -1635,6 +1635,18 @@
>              '*vport': 'int',
>              '*segment': 'str' } }
>  
> +##
> +# @BlockdevOptionsNFS
> +#
> +# Driver specific block device options for NFS.
> +#
> +# @debug:       #optional set libnfs debug level (default: 0 = disabled)
> +#
> +# Since: 2.4
> +##
> +{ 'struct': 'BlockdevOptionsNFS',
> +  'base': 'BlockdevOptionsFile',
> +  'data': { '*debug': 'int' } }
>  
>  ##
>  # @BlkdebugEvent
> @@ -1816,7 +1828,7 @@
>        'https':      'BlockdevOptionsFile',
>  # TODO iscsi: Wait for structured options
>  # TODO nbd: Should take InetSocketAddress for 'host'?
> -# TODO nfs: Wait for structured options
> +      'nfs':        'BlockdevOptionsNFS',
>        'null-aio':   'BlockdevOptionsNull',
>        'null-co':    'BlockdevOptionsNull',
>        'parallels':  'BlockdevOptionsGenericFormat',
> -- 
> 1.9.1
>