diff mbox

[PULL,1/2] block/gluster: add support to choose libgfapi logfile

Message ID 1473745333-7548-2-git-send-email-jcody@redhat.com
State New
Headers show

Commit Message

Jeff Cody Sept. 13, 2016, 5:42 a.m. UTC
From: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>

currently all the libgfapi logs defaults to '/dev/stderr' as it was hardcoded
in a call to glfs logging api. When the debug level is chosen to DEBUG/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
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

 *JSON Style:
  ----------
  'json:{
           "driver":"qcow2",
           "file":{
              "driver":"gluster",
              "volume":"volname",
              "path":"image.qcow2",
              "debug":"9",
              "logfile":"/var/log/qemu/qemu-gfapi.log",
              "server":[
                 {
                    "type":"tcp",
                    "host":"1.2.3.4",
                    "port":24007
                 },
                 {
                    "type":"unix",
                    "socket":"/var/run/glusterd.socket"
                 }
              ]
           }
        }'

Reviewed-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/gluster.c      | 42 ++++++++++++++++++++++++++++++++++++++----
 qapi/block-core.json |  5 ++++-
 2 files changed, 42 insertions(+), 5 deletions(-)

Comments

Eric Blake Sept. 13, 2016, 3:23 p.m. UTC | #1
On 09/13/2016 12:42 AM, Jeff Cody wrote:
> From: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
> 
> currently all the libgfapi logs defaults to '/dev/stderr' as it was hardcoded
> in a call to glfs logging api. When the debug level is chosen to DEBUG/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
> in logging to the specified file and also help in persisting the gfapi logs.
> 

> +++ b/qapi/block-core.json
> @@ -2154,13 +2154,16 @@
>  #
>  # @debug-level: #optional libgfapi log level (default '4' which is Error)
>  #
> +# @logfile:     #optional libgfapi log file (default /dev/stderr)
> +#
>  # Since: 2.7

logfile missed 2.7; please add a '(since 2.8)' designator on the
@logfile line as a followup patch.
Jeff Cody Sept. 13, 2016, 3:25 p.m. UTC | #2
On Tue, Sep 13, 2016 at 10:23:32AM -0500, Eric Blake wrote:
> On 09/13/2016 12:42 AM, Jeff Cody wrote:
> > From: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
> > 
> > currently all the libgfapi logs defaults to '/dev/stderr' as it was hardcoded
> > in a call to glfs logging api. When the debug level is chosen to DEBUG/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
> > in logging to the specified file and also help in persisting the gfapi logs.
> > 
> 
> > +++ b/qapi/block-core.json
> > @@ -2154,13 +2154,16 @@
> >  #
> >  # @debug-level: #optional libgfapi log level (default '4' which is Error)
> >  #
> > +# @logfile:     #optional libgfapi log file (default /dev/stderr)
> > +#
> >  # Since: 2.7
> 
> logfile missed 2.7; please add a '(since 2.8)' designator on the
> @logfile line as a followup patch.
> 
>

Yep, thanks for catching that Eric.
diff mbox

Patch

diff --git a/block/gluster.c b/block/gluster.c
index 01b479f..e7bd13c 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -30,6 +30,8 @@ 
 #define GLUSTER_DEFAULT_PORT        24007
 #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 */
 
 #define GERR_INDEX_HINT "hint: check in 'server' array index '%d'\n"
 
@@ -44,6 +46,7 @@  typedef struct GlusterAIOCB {
 typedef struct BDRVGlusterState {
     struct glfs *glfs;
     struct glfs_fd *fd;
+    char *logfile;
     bool supports_seek_data;
     int debug_level;
 } BDRVGlusterState;
@@ -73,6 +76,11 @@  static QemuOptsList qemu_gluster_create_opts = {
             .type = QEMU_OPT_NUMBER,
             .help = "Gluster log level, valid range is 0-9",
         },
+        {
+            .name = GLUSTER_OPT_LOGFILE,
+            .type = QEMU_OPT_STRING,
+            .help = "Logfile path of libgfapi",
+        },
         { /* end of list */ }
     }
 };
@@ -91,6 +99,11 @@  static QemuOptsList runtime_opts = {
             .type = QEMU_OPT_NUMBER,
             .help = "Gluster log level, valid range is 0-9",
         },
+        {
+            .name = GLUSTER_OPT_LOGFILE,
+            .type = QEMU_OPT_STRING,
+            .help = "Logfile path of libgfapi",
+        },
         { /* end of list */ }
     },
 };
@@ -341,7 +354,7 @@  static struct glfs *qemu_gluster_glfs_init(BlockdevOptionsGluster *gconf,
         }
     }
 
-    ret = glfs_set_logging(glfs, "-", gconf->debug_level);
+    ret = glfs_set_logging(glfs, gconf->logfile, gconf->debug_level);
     if (ret < 0) {
         goto out;
     }
@@ -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]]volume/path[?socket=...]"
+                                    "[,file.debug=N]"
+                                    "[,file.logfile=/path/filename.log]\n");
             errno = -ret;
             return NULL;
         }
@@ -586,7 +601,9 @@  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,"
                              "file.server.0.host=1.2.3.4,"
                              "file.server.0.port=24007,"
                              "file.server.1.transport=unix,"
@@ -677,7 +694,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 +717,13 @@  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);
+    s->logfile = g_strdup(logfile ? logfile : GLUSTER_LOGFILE_DEFAULT);
+
+    gconf->logfile = g_strdup(s->logfile);
+    gconf->has_logfile = true;
+
     s->glfs = qemu_gluster_init(gconf, filename, options, errp);
     if (!s->glfs) {
         ret = -errno;
@@ -738,6 +762,7 @@  out:
     if (!ret) {
         return ret;
     }
+    g_free(s->logfile);
     if (s->fd) {
         glfs_close(s->fd);
     }
@@ -769,6 +794,8 @@  static int qemu_gluster_reopen_prepare(BDRVReopenState *state,
     gconf = g_new0(BlockdevOptionsGluster, 1);
     gconf->debug_level = s->debug_level;
     gconf->has_debug_level = true;
+    gconf->logfile = g_strdup(s->logfile);
+    gconf->has_logfile = true;
     reop_s->glfs = qemu_gluster_init(gconf, state->bs->filename, NULL, errp);
     if (reop_s->glfs == NULL) {
         ret = -errno;
@@ -914,6 +941,12 @@  static int qemu_gluster_create(const char *filename,
     }
     gconf->has_debug_level = true;
 
+    gconf->logfile = qemu_opt_get_del(opts, GLUSTER_OPT_LOGFILE);
+    if (!gconf->logfile) {
+        gconf->logfile = g_strdup(GLUSTER_LOGFILE_DEFAULT);
+    }
+    gconf->has_logfile = true;
+
     glfs = qemu_gluster_init(gconf, filename, NULL, errp);
     if (!glfs) {
         ret = -errno;
@@ -1025,6 +1058,7 @@  static void qemu_gluster_close(BlockDriverState *bs)
 {
     BDRVGlusterState *s = bs->opaque;
 
+    g_free(s->logfile);
     if (s->fd) {
         glfs_close(s->fd);
         s->fd = NULL;
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 31f9990..59128f7 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2154,13 +2154,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' } }
 
 ##
 # @BlockdevOptions