On Sun, Mar 22, 2015 at 04:38:50PM -0400, Russell Bryant wrote:
> ovn-nbctl is intended to be a utility for manually interacting with
> the OVN northbound db.  A real consumer of OVN, such as OpenStack,
> should be using ovsdb directly.  However, a utility like this is
> useful during development and testing of both ovn itself as well as
> the OpenStack Neutron plugin.  It could also be used when debugging
> the state of an ovn deployment.
> 
> The commands related to logical switches and ports have been
> implemented.  You can add, delete, list, and get/set external:ids.
> On ports you can also get/set MAC addresses.
> 
> Signed-off-by: Russell Bryant <rbry...@redhat.com>
> ---
> 
> 
> v1
>  - first cut of lswitch-* commands
> v2
>  - fix handling of multiple switches with the same name
> v3
>  - add -d/--database option
>  - add lport-* commands
> v4
>  - add --database option to support a non-default or remote db
>  - set up and use vlog
>  - add tls options
>  - other minor cleanups
>  - (I think I'm actually done for now this time ...)

Thanks a lot!

One thing we'll have to think about here is clean transactions.  The
code as-is does not use any "verify" operations to make sure that the
overall transaction is atomic.  This might not be a big deal for
ovn-nbctl, and it definitely is not yet, but it is worth thinking about
(especially since OpenStack will want atomic transactions so if we catch
any issues soon then we'll be better prepared for OpenStack's real use
case).

I noticed that some of the VLOG invocations put a new-line at the end.
That's not necessary, although it's harmless, and our convention is to
leave it out, so I removed them.

I noticed a few long lines, so I wrapped them.

The manpage needed some font adjustments, so I did that.

I also changed the subject line of the commit message to start with a
capital letter and end with a period, because for whatever reason that's
my preferred convention in OVS.

With those changes I applied this to the ovn branch.  Thanks again!

diff --git a/ovn/ovn-nbctl.8.xml b/ovn/ovn-nbctl.8.xml
index dad5e9a..54f9620 100644
--- a/ovn/ovn-nbctl.8.xml
+++ b/ovn/ovn-nbctl.8.xml
@@ -4,42 +4,42 @@
     <p>ovn-nbctl -- Open Virtual Network northbound db management utility</p>
 
     <h1>Synopsys</h1>
-    <p>ovn-nbctl [OPTIONS] COMMAND [ARG...]</p>
+    <p><code>ovn-nbctl</code> [<var>options</var>] <var>command</var> 
[<var>arg</var>...]</p>
 
     <h1>Description</h1>
     <p>This utility can be used to manage the OVN northbound database.</p>
 
     <h1>Logical Switch Commands</h1>
-    <p>lswitch-add [name]</p>
-    <p>lswitch-del &lt;lswitch&gt;</p>
-    <p>lswitch-list</p>
-    <p>lswitch-set-external-id &lt;lswitch&gt; &lt;key&gt; [value]</p>
-    <p>lswitch-get-external-id &lt;lswitch&gt; [key]</p>
+    <p><code>lswitch-add</code> [<var>name</var>]</p>
+    <p><code>lswitch-del</code> <var>lswitch</var></p>
+    <p><code>lswitch-list</code></p>
+    <p><code>lswitch-set-external-id</code> <var>lswitch</var> <var>key</var> 
[<var>value</var>]</p>
+    <p><code>lswitch-get-external-id</code> <var>lswitch</var> 
[<var>key</var>]</p>
 
     <h1>Logical Port Commands</h1>
-    <p>lport-add &lt;name&gt; &lt;lswitch&gt;</p>
-    <p>lport-del &lt;lport&gt;</p>
-    <p>lport-list &lt;lswitch&gt;</p>
-    <p>lport-set-external-id &lt;lport&gt; &lt;key&gt; [value]</p>
-    <p>lport-get-external-id &lt;lport&gt; [key]</p>
-    <p>lport-set-macs &lt;lport&gt; [MAC] [MAC] [...]</p>
-    <p>lport-get-macs &lt;lport&gt;</p>
+    <p><code>lport-add</code> <var>name</var> <var>lswitch</var></p>
+    <p><code>lport-del</code> <var>lport</var></p>
+    <p><code>lport-list</code> <var>lswitch</var></p>
+    <p><code>lport-set-external-id</code> <var>lport</var> <var>key</var> 
[<var>value</var>]</p>
+    <p><code>lport-get-external-id</code> <var>lport</var> [<var>key</var>]</p>
+    <p><code>lport-set-macs</code> <var>lport</var> [<var>mac</var>] 
[<var>mac</var>] [...]</p>
+    <p><code>lport-get-macs</code> <var>lport</var></p>
 
     <h1>Options</h1>
-    <p>-d DATABASE | --database DATABASE</p>
-    <p>-h | --help</p>
-    <p>-o | --options</p>
-    <p>-V | --version</p>
+    <p><code>-d</code> <var>database</var> | <code>--database</code> 
<var>database</var></p>
+    <p><code>-h</code> | <code>--help</code></p>
+    <p><code>-o</code> | <code>--options</code></p>
+    <p><code>-V</code> | <code>--version</code></p>
 
     <h1>Logging options</h1>
-    <p>-vSPEC, --verbose=SPEC</p>
-    <p>-v, --verbose</p>
-    <p>--log-file[=FILE]</p>
-    <p>--syslog-target=HOST:PORT</p>
+    <p><code>-v</code><var>spec</var>, 
<code>--verbose=</code><var>spec</var></p>
+    <p><code>-v</code>, <code>--verbose</code></p>
+    <p><code>--log-file</code>[<code>=</code><var>file</var>]</p>
+    
<p><code>--syslog-target=</code><var>host</var><code>:</code><var>port</var></p>
 
     <h1>PKI configuration (required to use SSL)</h1>
-    <p>-p, --private-key=FILE  file with private key</p>
-    <p>-c, --certificate=FILE  file with certificate for private key</p>
-    <p>-C, --ca-cert=FILE      file with peer CA certificate</p>
+    <p><code>-p</code>, <code>--private-key=</code><var>file</var>  file with 
private key</p>
+    <p><code>-c</code>, <code>--certificate=</code><var>file</var>  file with 
certificate for private key</p>
+    <p><code>-C</code>, <code>--ca-cert=</code><var>file</var>      file with 
peer CA certificate</p>
 
 </manpage>
diff --git a/ovn/ovn-nbctl.c b/ovn/ovn-nbctl.c
index 3dc41e9..eade156 100644
--- a/ovn/ovn-nbctl.c
+++ b/ovn/ovn-nbctl.c
@@ -89,7 +89,8 @@ lswitch_by_name_or_uuid(struct nbctl_context *nb_ctx, const 
char *id)
 
     if (uuid_from_string(&lswitch_uuid, id)) {
         is_uuid = true;
-        lswitch = nbrec_logical_switch_get_for_uuid(nb_ctx->idl, 
&lswitch_uuid);
+        lswitch = nbrec_logical_switch_get_for_uuid(nb_ctx->idl,
+                                                    &lswitch_uuid);
     } else {
         const struct nbrec_logical_switch *iter;
 
@@ -99,7 +100,7 @@ lswitch_by_name_or_uuid(struct nbctl_context *nb_ctx, const 
char *id)
             }
             if (lswitch) {
                 VLOG_WARN("There is more than one logical switch named '%s'. "
-                        "Use a UUID.\n", id);
+                        "Use a UUID.", id);
                 lswitch = NULL;
                 duplicate = true;
                 break;
@@ -109,7 +110,7 @@ lswitch_by_name_or_uuid(struct nbctl_context *nb_ctx, const 
char *id)
     }
 
     if (!lswitch && !duplicate) {
-        VLOG_WARN("lswitch not found for %s: '%s'\n",
+        VLOG_WARN("lswitch not found for %s: '%s'",
                 is_uuid ? "UUID" : "name", id);
     }
 
@@ -150,7 +151,8 @@ do_lswitch_list(struct ovs_cmdl_context *ctx)
     const struct nbrec_logical_switch *lswitch;
 
     NBREC_LOGICAL_SWITCH_FOR_EACH(lswitch, nb_ctx->idl) {
-        printf(UUID_FMT " (%s)\n", UUID_ARGS(&lswitch->header_.uuid), 
lswitch->name);
+        printf(UUID_FMT " (%s)\n",
+               UUID_ARGS(&lswitch->header_.uuid), lswitch->name);
     }
 }
 
@@ -232,7 +234,7 @@ lport_by_name_or_uuid(struct nbctl_context *nb_ctx, const 
char *id)
     }
 
     if (!lport) {
-        VLOG_WARN("lport not found for %s: '%s'\n",
+        VLOG_WARN("lport not found for %s: '%s'",
                 is_uuid ? "UUID" : "name", id);
     }
 
@@ -305,7 +307,8 @@ do_lport_list(struct ovs_cmdl_context *ctx)
         if (!match) {
             continue;
         }
-        printf(UUID_FMT " (%s)\n", UUID_ARGS(&lport->header_.uuid), 
lport->name);
+        printf(UUID_FMT " (%s)\n",
+               UUID_ARGS(&lport->header_.uuid), lport->name);
     }
 }
 
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to