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

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

[Peter Lieven]

recent libnfs versions support logging debug messages. Add
support for it in qemu through an URL parameter.

Example:
 qemu -cdrom nfs://127.0.0.1/iso/my.iso?debug=2
---
v3->v4: revert to the initial version, but limit max debug level
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 |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/block/nfs.c b/block/nfs.c
index fd79f89..fbea25a 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -36,6 +36,7 @@
 #include <nfsc/libnfs.h>
 
 #define QEMU_NFS_MAX_READAHEAD_SIZE 1048576
+#define QEMU_NFS_MAX_DEBUG_LEVEL 2
 
 typedef struct NFSClient {
     struct nfs_context *context;
@@ -334,6 +335,15 @@ static int64_t nfs_client_open(NFSClient *client, const char *filename,
             }
             nfs_set_readahead(client->context, val);
 #endif
+#ifdef LIBNFS_FEATURE_DEBUG
+        } else if (!strcmp(qp->p[i].name, "debug")) {
+            if (val > QEMU_NFS_MAX_DEBUG_LEVEL) {
+                error_report("NFS Warning: Limiting NFS debug level"
+                             " to %d", QEMU_NFS_MAX_DEBUG_LEVEL);
+                val = QEMU_NFS_MAX_DEBUG_LEVEL;
+            }
+            nfs_set_debug(client->context, val);
+#endif
         } else {
             error_setg(errp, "Unknown NFS parameter name: %s",
                        qp->p[i].name);
-- 
1.7.9.5

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

[Stefan Hajnoczi]

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

On Thu, Nov 05, 2015 at 11:25:34PM +0100, Peter Lieven wrote:
> recent libnfs versions support logging debug messages. Add
> support for it in qemu through an URL parameter.
> 
> Example:
>  qemu -cdrom nfs://127.0.0.1/iso/my.iso?debug=2
> ---
> v3->v4: revert to the initial version, but limit max debug level
> 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 |   10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/block/nfs.c b/block/nfs.c
> index fd79f89..fbea25a 100644
> --- a/block/nfs.c
> +++ b/block/nfs.c
> @@ -36,6 +36,7 @@
>  #include <nfsc/libnfs.h>
>  
>  #define QEMU_NFS_MAX_READAHEAD_SIZE 1048576
> +#define QEMU_NFS_MAX_DEBUG_LEVEL 2
>  
>  typedef struct NFSClient {
>      struct nfs_context *context;
> @@ -334,6 +335,15 @@ static int64_t nfs_client_open(NFSClient *client, const char *filename,
>              }
>              nfs_set_readahead(client->context, val);
>  #endif
> +#ifdef LIBNFS_FEATURE_DEBUG
> +        } else if (!strcmp(qp->p[i].name, "debug")) {
> +            if (val > QEMU_NFS_MAX_DEBUG_LEVEL) {
> +                error_report("NFS Warning: Limiting NFS debug level"
> +                             " to %d", QEMU_NFS_MAX_DEBUG_LEVEL);
> +                val = QEMU_NFS_MAX_DEBUG_LEVEL;
> +            }
> +            nfs_set_debug(client->context, val);
> +#endif

Please include the rationale for limiting the debug level as a comment
in the code.  Otherwise people may read the code and not understand why
we do this.

The error message should indicate how to set high debug levels (if
possible).  A simple "You can't do that" error message is frustrating
:).

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

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

[Peter Lieven]

Am 06.11.2015 um 12:23 schrieb Stefan Hajnoczi:
> On Thu, Nov 05, 2015 at 11:25:34PM +0100, Peter Lieven wrote:
>> recent libnfs versions support logging debug messages. Add
>> support for it in qemu through an URL parameter.
>>
>> Example:
>>  qemu -cdrom nfs://127.0.0.1/iso/my.iso?debug=2
>> ---
>> v3->v4: revert to the initial version, but limit max debug level
>> 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 |   10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/block/nfs.c b/block/nfs.c
>> index fd79f89..fbea25a 100644
>> --- a/block/nfs.c
>> +++ b/block/nfs.c
>> @@ -36,6 +36,7 @@
>>  #include <nfsc/libnfs.h>
>>  
>>  #define QEMU_NFS_MAX_READAHEAD_SIZE 1048576
>> +#define QEMU_NFS_MAX_DEBUG_LEVEL 2
>>  
>>  typedef struct NFSClient {
>>      struct nfs_context *context;
>> @@ -334,6 +335,15 @@ static int64_t nfs_client_open(NFSClient *client, const char *filename,
>>              }
>>              nfs_set_readahead(client->context, val);
>>  #endif
>> +#ifdef LIBNFS_FEATURE_DEBUG
>> +        } else if (!strcmp(qp->p[i].name, "debug")) {
>> +            if (val > QEMU_NFS_MAX_DEBUG_LEVEL) {
>> +                error_report("NFS Warning: Limiting NFS debug level"
>> +                             " to %d", QEMU_NFS_MAX_DEBUG_LEVEL);
>> +                val = QEMU_NFS_MAX_DEBUG_LEVEL;
>> +            }
>> +            nfs_set_debug(client->context, val);
>> +#endif
> Please include the rationale for limiting the debug level as a comment
> in the code.  Otherwise people may read the code and not understand why
> we do this.

Okay. Then we should add this to the readahead limitation as well.

>
> The error message should indicate how to set high debug levels (if
> possible).  A simple "You can't do that" error message is frustrating
> :).

But thats the case at the moment ;-)

Peter