On Tue, Jan 12, 2016 at 02:24:53PM -0800, William Tu wrote:
> This patch forces appending "parent_process_name(PID)" when invoking
> ovs-vsctl, in order to assist debugging. The patch is for Linux only.
> For example:
> User adds br0 by "ovs-vsctl add-br0", the log shows:
> "ovs-vsctl (invoked by base(1528)): ovs-vsctl add-br br0"
>
> Signed-off-by: William Tu <[email protected]>
Thanks for the new version.
I applied the following incremental to it, for mostly stylistic changes:
* Extra blank line.
* More ifdefs than needed.
* Unused variable 'total_size'.
* VLOG_ERR indicates an error that can't be recovered from, so downgrade
to VLOG_WARN.
* VLOG argument doesn't need trailing \n
* Error log message should include what the error was.
* fread() is overkill for reading a character, that's what getc() is
for.
* We customarily use for (;;) for an infinite loop.
* In testing I found "(invoked by /bin/bash (pid 20216))" easier to read
and clearer than "(invoked by /bin/bash(20216))".
* ds_steal_cstr() doesn't need to be followed by destroying the string
(though it does no harm).
I also changed the log message first line to start with a capital letter
and end with a period, as is my preference.
With those changes, I applied this to master.
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index 801113a..c5a28e0 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -2468,53 +2468,43 @@ run_prerequisites(struct ctl_command *commands, size_t
n_commands,
}
}
-#ifdef __linux__
-
-static char*
+static char *
vsctl_parent_process_info(void)
{
+#ifdef __linux__
pid_t parent_pid;
char *procfile;
- char *msg_ppid;
- char *msg;
struct ds s;
FILE *f;
- size_t total_size = 0;
parent_pid = getppid();
procfile = xasprintf("/proc/%d/cmdline", parent_pid);
f = fopen(procfile, "r");
if (!f) {
- VLOG_ERR("can not open file: %s\n", procfile);
+ VLOG_WARN("%s: open failed (%s)", procfile, ovs_strerror(errno));
free(procfile);
return NULL;
}
+ free(procfile);
ds_init(&s);
-
- while (1) {
- size_t size;
- char c;
- size = fread(&c, 1, 1, f);
- if (c == '\0' || size < 1) {
+ for (;;) {
+ int c = getc(f);
+ if (!c || c == EOF) {
break;
}
ds_put_char(&s, c);
- total_size += size;
}
fclose(f);
- msg_ppid = xasprintf("(%d)", parent_pid);
- ds_put_cstr(&s, msg_ppid);
+ ds_put_format(&s, " (pid %d)", parent_pid);
- msg = ds_steal_cstr(&s);
- free(procfile);
- free(msg_ppid);
- ds_destroy(&s);
- return msg;
-}
+ return ds_steal_cstr(&s);
+#else
+ return NULL;
#endif
+}
static void
do_vsctl(const char *args, struct ctl_command *commands, size_t n_commands,
@@ -2536,19 +2526,14 @@ do_vsctl(const char *args, struct ctl_command
*commands, size_t n_commands,
ovsdb_idl_txn_set_dry_run(txn);
}
-#ifdef __linux__
ppid_info = vsctl_parent_process_info();
if (ppid_info) {
ovsdb_idl_txn_add_comment(txn, "ovs-vsctl (invoked by %s): %s",
ppid_info, args);
free(ppid_info);
- }
- else {
-#endif
+ } else {
ovsdb_idl_txn_add_comment(txn, "ovs-vsctl: %s", args);
-#ifdef __linux__
}
-#endif
ovs = ovsrec_open_vswitch_first(idl);
if (!ovs) {
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
