[libvirt] [PATCH v1 1/1] qemu/gluster: add option for tuning debug logging level

[Peter Krempa]

On Thu, Sep 15, 2016 at 13:49:31 +0100, Daniel Berrange wrote:
> On Thu, Sep 15, 2016 at 03:32:48PM +0530, Prasanna Kumar Kalever wrote:
> > This helps in selecting log level of the gluster gfapi, output to stderr.
> > The option is 'qemu_gfapi_debuglevel', can be tuned by editing
> > '/etc/libvirt/qemu.conf'
> > 
> > Debug levels ranges 0-9, with 9 being the most verbose, and 0 representing
> > no debugging output.  The default is the same as it was before, which
> > is a level of 4.  The current logging levels defined in the gluster
> > gfapi are:
> > 
> >     0 - None
> >     1 - Emergency
> >     2 - Alert
> >     3 - Critical
> >     4 - Error
> >     5 - Warning
> >     6 - Notice
> >     7 - Info
> >     8 - Debug
> >     9 - Trace
> > 
> > Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever at redhat.com>
> > --
> > v1: Initial post
> > ---
> >  src/qemu/qemu.conf        | 20 ++++++++++++++++++++
> >  src/qemu/qemu_command.c   | 12 +++++++++++-
> >  src/qemu/qemu_conf.c      |  3 +++
> >  src/qemu/qemu_conf.h      |  1 +
> >  src/util/virstoragefile.h |  2 ++
> >  5 files changed, 37 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
> > index e4c2aae..c6c8f3a 100644
> > --- a/src/qemu/qemu.conf
> > +++ b/src/qemu/qemu.conf
> > @@ -621,3 +621,23 @@
> >  #          rollover when a size limit is hit.
> >  #
> >  #stdio_handler = "logd"
> > +
> > +# Qemu gluster libgfapi log level, debug levels are 0-9, with 9 being the
> > +# most verbose, and 0 representing no debugging output.
> > +#
> > +# The current logging levels defined in the gluster GFAPI are:
> > +#
> > +#    0 - None
> > +#    1 - Emergency
> > +#    2 - Alert
> > +#    3 - Critical
> > +#    4 - Error
> > +#    5 - Warning
> > +#    6 - Notice
> > +#    7 - Info
> > +#    8 - Debug
> > +#    9 - Trace
> > +#
> > +# Defaults to 4
> > +#
> > +# qemu_gfapi_debuglevel = 9
> 
> Every option in this file is about QEMU so don't need that
> as a name prefix. 'gfapi' is also pretty obscure naming.
> 
> Lets just call it 'glusterfs_debug_level'

glusterfs is unfortunately misleading a bit as it's usualy used when
referring to the FUSE driver for accessing gluster volumes. I'd drop the
'fs' suffix in this case.

> 
> > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > index 3a61863..650eedc 100644
> > --- a/src/qemu/qemu_command.c
> > +++ b/src/qemu/qemu_command.c
> > @@ -859,6 +859,7 @@ qemuBuildGlusterDriveJSON(virStorageSourcePtr src)
> >       /* { driver:"gluster",
> >        *   volume:"testvol",
> >        *   path:"/a.img",
> > +      *   debug:9,
> >        *   server :[{type:"tcp", host:"1.2.3.4", port:24007},
> >        *            {type:"unix", socket:"/tmp/glusterd.socket"}, ...]}
> >        */
> > @@ -866,6 +867,7 @@ qemuBuildGlusterDriveJSON(virStorageSourcePtr src)
> >                                   "s:driver", protocol,
> >                                   "s:volume", src->volume,
> >                                   "s:path", src->path,
> > +                                 "u:debug", src->debug_level,
> >                                   "a:server", servers, NULL) < 0)
> >            virJSONValueFree(servers);
> >
> 
> I think you might need a capability check aganist QEMU to see if 'debug'
> is a permitted parameter before using it, unless some existing capability
> check is sufficient }

It's sufficient as-is in the JSON object code path. It might be
necessary to add a capability check when used in the URI (legacy) code
path.

Peter