diff mbox

[ovs-dev,v2,1/2] ovs-vsctl: add parent process name and ID

Message ID 1452626932-106395-1-git-send-email-u9012063@gmail.com
State Changes Requested
Headers show

Commit Message

William Tu Jan. 12, 2016, 7:28 p.m. UTC 
This patch forces appending "-- comment parent_process_name(PID)"
when invoking ovs-vsctl, in order to assist debugging.
For example:
    User adds br0 by "ovs-vsctl add-br0", the log shows:
    "ovs-vsctl: ovs-vsctl add-br br0 -- comment bash(1528)"

Signed-off-by: William Tu <u9012063@gmail.com>
---
 utilities/ovs-vsctl.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)

Comments

Ben Pfaff Jan. 12, 2016, 7:34 p.m. UTC | #1 
On Tue, Jan 12, 2016 at 11:28:51AM -0800, William Tu wrote:
> This patch forces appending "-- comment parent_process_name(PID)"
> when invoking ovs-vsctl, in order to assist debugging.
> For example:
>     User adds br0 by "ovs-vsctl add-br0", the log shows:
>     "ovs-vsctl: ovs-vsctl add-br br0 -- comment bash(1528)"
> 
> Signed-off-by: William Tu <u9012063@gmail.com>

I'd change this from "not-Windows" to "Linux-only", because *BSD
etc. don't have Linux-like /proc.

Also I think you need to #if-out the function instead of just returning
NULL because I don't think that Windows has getppid().

> -    ovsdb_idl_txn_add_comment(txn, "ovs-vsctl: %s", args);
> +    ppid_info = vsctl_parent_process_info();
> +    if (ppid_info) {
> +        ovsdb_idl_txn_add_comment(txn, "ovs-vsctl: %s -- comment %s",
> +                                  args, ppid_info);

I don't think there's value in making the parent process look like an
ovs-vsctl command, in fact it might be confusing in the log.  Perhaps
something like "ovs-vsctl (invoked by %s): %s".

Thanks,

Ben.
diff mbox

Patch

diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index 36290db..4dbfbe6 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -2468,6 +2468,48 @@  run_prerequisites(struct ctl_command *commands, size_t n_commands,
     }
 }
 
+static char*
+vsctl_parent_process_info(void)
+{
+    pid_t parent_pid;
+    char *procfile;
+    char *msg_ppid;
+    char *msg;
+    struct ds s;
+    FILE *f;
+    size_t total_size = 0;
+
+#if defined(_WIN32) || defined (_WIN64)
+    return NULL;
+#endif
+
+    ds_init(&s);
+    parent_pid = getppid();
+    procfile = xasprintf("/proc/%d/cmdline", parent_pid);
+
+    f = fopen(procfile, "r");
+    while (1) {
+        size_t size;
+        char c;
+        size = fread(&c, 1, 1, f);
+		if (c == '\0' || size < 1) {
+            break;
+        }
+        ds_put_char(&s, c);
+        total_size += size;
+    }
+    fclose(f);
+
+    msg_ppid = xasprintf("(%d)", parent_pid);
+    ds_put_cstr(&s, msg_ppid);
+
+    msg = ds_steal_cstr(&s);
+    free(procfile);
+    free(msg_ppid);
+    ds_destroy(&s);
+    return msg;
+}
+
 static void
 do_vsctl(const char *args, struct ctl_command *commands, size_t n_commands,
          struct ovsdb_idl *idl)
@@ -2481,13 +2523,22 @@  do_vsctl(const char *args, struct ctl_command *commands, size_t n_commands,
     struct shash_node *node;
     int64_t next_cfg = 0;
     char *error = NULL;
+    char *ppid_info = NULL;
 
     txn = the_idl_txn = ovsdb_idl_txn_create(idl);
     if (dry_run) {
         ovsdb_idl_txn_set_dry_run(txn);
     }
 
-    ovsdb_idl_txn_add_comment(txn, "ovs-vsctl: %s", args);
+    ppid_info = vsctl_parent_process_info();
+    if (ppid_info) {
+        ovsdb_idl_txn_add_comment(txn, "ovs-vsctl: %s -- comment %s",
+                                  args, ppid_info);
+        free(ppid_info);
+    }
+    else {
+        ovsdb_idl_txn_add_comment(txn, "ovs-vsctl: %s", args);
+    }
 
     ovs = ovsrec_open_vswitch_first(idl);
     if (!ovs) {