Libguestfs - Aug 2015 - [PATCH v5 1/2] cat: move get_journal_field to fish/journal.c

There seems to be a minor issue when user wants to run it through pager (more)
and wants cancel it. User will end up with stuck guestfish until journal-view
transfers all journal items.

Output is configurable, it's the same format as virt-log has, since both
uses same code.

Maros Zatko (2):
  cat: move get_journal_field to fish/journal.c
  fish: add journal-view command (RHBZ#988100)

 cat/Makefile.am       |   1 +
 cat/log.c             | 114 ++--------------------------------------
 fish/Makefile.am      |   1 +
 fish/journal.c        | 141 ++++++++++++++++++++++++++++++++++++++++++++++++++
 generator/Makefile.am |   6 ++-
 generator/actions.ml  |  22 ++++++++
 generator/journal.ml  |  97 ++++++++++++++++++++++++++++++++++
 generator/main.ml     |   3 ++
 8 files changed, 272 insertions(+), 113 deletions(-)
 create mode 100644 fish/journal.c
 create mode 100644 generator/journal.ml

-- 
1.9.3

---
 cat/Makefile.am  |   1 +
 cat/log.c        | 114 +-------------------------------------------
 fish/Makefile.am |   1 +
 fish/journal.c   | 142 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 145 insertions(+), 113 deletions(-)
 create mode 100644 fish/journal.c

diff --git a/cat/Makefile.am b/cat/Makefile.am
index d0db6fa..d472100 100644
--- a/cat/Makefile.am
+++ b/cat/Makefile.am
@@ -38,6 +38,7 @@ bin_PROGRAMS = virt-cat virt-filesystems virt-log virt-ls
 SHARED_SOURCE_FILES = \
 	../fish/domain.c \
 	../fish/inspect.c \
+	../fish/journal.c \
 	../fish/keys.c \
 	../fish/options.h \
 	../fish/options.c \
diff --git a/cat/log.c b/cat/log.c
index 616baed..f05eae0 100644
--- a/cat/log.c
+++ b/cat/log.c
@@ -273,128 +273,16 @@ do_log (void)
   return 0;
 }
 
-/* Find the value of the named field from the list of attributes.  If
- * not found, returns NULL (not an error).  If found, returns a
- * pointer to the field, and the length of the field.  NOTE: The field
- * is NOT \0-terminated, so you have to print it using "%.*s".
- *
- * There may be multiple fields with the same name.  In this case, the
- * function returns the first entry.
- */
-static const char *
-get_journal_field (const struct guestfs_xattr_list *xattrs, const char *name,
-                   size_t *len_rtn)
-{
-  uint32_t i;
-
-  for (i = 0; i < xattrs->len; ++i) {
-    if (STREQ (name, xattrs->val[i].attrname)) {
-      *len_rtn = xattrs->val[i].attrval_len;
-      return xattrs->val[i].attrval;
-    }
-  }
-
-  return NULL;                  /* not found */
-}
-
-static const char *const log_level_table[] = {
-  [LOG_EMERG] = "emerg",
-  [LOG_ALERT] = "alert",
-  [LOG_CRIT] = "crit",
-  [LOG_ERR] = "err",
-  [LOG_WARNING] = "warning",
-  [LOG_NOTICE] = "notice",
-  [LOG_INFO] = "info",
-  [LOG_DEBUG] = "debug"
-};
-
 static int
 do_log_journal (void)
 {
-  int r;
-  unsigned errors = 0;
-
   if (guestfs_journal_open (g, JOURNAL_DIR) == -1)
     return -1;
 
-  while ((r = guestfs_journal_next (g)) > 0) {
-    CLEANUP_FREE_XATTR_LIST struct guestfs_xattr_list *xattrs = NULL;
-    const char *priority_str, *identifier, *comm, *pid, *message;
-    size_t priority_len, identifier_len, comm_len, pid_len, message_len;
-    int priority = LOG_INFO;
-    int64_t ts;
-
-    /* The question is what fields to display.  We should probably
-     * make this configurable, but for now use the "short" format
from
-     * journalctl.  (XXX)
-     */
-
-    xattrs = guestfs_journal_get (g);
-    if (xattrs == NULL)
-      return -1;
-
-    ts = guestfs_journal_get_realtime_usec (g); /* error checked below */
-
-    priority_str = get_journal_field (xattrs, "PRIORITY",
&priority_len);
-    //hostname = get_journal_field (xattrs, "_HOSTNAME",
&hostname_len);
-    identifier = get_journal_field (xattrs, "SYSLOG_IDENTIFIER",
-                                    &identifier_len);
-    comm = get_journal_field (xattrs, "_COMM", &comm_len);
-    pid = get_journal_field (xattrs, "_PID", &pid_len);
-    message = get_journal_field (xattrs, "MESSAGE",
&message_len);
-
-    /* Timestamp. */
-    if (ts >= 0) {
-      char buf[64];
-      time_t t = ts / 1000000;
-      struct tm tm;
-
-      if (strftime (buf, sizeof buf, "%b %d %H:%M:%S",
-                    localtime_r (&t, &tm)) <= 0) {
-        fprintf (stderr, _("%s: could not format journal entry
timestamp\n"),
-                 guestfs_int_program_name);
-        errors++;
-        continue;
-      }
-      fputs (buf, stdout);
-    }
-
-    /* Hostname. */
-    /* We don't print this because it is assumed each line from the
-     * guest will have the same hostname.  (XXX)
-     */
-    //if (hostname)
-    //  printf (" %.*s", (int) hostname_len, hostname);
-
-    /* Identifier. */
-    if (identifier)
-      printf (" %.*s", (int) identifier_len, identifier);
-    else if (comm)
-      printf (" %.*s", (int) comm_len, comm);
-
-    /* PID */
-    if (pid)
-      printf ("[%.*s]", (int) pid_len, pid);
-
-    /* Log level. */
-    if (priority_str && *priority_str >= '0' &&
*priority_str <= '7')
-      priority = *priority_str - '0';
-
-    printf (" %s:", log_level_table[priority]);
-
-    /* Message. */
-    if (message)
-      printf (" %.*s", (int) message_len, message);
-
-    printf ("\n");
-  }
-  if (r == -1)                  /* error from guestfs_journal_next */
-    return -1;
-
   if (guestfs_journal_close (g) == -1)
     return -1;
 
-  return errors > 0 ? -1 : 0;
+  return 0;
 }
 
 static int
diff --git a/fish/Makefile.am b/fish/Makefile.am
index c4b82ae..e4b4fcf 100644
--- a/fish/Makefile.am
+++ b/fish/Makefile.am
@@ -94,6 +94,7 @@ guestfish_SOURCES = \
 	glob.c \
 	help.c \
 	hexedit.c \
+	journal.c \
 	lcd.c \
 	man.c \
 	more.c \
diff --git a/fish/journal.c b/fish/journal.c
new file mode 100644
index 0000000..9afdf60
--- /dev/null
+++ b/fish/journal.c
@@ -0,0 +1,142 @@
+/* guestfish - guest filesystem shell
+ * Copyright (C) 2009-2015 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
USA.
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <libintl.h>
+#include <time.h>
+#include <syslog.h>
+
+#include "fish.h"
+#include "journal.h"
+
+/* Find the value of the named field from the list of attributes.  If
+ * not found, returns NULL (not an error).  If found, returns a
+ * pointer to the field, and the length of the field.  NOTE: The field
+ * is NOT \0-terminated, so you have to print it using "%.*s".
+ *
+ * There may be multiple fields with the same name.  In this case, the
+ * function returns the first entry.
+ */
+static const char *
+get_journal_field (const struct guestfs_xattr_list *xattrs, const char *name,
+                   size_t *len_rtn)
+{
+  uint32_t i;
+
+  for (i = 0; i < xattrs->len; ++i) {
+    if (STREQ (name, xattrs->val[i].attrname)) {
+      *len_rtn = xattrs->val[i].attrval_len;
+      return xattrs->val[i].attrval;
+    }
+  }
+
+  return NULL;                  /* not found */
+}
+
+static const char *const log_level_table[] = {
+  [LOG_EMERG] = "emerg",
+  [LOG_ALERT] = "alert",
+  [LOG_CRIT] = "crit",
+  [LOG_ERR] = "err",
+  [LOG_WARNING] = "warning",
+  [LOG_NOTICE] = "notice",
+  [LOG_INFO] = "info",
+  [LOG_DEBUG] = "debug"
+};
+
+static const char *
+lookup_field (char field)
+{
+  size_t i = 0;
+  for (i = 0; i < sizeof journal_fields / sizeof *journal_fields; i += 2) {
+    if (field == journal_fields[i][0])
+      return journal_fields[i+1];
+  }
+  return NULL;
+}
+
+/* Fetch and print journal fields in specified order
+ * default is '~3axv'
+ */
+int
+journal_view (const char *fields)
+{
+  int r;
+  int errors = 0;
+  while ((r = guestfs_journal_next(g)) > 0) {
+    CLEANUP_FREE_XATTR_LIST struct guestfs_xattr_list *xattrs = NULL;
+    int64_t ts;
+    int priority = LOG_INFO;
+
+    xattrs = guestfs_journal_get (g);
+    if (xattrs == NULL)
+      return -1;
+
+    size_t f_id = 0;
+    for (f_id = 0; f_id < strlen (fields); ++f_id) {
+      if (fields[f_id] == '~') {
+        ts = guestfs_journal_get_realtime_usec (g);
+        /* Timestamp. */
+        if (ts >= 0) {
+          char buf[64];
+          time_t t = ts / 1000000;
+          struct tm tm;
+
+          if (strftime (buf, sizeof buf, "%b %d %H:%M:%S",
+                        localtime_r (&t, &tm)) <= 0) {
+            fprintf (stderr, _("could not format journal entry
timestamp\n"));
+            errors++;
+            continue;
+          }
+          fputs (buf, stdout);
+        }
+      }
+
+      const char *field_name, *field_val;
+      size_t field_len;
+      if ((field_name = lookup_field (fields[f_id]))) {
+        field_val = get_journal_field (xattrs, field_name, &field_len);
+        if (STREQ (field_name, "PRIORITY")) {
+          if (field_val && *field_val >= '0' &&
*field_val <= '7')
+            priority = *field_val - '0';
+          printf (" %s:", log_level_table[priority]);
+        } else if (field_val) {
+          printf (" %.*s", (int)field_len, field_val);
+        }
+      }
+    }
+    printf ("\n");
+  }
+
+  return errors > 0 ? -1 : 0;
+}
+
+int
+run_journal_view (const char *cmd, size_t argc, char *argv[])
+{
+  if (argc > 0)
+    return journal_view (argv[0]);
+  return journal_view ("~3axv");
+}
-- 
1.9.3

Lets user view journald log from VM in a similar format as journalctl
uses.

Fixes RFE: journal reader in guestfish
---
 cat/log.c             |  4 +++
 fish/journal.c        |  3 +-
 generator/Makefile.am |  6 ++--
 generator/actions.ml  | 22 ++++++++++++
 generator/journal.ml  | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++
 generator/main.ml     |  3 ++
 6 files changed, 131 insertions(+), 4 deletions(-)
 create mode 100644 generator/journal.ml

diff --git a/cat/log.c b/cat/log.c
index f05eae0..157ee55 100644
--- a/cat/log.c
+++ b/cat/log.c
@@ -36,6 +36,7 @@
 
 #include "guestfs.h"
 #include "options.h"
+#include "journal.h"
 
 /* Currently open libguestfs handle. */
 guestfs_h *g;
@@ -279,6 +280,9 @@ do_log_journal (void)
   if (guestfs_journal_open (g, JOURNAL_DIR) == -1)
     return -1;
 
+  if (journal_view ("~3axv") == -1)
+    return -1;
+
   if (guestfs_journal_close (g) == -1)
     return -1;
 
diff --git a/fish/journal.c b/fish/journal.c
index 9afdf60..579a1d1 100644
--- a/fish/journal.c
+++ b/fish/journal.c
@@ -83,9 +83,8 @@ lookup_field (char field)
 int
 journal_view (const char *fields)
 {
-  int r;
   int errors = 0;
-  while ((r = guestfs_journal_next(g)) > 0) {
+  while (guestfs_journal_next(g) > 0) {
     CLEANUP_FREE_XATTR_LIST struct guestfs_xattr_list *xattrs = NULL;
     int64_t ts;
     int priority = LOG_INFO;
diff --git a/generator/Makefile.am b/generator/Makefile.am
index a3fe50d..bd466c2 100644
--- a/generator/Makefile.am
+++ b/generator/Makefile.am
@@ -37,6 +37,7 @@ sources = \
 	haskell.ml \
 	java.ml \
 	lua.ml \
+	journal.ml \
 	main.ml \
 	ocaml.ml \
 	optgroups.ml \
@@ -60,13 +61,14 @@ sources = \
 objects = \
 	types.cmo \
 	utils.cmo \
+	pr.cmo \
+	docstrings.cmo \
+	journal.cmo \
 	actions.cmo \
 	structs.cmo \
 	optgroups.cmo \
 	prepopts.cmo \
 	events.cmo \
-	pr.cmo \
-	docstrings.cmo \
 	checks.cmo \
 	c.cmo \
 	xdr.cmo \
diff --git a/generator/actions.ml b/generator/actions.ml
index 13c8bc8..e914fd3 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -21,6 +21,8 @@
 open Types
 open Utils
 
+open Journal
+
 (* Default settings for all action fields.  So we copy and override
  * this struct by writing '{ defaults with name = &c }'
  *)
@@ -12897,6 +12899,26 @@ environment variable.
 See also L</hexdump>." };
 
   { defaults with
+    name = "journal_view";
+    shortdesc = "view journald log";
+    longdesc = "  journal-view [FORMAT]
+
+View journald log in format similar to journalctl.
+
+=over
+
+"
+^ (Journal.ops_to_pod_string ()) ^
+"
+=back
+
+Default format is C<~3axv>
+
+For fields description see C<man SYSTEMD.JOURNAL-FIELDS>
+
+Use C<journal-open> first." };
+
+  { defaults with
     name = "lcd";
     shortdesc = "change working directory";
     longdesc = " lcd directory
diff --git a/generator/journal.ml b/generator/journal.ml
new file mode 100644
index 0000000..46d93cd
--- /dev/null
+++ b/generator/journal.ml
@@ -0,0 +1,97 @@
+(* libguestfs
+ * Copyright (C) 2015 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ *)
+
+(* Please read generator/README first. *)
+
+open Printf
+
+open String
+open Docstrings
+open Pr
+
+open Char
+open List
+
+(* Arguments used by journal-view command *)
+
+type op_type = Options of (char * string) list;;       (* option, option name*)
+
+let op = Options [
+  (* Trusted fields *)
+  ('a', "_PID");
+  ('b', "_UID");
+  ('c', "_GID");
+  ('d', "_COMM");
+  ('e', "_EXE");
+  ('f', "_CMDLINE");
+  ('g', "_CAP_EFFECTIVE");
+  ('h', "_AUDIT_SESSION");
+  ('i', "_AUDIT_LOGINUID");
+  ('j', "_SYSTEMD_CGROUP");
+  ('k', "_SYSTEMD_SESSION");
+  ('l', "_SYSTEMD_UNIT");
+  ('m', "_SYSTEMD_USER_UNIT");
+  ('n', "_SYSTEMD_OWNER_UID");
+  ('o', "_SYSTEMD_SLICE");
+  ('p', "_SELINUX_CONTEXT");
+  ('q', "_SOURCE_REALTIME_TIMESTAMP");
+  ('r', "_BOOT_ID");
+  ('s', "_MACHINE_ID");
+  ('t', "_HOSTNAME");
+  ('u', "_TRANSPORT");
+  (* User fields *)
+  ('v', "MESSAGE");
+  ('w', "MESSAGE_ID");
+  ('x', "PRIORITY");
+  ('y', "CODE_FILE");
+  ('z', "CODE_LINE");
+  ('0', "CODE_FUNC");
+  ('1', "ERRNO");
+  ('2', "SYSLOG_FACILITY");
+  ('3', "SYSLOG_IDENTIFIER");
+  ('4', "SYSLOG_PID");
+  ('~', "timestamp")
+];;
+
+let deopt x = match x with Options y -> y;;
+
+let string_list_to_string s = List.fold_left (fun a b -> a ^ b) ""
s;;
+
+let ops_to_pod_string () +  List.fold_left (fun a b -> a ^ b) ""
+    (List.map (fun (a,b) -> "=item " ^ escaped a ^ " " ^
b ^ "\n\n")
+      (deopt op));;
+
+let ops_to_c_array arrname () +  let decl = "static const char *const
" ^ arrname ^ "[] = {\n" in
+  decl ^ String.concat ",\n"
+    (List.map (fun (a,b) -> "  \"" ^ escaped a ^
"\", \"" ^ b ^ "\"")
+      (deopt op)) ^ "\n};";;
+
+let generate_journal_h () +  generate_header CStyle LGPLv2plus;
+  pr "#include <config.h>\n";
+  pr "\n";
+  pr "#ifndef JOURNAL_H\n";
+  pr "#define JOURNAL_H\n";
+  pr "\n";
+  pr "%s\n" (ops_to_c_array "journal_fields" ());
+  pr "\n";
+  pr "/* in journal.c */\n";
+  pr "extern int journal_view (const char *fields);\n";
+  pr "#endif /* JOURNAL_H */\n";
diff --git a/generator/main.ml b/generator/main.ml
index 1e0e7d6..639920d 100644
--- a/generator/main.ml
+++ b/generator/main.ml
@@ -46,6 +46,7 @@ open Golang
 open Bindtests
 open Errnostring
 open Customize
+open Journal
 
 let perror msg = function
   | Unix_error (err, _, _) ->
@@ -212,6 +213,8 @@ Run it from the top source directory using the command
   output_to "customize/customize-synopsis.pod"
generate_customize_synopsis_pod;
   output_to "customize/customize-options.pod"
generate_customize_options_pod;
 
+  output_to "fish/journal.h" generate_journal_h;
+
   (* Generate the list of files generated -- last. *)
   printf "generated %d lines of code\n" (get_lines_generated ());
   let files = List.sort compare (get_files_generated ()) in
-- 
1.9.3

Hi,

a general note on the patches: please make sure everything builds and
works after applying each patch. Otherwise, bisecting code or simply
checking each works is not possible.

In this specific case, patch #1 will not build, since fish/journal.h
is being created with code in patch #2, and won't work either, since
do_log_journal does not call the new code.

In data lunedì 31 agosto 2015 15:07:35, Maros Zatko ha
scritto:
> There seems to be a minor issue when user wants to run it through pager
(more)
> and wants cancel it. User will end up with stuck guestfish until
journal-view
> transfers all journal items.
An option could be hooking to the current SIGINT handling in fish.c,
and canceling the dump of journal entries.
> Output is configurable, it's the same format as virt-log has, since
both
> uses same code.
> 
> Maros Zatko (2):
>   cat: move get_journal_field to fish/journal.c
>   fish: add journal-view command (RHBZ#988100)
> 
>  cat/Makefile.am       |   1 +
>  cat/log.c             | 114 ++--------------------------------------
>  fish/Makefile.am      |   1 +
>  fish/journal.c        | 141
++++++++++++++++++++++++++++++++++++++++++++++++++
>  generator/Makefile.am |   6 ++-
>  generator/actions.ml  |  22 ++++++++
>  generator/journal.ml  |  97 ++++++++++++++++++++++++++++++++++
>  generator/main.ml     |   3 ++
>  8 files changed, 272 insertions(+), 113 deletions(-)
>  create mode 100644 fish/journal.c
>  create mode 100644 generator/journal.ml
Thanks,
-- 
Pino Toscano

In data lunedì 31 agosto 2015 15:07:36, Maros Zatko ha
scritto:
> ---
>  cat/Makefile.am  |   1 +
>  cat/log.c        | 114 +-------------------------------------------
>  fish/Makefile.am |   1 +
>  fish/journal.c   | 142
+++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 145 insertions(+), 113 deletions(-)
>  create mode 100644 fish/journal.c
> 
> diff --git a/cat/Makefile.am b/cat/Makefile.am
> index d0db6fa..d472100 100644
> --- a/cat/Makefile.am
> +++ b/cat/Makefile.am
> @@ -38,6 +38,7 @@ bin_PROGRAMS = virt-cat virt-filesystems virt-log virt-ls
>  SHARED_SOURCE_FILES = \
>  	../fish/domain.c \
>  	../fish/inspect.c \
> +	../fish/journal.c \
>  	../fish/keys.c \
>  	../fish/options.h \
>  	../fish/options.c \
> diff --git a/cat/log.c b/cat/log.c
> index 616baed..f05eae0 100644
> --- a/cat/log.c
> +++ b/cat/log.c
> @@ -273,128 +273,16 @@ do_log (void)
>    return 0;
>  }
>  
> -/* Find the value of the named field from the list of attributes.  If
> - * not found, returns NULL (not an error).  If found, returns a
> - * pointer to the field, and the length of the field.  NOTE: The field
> - * is NOT \0-terminated, so you have to print it using "%.*s".
> - *
> - * There may be multiple fields with the same name.  In this case, the
> - * function returns the first entry.
> - */
> -static const char *
> -get_journal_field (const struct guestfs_xattr_list *xattrs, const char
*name,
> -                   size_t *len_rtn)
> -{
> -  uint32_t i;
> -
> -  for (i = 0; i < xattrs->len; ++i) {
> -    if (STREQ (name, xattrs->val[i].attrname)) {
> -      *len_rtn = xattrs->val[i].attrval_len;
> -      return xattrs->val[i].attrval;
> -    }
> -  }
> -
> -  return NULL;                  /* not found */
> -}
> -
> -static const char *const log_level_table[] = {
> -  [LOG_EMERG] = "emerg",
> -  [LOG_ALERT] = "alert",
> -  [LOG_CRIT] = "crit",
> -  [LOG_ERR] = "err",
> -  [LOG_WARNING] = "warning",
> -  [LOG_NOTICE] = "notice",
> -  [LOG_INFO] = "info",
> -  [LOG_DEBUG] = "debug"
> -};
> -
>  static int
>  do_log_journal (void)
>  {
> -  int r;
> -  unsigned errors = 0;
> -
>    if (guestfs_journal_open (g, JOURNAL_DIR) == -1)
>      return -1;
>  
> -  while ((r = guestfs_journal_next (g)) > 0) {
> -    CLEANUP_FREE_XATTR_LIST struct guestfs_xattr_list *xattrs = NULL;
> -    const char *priority_str, *identifier, *comm, *pid, *message;
> -    size_t priority_len, identifier_len, comm_len, pid_len, message_len;
> -    int priority = LOG_INFO;
> -    int64_t ts;
> -
> -    /* The question is what fields to display.  We should probably
> -     * make this configurable, but for now use the "short"
format from
> -     * journalctl.  (XXX)
> -     */
> -
> -    xattrs = guestfs_journal_get (g);
> -    if (xattrs == NULL)
> -      return -1;
> -
> -    ts = guestfs_journal_get_realtime_usec (g); /* error checked below */
> -
> -    priority_str = get_journal_field (xattrs, "PRIORITY",
&priority_len);
> -    //hostname = get_journal_field (xattrs, "_HOSTNAME",
&hostname_len);
> -    identifier = get_journal_field (xattrs, "SYSLOG_IDENTIFIER",
> -                                    &identifier_len);
> -    comm = get_journal_field (xattrs, "_COMM", &comm_len);
> -    pid = get_journal_field (xattrs, "_PID", &pid_len);
> -    message = get_journal_field (xattrs, "MESSAGE",
&message_len);
> -
> -    /* Timestamp. */
> -    if (ts >= 0) {
> -      char buf[64];
> -      time_t t = ts / 1000000;
> -      struct tm tm;
> -
> -      if (strftime (buf, sizeof buf, "%b %d %H:%M:%S",
> -                    localtime_r (&t, &tm)) <= 0) {
> -        fprintf (stderr, _("%s: could not format journal entry
timestamp\n"),
> -                 guestfs_int_program_name);
> -        errors++;
> -        continue;
> -      }
> -      fputs (buf, stdout);
> -    }
> -
> -    /* Hostname. */
> -    /* We don't print this because it is assumed each line from the
> -     * guest will have the same hostname.  (XXX)
> -     */
> -    //if (hostname)
> -    //  printf (" %.*s", (int) hostname_len, hostname);
> -
> -    /* Identifier. */
> -    if (identifier)
> -      printf (" %.*s", (int) identifier_len, identifier);
> -    else if (comm)
> -      printf (" %.*s", (int) comm_len, comm);
> -
> -    /* PID */
> -    if (pid)
> -      printf ("[%.*s]", (int) pid_len, pid);
> -
> -    /* Log level. */
> -    if (priority_str && *priority_str >= '0' &&
*priority_str <= '7')
> -      priority = *priority_str - '0';
> -
> -    printf (" %s:", log_level_table[priority]);
> -
> -    /* Message. */
> -    if (message)
> -      printf (" %.*s", (int) message_len, message);
> -
> -    printf ("\n");
> -  }
> -  if (r == -1)                  /* error from guestfs_journal_next */
> -    return -1;
> -
>    if (guestfs_journal_close (g) == -1)
>      return -1;
>  
> -  return errors > 0 ? -1 : 0;
> +  return 0;
>  }
>  
>  static int
> diff --git a/fish/Makefile.am b/fish/Makefile.am
> index c4b82ae..e4b4fcf 100644
> --- a/fish/Makefile.am
> +++ b/fish/Makefile.am
> @@ -94,6 +94,7 @@ guestfish_SOURCES = \
>  	glob.c \
>  	help.c \
>  	hexedit.c \
> +	journal.c \
>  	lcd.c \
>  	man.c \
>  	more.c \
> diff --git a/fish/journal.c b/fish/journal.c
> new file mode 100644
> index 0000000..9afdf60
> --- /dev/null
> +++ b/fish/journal.c
> @@ -0,0 +1,142 @@
> +/* guestfish - guest filesystem shell
> + * Copyright (C) 2009-2015 Red Hat Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
02110-1301 USA.
> + */
> +
> +#include <config.h>
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <fcntl.h>
> +#include <inttypes.h>
> +#include <libintl.h>
> +#include <time.h>
> +#include <syslog.h>
> +
> +#include "fish.h"
This will create issues (I guess) when using journal.c as source in
virt-cat. I imagine you did that because of the declaration of 'g',
so pass it as argument to journal_view, like I did for fish/windows.h.
> +#include "journal.h"
> +
> +/* Find the value of the named field from the list of attributes.  If
> + * not found, returns NULL (not an error).  If found, returns a
> + * pointer to the field, and the length of the field.  NOTE: The field
> + * is NOT \0-terminated, so you have to print it using "%.*s".
> + *
> + * There may be multiple fields with the same name.  In this case, the
> + * function returns the first entry.
> + */
> +static const char *
> +get_journal_field (const struct guestfs_xattr_list *xattrs, const char
*name,
> +                   size_t *len_rtn)
> +{
> +  uint32_t i;
> +
> +  for (i = 0; i < xattrs->len; ++i) {
> +    if (STREQ (name, xattrs->val[i].attrname)) {
> +      *len_rtn = xattrs->val[i].attrval_len;
> +      return xattrs->val[i].attrval;
> +    }
> +  }
> +
> +  return NULL;                  /* not found */
> +}
> +
> +static const char *const log_level_table[] = {
> +  [LOG_EMERG] = "emerg",
> +  [LOG_ALERT] = "alert",
> +  [LOG_CRIT] = "crit",
> +  [LOG_ERR] = "err",
> +  [LOG_WARNING] = "warning",
> +  [LOG_NOTICE] = "notice",
> +  [LOG_INFO] = "info",
> +  [LOG_DEBUG] = "debug"
> +};
> +
> +static const char *
> +lookup_field (char field)
> +{
> +  size_t i = 0;
> +  for (i = 0; i < sizeof journal_fields / sizeof *journal_fields; i +=
2) {
> +    if (field == journal_fields[i][0])
> +      return journal_fields[i+1];
> +  }
> +  return NULL;
> +}
> +
> +/* Fetch and print journal fields in specified order
> + * default is '~3axv'
> + */
> +int
> +journal_view (const char *fields)
> +{
> +  int r;
> +  int errors = 0;
> +  while ((r = guestfs_journal_next(g)) > 0) {
> +    CLEANUP_FREE_XATTR_LIST struct guestfs_xattr_list *xattrs = NULL;
> +    int64_t ts;
> +    int priority = LOG_INFO;
> +
> +    xattrs = guestfs_journal_get (g);
> +    if (xattrs == NULL)
> +      return -1;
> +
> +    size_t f_id = 0;
> +    for (f_id = 0; f_id < strlen (fields); ++f_id) {
> +      if (fields[f_id] == '~') {
> +        ts = guestfs_journal_get_realtime_usec (g);
> +        /* Timestamp. */
> +        if (ts >= 0) {
> +          char buf[64];
> +          time_t t = ts / 1000000;
> +          struct tm tm;
> +
> +          if (strftime (buf, sizeof buf, "%b %d %H:%M:%S",
> +                        localtime_r (&t, &tm)) <= 0) {
> +            fprintf (stderr, _("could not format journal entry
timestamp\n"));
> +            errors++;
> +            continue;
> +          }
> +          fputs (buf, stdout);
> +        }
> +      }
Optimization: the rest of the code can be put in an else of this if.
> +
> +      const char *field_name, *field_val;
> +      size_t field_len;
> +      if ((field_name = lookup_field (fields[f_id]))) {
If possible, avoid conditions with side effects -- assign field_name
outside the if.
> +        field_val = get_journal_field (xattrs, field_name,
&field_len);
> +        if (STREQ (field_name, "PRIORITY")) {
> +          if (field_val && *field_val >= '0' &&
*field_val <= '7')
> +            priority = *field_val - '0';
> +          printf (" %s:", log_level_table[priority]);
> +        } else if (field_val) {
> +          printf (" %.*s", (int)field_len, field_val);
> +        }
> +      }
Should there be some warning about unknown fields in the format string?
> +    }
> +    printf ("\n");
> +  }
> +
> +  return errors > 0 ? -1 : 0;
> +}
> +
> +int
> +run_journal_view (const char *cmd, size_t argc, char *argv[])
> +{
> +  if (argc > 0)
> +    return journal_view (argv[0]);
> +  return journal_view ("~3axv");
> +}
> 
Thanks,
-- 
Pino Toscano

In data lunedì 31 agosto 2015 15:07:37, Maros Zatko ha
scritto:
> Lets user view journald log from VM in a similar format as journalctl
> uses.
"Let the user view the journald log from a guest, with a format similar
to what journalctl uses."
> 
> Fixes RFE: journal reader in guestfish
> ---
>  cat/log.c             |  4 +++
>  fish/journal.c        |  3 +-
>  generator/Makefile.am |  6 ++--
>  generator/actions.ml  | 22 ++++++++++++
>  generator/journal.ml  | 97
+++++++++++++++++++++++++++++++++++++++++++++++++++
>  generator/main.ml     |  3 ++
>  6 files changed, 131 insertions(+), 4 deletions(-)
>  create mode 100644 generator/journal.ml
> 
> diff --git a/cat/log.c b/cat/log.c
> index f05eae0..157ee55 100644
> --- a/cat/log.c
> +++ b/cat/log.c
> @@ -36,6 +36,7 @@
>  
>  #include "guestfs.h"
>  #include "options.h"
> +#include "journal.h"
>  
>  /* Currently open libguestfs handle. */
>  guestfs_h *g;
> @@ -279,6 +280,9 @@ do_log_journal (void)
>    if (guestfs_journal_open (g, JOURNAL_DIR) == -1)
>      return -1;
>  
> +  if (journal_view ("~3axv") == -1)
> +    return -1;
> +
>    if (guestfs_journal_close (g) == -1)
>      return -1;
>  
This should be part of patch #1 if possible.
> diff --git a/fish/journal.c b/fish/journal.c
> index 9afdf60..579a1d1 100644
> --- a/fish/journal.c
> +++ b/fish/journal.c
> @@ -83,9 +83,8 @@ lookup_field (char field)
>  int
>  journal_view (const char *fields)
>  {
> -  int r;
>    int errors = 0;
> -  while ((r = guestfs_journal_next(g)) > 0) {
> +  while (guestfs_journal_next(g) > 0) {
>      CLEANUP_FREE_XATTR_LIST struct guestfs_xattr_list *xattrs = NULL;
>      int64_t ts;
>      int priority = LOG_INFO;
Ditto.
> diff --git a/generator/Makefile.am b/generator/Makefile.am
> index a3fe50d..bd466c2 100644
> --- a/generator/Makefile.am
> +++ b/generator/Makefile.am
> @@ -37,6 +37,7 @@ sources = \
>  	haskell.ml \
>  	java.ml \
>  	lua.ml \
> +	journal.ml \
>  	main.ml \
>  	ocaml.ml \
>  	optgroups.ml \
> @@ -60,13 +61,14 @@ sources = \
>  objects = \
>  	types.cmo \
>  	utils.cmo \
> +	pr.cmo \
> +	docstrings.cmo \
> +	journal.cmo \
>  	actions.cmo \
>  	structs.cmo \
>  	optgroups.cmo \
>  	prepopts.cmo \
>  	events.cmo \
> -	pr.cmo \
> -	docstrings.cmo \
>  	checks.cmo \
>  	c.cmo \
>  	xdr.cmo \
> diff --git a/generator/actions.ml b/generator/actions.ml
> index 13c8bc8..e914fd3 100644
> --- a/generator/actions.ml
> +++ b/generator/actions.ml
> @@ -21,6 +21,8 @@
>  open Types
>  open Utils
>  
> +open Journal
> +
>  (* Default settings for all action fields.  So we copy and override
>   * this struct by writing '{ defaults with name = &c }'
>   *)
> @@ -12897,6 +12899,26 @@ environment variable.
>  See also L</hexdump>." };
>  
>    { defaults with
> +    name = "journal_view";
> +    shortdesc = "view journald log";
> +    longdesc = "  journal-view [FORMAT]
> +
> +View journald log in format similar to journalctl.
"journalctl" could be linkified, so L<journalctl(1)>.
> +
> +=over
> +
> +"
> +^ (Journal.ops_to_pod_string ()) ^
> +"
> +=back
> +
> +Default format is C<~3axv>
"The default format is C<~3axv>."
> +
> +For fields description see C<man SYSTEMD.JOURNAL-FIELDS>
There's the proper format for linking man pages, so it should be
L<systemd.journal-fields(7)>.
> +
> +Use C<journal-open> first." };
C<guestfs_journal_open> here, so it gets proper linking.
> +
> +  { defaults with
>      name = "lcd";
>      shortdesc = "change working directory";
>      longdesc = " lcd directory
> diff --git a/generator/journal.ml b/generator/journal.ml
> new file mode 100644
> index 0000000..46d93cd
> --- /dev/null
> +++ b/generator/journal.ml
> @@ -0,0 +1,97 @@
> +(* libguestfs
> + * Copyright (C) 2015 Red Hat Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
02110-1301 USA
> + *)
> +
> +(* Please read generator/README first. *)
> +
> +open Printf
> +
> +open String
> +open Docstrings
> +open Pr
> +
> +open Char
> +open List
I guess you don't need to open List, if later you always use it like
List.foo.
> +
> +(* Arguments used by journal-view command *)
> +
> +type op_type = Options of (char * string) list;;       (* option, option
name*)
What is Options? I cannot find it neither in the OCaml standard library
nor in the generator own types...

As a style note, usually ;; at the end of top-level statements are not
used in libguestfs' code.
> +let op = Options [
> +  (* Trusted fields *)
> +  ('a', "_PID");
> +  ('b', "_UID");
> +  ('c', "_GID");
> +  ('d', "_COMM");
> +  ('e', "_EXE");
> +  ('f', "_CMDLINE");
> +  ('g', "_CAP_EFFECTIVE");
> +  ('h', "_AUDIT_SESSION");
> +  ('i', "_AUDIT_LOGINUID");
> +  ('j', "_SYSTEMD_CGROUP");
> +  ('k', "_SYSTEMD_SESSION");
> +  ('l', "_SYSTEMD_UNIT");
> +  ('m', "_SYSTEMD_USER_UNIT");
> +  ('n', "_SYSTEMD_OWNER_UID");
> +  ('o', "_SYSTEMD_SLICE");
> +  ('p', "_SELINUX_CONTEXT");
> +  ('q', "_SOURCE_REALTIME_TIMESTAMP");
> +  ('r', "_BOOT_ID");
> +  ('s', "_MACHINE_ID");
> +  ('t', "_HOSTNAME");
> +  ('u', "_TRANSPORT");
> +  (* User fields *)
> +  ('v', "MESSAGE");
> +  ('w', "MESSAGE_ID");
> +  ('x', "PRIORITY");
> +  ('y', "CODE_FILE");
> +  ('z', "CODE_LINE");
> +  ('0', "CODE_FUNC");
> +  ('1', "ERRNO");
> +  ('2', "SYSLOG_FACILITY");
> +  ('3', "SYSLOG_IDENTIFIER");
> +  ('4', "SYSLOG_PID");
> +  ('~', "timestamp")
> +];;
> +
> +let deopt x = match x with Options y -> y;;
Considering you always call deopt on 'op', declaring it as
(char * string) list should be enough, I guess.
> +let string_list_to_string s = List.fold_left (fun a b -> a ^ b)
"" s;;
Using String.concat would make it easier to read/understand... although
seems not used at all.
> +let ops_to_pod_string () > +  List.fold_left (fun a b -> a ^ b)
""
String.concat here would be easier.
> +    (List.map (fun (a,b) -> "=item " ^ escaped a ^ "
" ^ b ^ "\n\n")
> +      (deopt op));;
> +
> +let ops_to_c_array arrname () > +  let decl = "static const char
*const " ^ arrname ^ "[] = {\n" in
Considering it does not change, you could put the array opening and
closing directly in the printed C code.
> +  decl ^ String.concat ",\n"
> +    (List.map (fun (a,b) -> "  \"" ^ escaped a ^
"\", \"" ^ b ^ "\"")
> +      (deopt op)) ^ "\n};";;
IMHO this could be inline in generate_journal_h, just using List.iter
to print each line in the array.
> +let generate_journal_h () > +  generate_header CStyle LGPLv2plus;
> +  pr "#include <config.h>\n";
> +  pr "\n";
> +  pr "#ifndef JOURNAL_H\n";
> +  pr "#define JOURNAL_H\n";
> +  pr "\n";
> +  pr "%s\n" (ops_to_c_array "journal_fields" ());
> +  pr "\n";
> +  pr "/* in journal.c */\n";
> +  pr "extern int journal_view (const char *fields);\n";
> +  pr "#endif /* JOURNAL_H */\n";
The generated code here could be better: instead of generating an array
each field code (as string) and name one after the other, have a list
of structs:

 static const struct JournalField {
   char field;
   const char *name;
 } fields[] = {
   { 'a', "_PID" },
   ...
 };
>From what I see in these patches, it seems like the array with fields
is just an implementation detail of journal_view, and thus not worth
exposing in journal.h. Maybe generate a journal-fields.h (or even .c)
to include directly in journal.c, so journal.h can stay as
non-generated file.

Thanks,
-- 
Pino Toscano