Looks good to me. Thanks for getting this in.
Ethan
On Jul 17, 2012, at 12:12, Ben Pfaff <b...@nicira.com> wrote:
> On Tue, Jul 17, 2012 at 09:28:09AM -0700, Ethan Jackson wrote:
>> Commit a890678229 (userspace: Log version on startup.) added
>> additional logging to ovsdb-server and ovs-vswitchd, but failed to
>> make certain the unit tests still passed.
>>
>> Signed-off-by: Ethan Jackson <et...@nicira.com>
>
> Thanks.
>
> We have too many "ignore"s already in the unit tests, so I don't
> really want to add more. Looking closer, the real issue here, it
> seems to me, is that "ovsdb-server --run" is quite different from
> ovsdb-server's normal mode of operation. It's not using ovsdb-server
> as a daemon but as a fairly ordinary command-line tool. In that case
> I think it's better to just not log the message.
>
> So I made that change and I'll push the following soon, along with
> patches 1 and 3.
>
> (For anyone wondering why I'm shepherding this is, Ethan's on a plane
> at the moment and asked me to.)
>
> --8<--------------------------cut here-------------------------->8--
>
> From: Ethan Jackson <et...@nicira.com>
> Date: Tue, 17 Jul 2012 10:07:36 -0700
> Subject: [PATCH] tests: Fix unit test failures related to additional logging.
>
> Commit a890678229 (userspace: Log version on startup.) added
> additional logging to ovsdb-server and ovs-vswitchd, but failed to
> make certain the unit tests still passed.
>
> Signed-off-by: Ethan Jackson <et...@nicira.com>
> [b...@nicira.com changed the strategy for fixing ovsdb-server.at]
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> ovsdb/ovsdb-server.c | 7 ++++++-
> tests/ofproto-macros.at | 5 ++++-
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
> index b430b0e..139acc6 100644
> --- a/ovsdb/ovsdb-server.c
> +++ b/ovsdb/ovsdb-server.c
> @@ -137,7 +137,12 @@ main(int argc, char *argv[])
>
> daemonize_complete();
>
> - VLOG_INFO("%s (Open vSwitch) %s", program_name, VERSION);
> + if (!run_command) {
> + /* ovsdb-server is usually a long-running process, in which case it
> + * makes plenty of sense to log the version, but --run makes
> + * ovsdb-server more like a command-line tool, so skip it. */
> + VLOG_INFO("%s (Open vSwitch) %s", program_name, VERSION);
> + }
>
> unixctl_command_register("exit", "", 0, 0, ovsdb_server_exit, &exiting);
> unixctl_command_register("ovsdb-server/compact", "", 0, 0,
> diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
> index b0c4fa0..e7d6da4 100644
> --- a/tests/ofproto-macros.at
> +++ b/tests/ofproto-macros.at
> @@ -41,7 +41,9 @@ m4_define([OVS_VSWITCHD_START],
>
> dnl Start ovsdb-server.
> AT_CHECK([ovsdb-server --detach --pidfile --log-file
> --remote=punix:$OVS_RUNDIR/db.sock], [0], [], [stderr])
> - AT_CHECK([[sed < stderr '/vlog|INFO|opened log file/d']])
> + AT_CHECK([[sed < stderr '
> +/vlog|INFO|opened log file/d
> +/ovsdb_server|INFO|ovsdb-server (Open vSwitch)/d']])
> AT_CAPTURE_FILE([ovsdb-server.log])
>
> dnl Initialize database.
> @@ -52,6 +54,7 @@ m4_define([OVS_VSWITCHD_START],
> AT_CAPTURE_FILE([ovs-vswitchd.log])
> AT_CHECK([[sed < stderr '
> /vlog|INFO|opened log file/d
> +/vswitchd|INFO|ovs-vswitchd (Open vSwitch)/d
> /reconnect|INFO|/d
> /ofproto|INFO|using datapath ID/d
> /ofproto|INFO|datapath ID changed to fedcba9876543210/d']])
> --
> 1.7.2.5
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
