diff mbox series

[ovs-dev,2/2] ovn-nbctl: Clarify error messages in qos-add command.

Message ID 20180707211148.116369-2-jpettit@ovn.org
State Accepted
Headers show
Series [ovs-dev,1/2] ovn-nbctl: Correct qos-add documentation. | expand

Commit Message

Justin Pettit July 7, 2018, 9:11 p.m. UTC 
Signed-off-by: Justin Pettit <jpettit@ovn.org>
---
 ovn/utilities/ovn-nbctl.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

Comments

Yifeng Sun July 9, 2018, 7:39 p.m. UTC | #1 
Looks good to me, thanks.


Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>

On Sat, Jul 7, 2018 at 2:11 PM, Justin Pettit <jpettit@ovn.org> wrote:

> Signed-off-by: Justin Pettit <jpettit@ovn.org>
> ---
>  ovn/utilities/ovn-nbctl.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
> index fbdb5a4d9ae9..5638b0a197e0 100644
> --- a/ovn/utilities/ovn-nbctl.c
> +++ b/ovn/utilities/ovn-nbctl.c
> @@ -1761,14 +1761,15 @@ nbctl_qos_add(struct ctl_context *ctx)
>          if (!strncmp(ctx->argv[i], "dscp=", 5)) {
>              if (!ovs_scan(ctx->argv[i] + 5, "%"SCNd64, &dscp)
>                  || dscp < 0 || dscp > 63) {
> -                ctl_fatal("%s: dscp must in range 0...63.", ctx->argv[i]
> + 5);
> +                ctl_fatal("%s: dscp must be in the range 0...63",
> +                          ctx->argv[i] + 5);
>                  return;
>              }
>          }
>          else if (!strncmp(ctx->argv[i], "rate=", 5)) {
>              if (!ovs_scan(ctx->argv[i] + 5, "%"SCNd64, &rate)
>                  || rate < 1 || rate > UINT32_MAX) {
> -                ctl_fatal("%s: rate must in range 1...4294967295.",
> +                ctl_fatal("%s: rate must be in the range 1...4294967295.",
>                            ctx->argv[i] + 5);
>                  return;
>              }
> @@ -1776,20 +1777,20 @@ nbctl_qos_add(struct ctl_context *ctx)
>          else if (!strncmp(ctx->argv[i], "burst=", 6)) {
>              if (!ovs_scan(ctx->argv[i] + 6, "%"SCNd64, &burst)
>                  || burst < 1 || burst > UINT32_MAX) {
> -                ctl_fatal("%s: burst must in range 1...4294967295.",
> +                ctl_fatal("%s: burst must be in the range
> 1...4294967295.",
>                            ctx->argv[i] + 6);
>                  return;
>              }
>          } else {
> -            ctl_fatal("%s: must be start of \"dscp=\", \"rate=\",
> \"burst=\".",
> -                      ctx->argv[i]);
> +            ctl_fatal("%s: supported arguments are \"dscp=\", \"rate=\", "
> +                      "and \"burst=\"", ctx->argv[i]);
>              return;
>          }
>      }
>
>      /* Validate rate and dscp. */
>      if (-1 == dscp && !rate) {
> -        ctl_fatal("One of the rate or dscp must be configured.");
> +        ctl_fatal("Either \"rate\" and/or \"dscp\" must be specified");
>          return;
>      }
>
> --
> 2.17.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Justin Pettit July 10, 2018, 5:17 a.m. UTC | #2 
Thanks for the review.  I pushed these to master (and fixed a test failure that the original versions introduced.)

--Justin


> On Jul 9, 2018, at 2:39 PM, Yifeng Sun <pkusunyifeng@gmail.com> wrote:
> 
> Looks good to me, thanks.
> 
> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
> 
> On Sat, Jul 7, 2018 at 2:11 PM, Justin Pettit <jpettit@ovn.org> wrote:
> Signed-off-by: Justin Pettit <jpettit@ovn.org>
> ---
>  ovn/utilities/ovn-nbctl.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
> index fbdb5a4d9ae9..5638b0a197e0 100644
> --- a/ovn/utilities/ovn-nbctl.c
> +++ b/ovn/utilities/ovn-nbctl.c
> @@ -1761,14 +1761,15 @@ nbctl_qos_add(struct ctl_context *ctx)
>          if (!strncmp(ctx->argv[i], "dscp=", 5)) {
>              if (!ovs_scan(ctx->argv[i] + 5, "%"SCNd64, &dscp)
>                  || dscp < 0 || dscp > 63) {
> -                ctl_fatal("%s: dscp must in range 0...63.", ctx->argv[i] + 5);
> +                ctl_fatal("%s: dscp must be in the range 0...63",
> +                          ctx->argv[i] + 5);
>                  return;
>              }
>          }
>          else if (!strncmp(ctx->argv[i], "rate=", 5)) {
>              if (!ovs_scan(ctx->argv[i] + 5, "%"SCNd64, &rate)
>                  || rate < 1 || rate > UINT32_MAX) {
> -                ctl_fatal("%s: rate must in range 1...4294967295.",
> +                ctl_fatal("%s: rate must be in the range 1...4294967295.",
>                            ctx->argv[i] + 5);
>                  return;
>              }
> @@ -1776,20 +1777,20 @@ nbctl_qos_add(struct ctl_context *ctx)
>          else if (!strncmp(ctx->argv[i], "burst=", 6)) {
>              if (!ovs_scan(ctx->argv[i] + 6, "%"SCNd64, &burst)
>                  || burst < 1 || burst > UINT32_MAX) {
> -                ctl_fatal("%s: burst must in range 1...4294967295.",
> +                ctl_fatal("%s: burst must be in the range 1...4294967295.",
>                            ctx->argv[i] + 6);
>                  return;
>              }
>          } else {
> -            ctl_fatal("%s: must be start of \"dscp=\", \"rate=\", \"burst=\".",
> -                      ctx->argv[i]);
> +            ctl_fatal("%s: supported arguments are \"dscp=\", \"rate=\", "
> +                      "and \"burst=\"", ctx->argv[i]);
>              return;
>          }
>      }
> 
>      /* Validate rate and dscp. */
>      if (-1 == dscp && !rate) {
> -        ctl_fatal("One of the rate or dscp must be configured.");
> +        ctl_fatal("Either \"rate\" and/or \"dscp\" must be specified");
>          return;
>      }
>  
> -- 
> 2.17.1
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index fbdb5a4d9ae9..5638b0a197e0 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -1761,14 +1761,15 @@  nbctl_qos_add(struct ctl_context *ctx)
         if (!strncmp(ctx->argv[i], "dscp=", 5)) {
             if (!ovs_scan(ctx->argv[i] + 5, "%"SCNd64, &dscp)
                 || dscp < 0 || dscp > 63) {
-                ctl_fatal("%s: dscp must in range 0...63.", ctx->argv[i] + 5);
+                ctl_fatal("%s: dscp must be in the range 0...63",
+                          ctx->argv[i] + 5);
                 return;
             }
         }
         else if (!strncmp(ctx->argv[i], "rate=", 5)) {
             if (!ovs_scan(ctx->argv[i] + 5, "%"SCNd64, &rate)
                 || rate < 1 || rate > UINT32_MAX) {
-                ctl_fatal("%s: rate must in range 1...4294967295.",
+                ctl_fatal("%s: rate must be in the range 1...4294967295.",
                           ctx->argv[i] + 5);
                 return;
             }
@@ -1776,20 +1777,20 @@  nbctl_qos_add(struct ctl_context *ctx)
         else if (!strncmp(ctx->argv[i], "burst=", 6)) {
             if (!ovs_scan(ctx->argv[i] + 6, "%"SCNd64, &burst)
                 || burst < 1 || burst > UINT32_MAX) {
-                ctl_fatal("%s: burst must in range 1...4294967295.",
+                ctl_fatal("%s: burst must be in the range 1...4294967295.",
                           ctx->argv[i] + 6);
                 return;
             }
         } else {
-            ctl_fatal("%s: must be start of \"dscp=\", \"rate=\", \"burst=\".",
-                      ctx->argv[i]);
+            ctl_fatal("%s: supported arguments are \"dscp=\", \"rate=\", "
+                      "and \"burst=\"", ctx->argv[i]);
             return;
         }
     }
 
     /* Validate rate and dscp. */
     if (-1 == dscp && !rate) {
-        ctl_fatal("One of the rate or dscp must be configured.");
+        ctl_fatal("Either \"rate\" and/or \"dscp\" must be specified");
         return;
     }