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

[Peter Krempa]

On Thu, Sep 15, 2016 at 15:32:48 +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(-)

This patch fails qemuxml2argvtest and make syntax-check. Please make
sure to run the testsuite and syntax-check before submission.

> 
> 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

Missing change to src/qemu/libvirtd_qemu.aug

> 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);
>  
> @@ -2177,6 +2179,7 @@ qemuBuildDriveDevStr(const virDomainDef *def,
>  
>  static int
>  qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
> +                              virQEMUDriverConfigPtr cfg,
>                                const virDomainDef *def,
>                                virQEMUCapsPtr qemuCaps)
>  {
> @@ -2255,6 +2258,13 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
>  
>          virCommandAddArg(cmd, "-drive");
>  
> +        if (disk->src &&
> +                disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER) {

Misaligned.

> +            if(cfg->qemuGfapiDebugLevel) {
> +                disk->src->debug_level = cfg->qemuGfapiDebugLevel;
> +            }

Fails at least two syntax-check rules.

> +        }

Since this is done for gluster only I don't see a reason to have a
debug_level property for the storage source. It would make sense if you
could configure it on a per disk basis.

> +
>          if (!(optstr = qemuBuildDriveStr(disk, driveBoot, qemuCaps)))
>              return -1;
>          virCommandAddArg(cmd, optstr);

Peter