From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48240) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zvz1V-0002qi-LY for qemu-devel@nongnu.org; Mon, 09 Nov 2015 21:51:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zvz1U-0006sG-ES for qemu-devel@nongnu.org; Mon, 09 Nov 2015 21:51:05 -0500 Date: Tue, 10 Nov 2015 10:50:50 +0800 From: Fam Zheng Message-ID: <20151110025050.GB15410@ad.usersys.redhat.com> References: <1447052973-14513-1-git-send-email-pl@kamp.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1447052973-14513-1-git-send-email-pl@kamp.de> Subject: Re: [Qemu-devel] [PATCH V5] block/nfs: add support for setting debug level List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Lieven Cc: kwolf@redhat.com, qemu-block@nongnu.org, stefanha@gmail.com, qemu-devel@nongnu.org, mreitz@redhat.com On Mon, 11/09 08:09, 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 > > Signed-off-by: Peter Lieven > --- > v4->v5: add a comment in the code why we limit the debug level [Stefan] > 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 | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/block/nfs.c b/block/nfs.c > index fd79f89..ab1e267 100644 > --- a/block/nfs.c > +++ b/block/nfs.c > @@ -36,6 +36,7 @@ > #include > > #define QEMU_NFS_MAX_READAHEAD_SIZE 1048576 > +#define QEMU_NFS_MAX_DEBUG_LEVEL 2 > > typedef struct NFSClient { > struct nfs_context *context; > @@ -334,6 +335,17 @@ 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")) { > + /* limit the maximum debug level to avoid potential flooding > + * of our log files. */ > + 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 It would be slightly nicer to do: } else if (!strcmp(qp->p[i].name, "debug")) { #ifdef LIBNFS_FEATURE_DEBUG /* limit the maximum debug level to avoid potential flooding * of our log files. */ 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); #else error_report("Specifying debug level is only supported by libnfs" " version >= XXX"); #endif So that you know which one out of QEMU and libnfs to upgrade when the option is refused. :) But it's your call. Either way: Reviewed-by: Fam Zheng > } else { > error_setg(errp, "Unknown NFS parameter name: %s", > qp->p[i].name); > -- > 1.9.1 >