On 07/22/2016 06:01 AM, Prasanna Kumar Kalever wrote: > currently all the libgfapi logs defaults to '/dev/stderr' as it was hardcoded > in a call to glfs logging api, in case if debug level is chosen to DEBUG/TRACE
s/api, in case if/api. When the/
s/TRACE/TRACE,/
> gfapi logs will be huge and fill/overflow the console view.
>
> this patch provides a commandline option to mention log file path which helps
s/this/This/
> in logging to the specified file and also help in persisting the gfapi logs.
>
> Usage:
> -----
> *URI Style:
> ---------
> -drive file=gluster://hostname/volname/image.qcow2,file.debug=9,\
> file.logfile=/var/log/qemu/qemu-gfapi.log
>
> +++ b/block/gluster.c
> @@ -26,10 +26,12 @@
> #define GLUSTER_OPT_IPV4 "ipv4"
> #define GLUSTER_OPT_IPV6 "ipv6"
> #define GLUSTER_OPT_SOCKET "socket"
> -#define GLUSTER_OPT_DEBUG "debug"
> #define GLUSTER_DEFAULT_PORT 24007
> +#define GLUSTER_OPT_DEBUG "debug"
Why move this line?
> #define GLUSTER_DEBUG_DEFAULT 4
> #define GLUSTER_DEBUG_MAX 9
> +#define GLUSTER_OPT_LOGFILE "logfile"
> +#define GLUSTER_LOGFILE_DEFAULT "-" /* handled in libgfapi as
> /dev/stderr */
>
> @@ -576,7 +589,9 @@ static struct glfs
> *qemu_gluster_init(BlockdevOptionsGluster *gconf,
> if (ret < 0) {
> error_setg(errp, "invalid URI");
> error_append_hint(errp, "Usage: file=gluster[+transport]://"
> -
> "[host[:port]]/volume/path[?socket=...]\n");
> + "[host[:port]]volname/image[?socket=...]"
Why did you change absolute "/volume/path" to relative "volname/image"?
> + "[,file.debug=N]"
> + "[,file.logfile=/path/filename.log]\n");
> errno = -ret;
> return NULL;
> }
> @@ -586,7 +601,8 @@ static struct glfs
> *qemu_gluster_init(BlockdevOptionsGluster *gconf,
> error_append_hint(errp, "Usage: "
> "-drive driver=qcow2,file.driver=gluster,"
> "file.volume=testvol,file.path=/path/a.qcow2"
> - "[,file.debug=9],file.server.0.type=tcp,"
> +
> "[,file.debug=9][,file.logfile=/path/filename.log]"
> + "file.server.0.type=tcp,"
Missing a comma between the file.logfile and file.server keys.
> "file.server.0.host=1.2.3.4,"
> "file.server.0.port=24007,"
> "file.server.1.transport=unix,"
> @@ -677,7 +693,7 @@ static int qemu_gluster_open(BlockDriverState *bs, QDict
> *options,
> BlockdevOptionsGluster *gconf = NULL;
> QemuOpts *opts;
> Error *local_err = NULL;
> - const char *filename;
> + const char *filename, *logfile;
>
> opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
> qemu_opts_absorb_qdict(opts, options, &local_err);
> @@ -700,6 +716,17 @@ static int qemu_gluster_open(BlockDriverState *bs,
> QDict *options,
> gconf = g_new0(BlockdevOptionsGluster, 1);
> gconf->debug_level = s->debug_level;
> gconf->has_debug_level = true;
> +
> + logfile = qemu_opt_get(opts, GLUSTER_OPT_LOGFILE);
> + if (logfile) {
> + s->logfile = g_strdup(logfile);
> + } else {
> + s->logfile = g_strdup(GLUSTER_LOGFILE_DEFAULT);
> + }
I might have written
s->logfile = g_strdup(logfile ? logfile : GLUSTER_LOGFILE_DEFAULT);
> +++ b/qapi/block-core.json
> @@ -2138,13 +2138,16 @@
> #
> # @debug-level: #optional libgfapi log level (default '4' which is Error)
> #
> +# @logfile: #optional libgfapi log file (default /dev/stderr)
> +#
> # Since: 2.7
> ##
> { 'struct': 'BlockdevOptionsGluster',
> 'data': { 'volume': 'str',
> 'path': 'str',
> 'server': ['GlusterServer'],
> - '*debug_level': 'int' } }
> + '*debug_level': 'int',
> + '*logfile': 'str' } }
Very borderline on whether this qualifies as a bugfix, or if it is a
feature to be deferred to 2.8. I'll let the maintainer chime in.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
