diff mbox series

[ovs-dev,v5,OVN] ovn-nbctl.c: Add an optional way to delete QoS by uuid

Message ID 20200319055246.26107-1-taoyunxiang@cmss.chinamobile.com
State Accepted
Headers show
Series [ovs-dev,v5,OVN] ovn-nbctl.c: Add an optional way to delete QoS by uuid | expand

Commit Message

taoyunxiang March 19, 2020, 5:52 a.m. UTC 
We can delete qos by specify ls and more parameters.
If CMS want to delete it exactly, it must specify detailed "match" field.
It's not an easy way, also maybe deleted by mistake.
This change adds a way to specify ls and uuid, which is optional.
You can still use the previous method to delete.

usage:
ovn-nbctl qos-del ls0 [UUID0]

Author: Tao YunXiang <taoyunxiang@cmss.chinamobile.com>
Co-authored-by: Liu Chang <liuchang@cmss.chinamobile.com>
Co-authored-by: Rong Yin <rongyin@cmss.chinamobile.com>
Signed-off-by: Tao YunXiang <taoyunxiang@cmss.chinamobile.com>
Signed-off-by: Liu Chang <liuchang@cmss.chinamobile.com>
Signed-off-by: Rong Yin <rongyin@cmss.chinamobile.com>

---
v4: Add a way to delete QoS by its name or uuid
v3: ovn-nbctl.c: Add a way to delete QoS by its name or uuid

---
 utilities/ovn-nbctl.c | 39 ++++++++++++++++++++++++++++-----------
 1 file changed, 28 insertions(+), 11 deletions(-)

Comments

0-day Robot March 19, 2020, 5:56 a.m. UTC | #1 
Bleep bloop.  Greetings Tao YunXiang, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line lacks whitespace around operator
#34 FILE: utilities/ovn-nbctl.c:607:
  qos-del SWITCH [{DIRECTION | UUID} [PRIORITY MATCH]]\n\

Lines checked: 98, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot
Numan Siddique March 24, 2020, 8:11 a.m. UTC | #2 
On Thu, Mar 19, 2020 at 11:23 AM Tao YunXiang
<taoyunxiang@cmss.chinamobile.com> wrote:
>
> We can delete qos by specify ls and more parameters.
> If CMS want to delete it exactly, it must specify detailed "match" field.
> It's not an easy way, also maybe deleted by mistake.
> This change adds a way to specify ls and uuid, which is optional.
> You can still use the previous method to delete.
>
> usage:
> ovn-nbctl qos-del ls0 [UUID0]
>
> Author: Tao YunXiang <taoyunxiang@cmss.chinamobile.com>
> Co-authored-by: Liu Chang <liuchang@cmss.chinamobile.com>
> Co-authored-by: Rong Yin <rongyin@cmss.chinamobile.com>
> Signed-off-by: Tao YunXiang <taoyunxiang@cmss.chinamobile.com>
> Signed-off-by: Liu Chang <liuchang@cmss.chinamobile.com>
> Signed-off-by: Rong Yin <rongyin@cmss.chinamobile.com>

Can you please add a few tests in the tests/ovn-nbctl.at which would help
in regressions ?

Thanks
Numan

>
> ---
> v4: Add a way to delete QoS by its name or uuid
> v3: ovn-nbctl.c: Add a way to delete QoS by its name or uuid
>
> ---
>  utilities/ovn-nbctl.c | 39 ++++++++++++++++++++++++++++-----------
>  1 file changed, 28 insertions(+), 11 deletions(-)
>
> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> index e80058e61..5b2fa6084 100644
> --- a/utilities/ovn-nbctl.c
> +++ b/utilities/ovn-nbctl.c
> @@ -604,7 +604,7 @@ ACL commands:\n\
>  QoS commands:\n\
>    qos-add SWITCH DIRECTION PRIORITY MATCH [rate=RATE [burst=BURST]] [dscp=DSCP]\n\
>                              add an QoS rule to SWITCH\n\
> -  qos-del SWITCH [DIRECTION [PRIORITY MATCH]]\n\
> +  qos-del SWITCH [{DIRECTION | UUID} [PRIORITY MATCH]]\n\
>                              remove QoS rules from SWITCH\n\
>    qos-list SWITCH           print QoS rules for SWITCH\n\
>  \n\
> @@ -2521,22 +2521,39 @@ nbctl_qos_del(struct ctl_context *ctx)
>      }
>
>      const char *direction;
> -    error = parse_direction(ctx->argv[2], &direction);
> -    if (error) {
> -        ctx->error = error;
> -        return;
> +    const struct uuid *qos_rule_uuid = NULL;
> +    struct uuid uuid_from_cmd;
> +    if (uuid_from_string(&uuid_from_cmd, ctx->argv[2])) {
> +        qos_rule_uuid = &uuid_from_cmd;
> +    } else {
> +        error = parse_direction(ctx->argv[2], &direction);
> +        if (error) {
> +            ctx->error = error;
> +            return;
> +        }
>      }
>
> -    /* If priority and match are not specified, delete all qos_rules with the
> -     * specified direction. */
> +    /* If uuid was specified, delete qos_rule with the
> +     * specified uuid. */
>      if (ctx->argc == 3) {
>          struct nbrec_qos **new_qos_rules
>              = xmalloc(sizeof *new_qos_rules * ls->n_qos_rules);
>
>          int n_qos_rules = 0;
> -        for (size_t i = 0; i < ls->n_qos_rules; i++) {
> -            if (strcmp(direction, ls->qos_rules[i]->direction)) {
> -                new_qos_rules[n_qos_rules++] = ls->qos_rules[i];
> +        if (qos_rule_uuid) {
> +            for (size_t i = 0; i < ls->n_qos_rules; i++) {
> +                if (!uuid_equals(qos_rule_uuid,
> +                                 &(ls->qos_rules[i]->header_.uuid))) {
> +                    new_qos_rules[n_qos_rules++] = ls->qos_rules[i];
> +                }
> +            }
> +        /* If priority and match are not specified, delete all qos_rules
> +         * with the specified direction. */
> +        } else {
> +            for (size_t i = 0; i < ls->n_qos_rules; i++) {
> +                if (strcmp(direction, ls->qos_rules[i]->direction)) {
> +                    new_qos_rules[n_qos_rules++] = ls->qos_rules[i];
> +                }
>              }
>          }
>
> @@ -6030,7 +6047,7 @@ static const struct ctl_command_syntax nbctl_commands[] = {
>      { "qos-add", 5, 7,
>        "SWITCH DIRECTION PRIORITY MATCH [rate=RATE [burst=BURST]] [dscp=DSCP]",
>        NULL, nbctl_qos_add, NULL, "--may-exist", RW },
> -    { "qos-del", 1, 4, "SWITCH [DIRECTION [PRIORITY MATCH]]", NULL,
> +    { "qos-del", 1, 4, "SWITCH [{DIRECTION | UUID} [PRIORITY MATCH]]", NULL,
>        nbctl_qos_del, NULL, "", RW },
>      { "qos-list", 1, 1, "SWITCH", NULL, nbctl_qos_list, NULL, "", RO },
>
> --
> 2.17.1
>
>
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
taoyunxiang May 7, 2020, 8:33 a.m. UTC | #3 
Hi  Numan,
I have add a new test, and summit v3, please help to review it.

Thanks,
Yun

--------------
taoyunxiang@cmss.chinamobile.com
>On Thu, Mar 19, 2020 at 11:23 AM Tao YunXiang
><taoyunxiang@cmss.chinamobile.com> wrote:
>>
>> We can delete qos by specify ls and more parameters.
>> If CMS want to delete it exactly, it must specify detailed "match" field.
>> It's not an easy way, also maybe deleted by mistake.
>> This change adds a way to specify ls and uuid, which is optional.
>> You can still use the previous method to delete.
>>
>> usage:
>> ovn-nbctl qos-del ls0 [UUID0]
>>
>> Author: Tao YunXiang <taoyunxiang@cmss.chinamobile.com>
>> Co-authored-by: Liu Chang <liuchang@cmss.chinamobile.com>
>> Co-authored-by: Rong Yin <rongyin@cmss.chinamobile.com>
>> Signed-off-by: Tao YunXiang <taoyunxiang@cmss.chinamobile.com>
>> Signed-off-by: Liu Chang <liuchang@cmss.chinamobile.com>
>> Signed-off-by: Rong Yin <rongyin@cmss.chinamobile.com>
>
>Can you please add a few tests in the tests/ovn-nbctl.at which would help
>in regressions ?
>
>Thanks
>Numan
>
>>
>> ---
>> v4: Add a way to delete QoS by its name or uuid
>> v3: ovn-nbctl.c: Add a way to delete QoS by its name or uuid
>>
>> ---
>>  utilities/ovn-nbctl.c | 39 ++++++++++++++++++++++++++++-----------
>>  1 file changed, 28 insertions(+), 11 deletions(-)
>>
>> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
>> index e80058e61..5b2fa6084 100644
>> --- a/utilities/ovn-nbctl.c
>> +++ b/utilities/ovn-nbctl.c
>> @@ -604,7 +604,7 @@ ACL commands:\n\
>>  QoS commands:\n\
>>    qos-add SWITCH DIRECTION PRIORITY MATCH [rate=RATE [burst=BURST]] [dscp=DSCP]\n\
>>                              add an QoS rule to SWITCH\n\
>> -  qos-del SWITCH [DIRECTION [PRIORITY MATCH]]\n\
>> +  qos-del SWITCH [{DIRECTION | UUID} [PRIORITY MATCH]]\n\
>>                              remove QoS rules from SWITCH\n\
>>    qos-list SWITCH           print QoS rules for SWITCH\n\
>>  \n\
>> @@ -2521,22 +2521,39 @@ nbctl_qos_del(struct ctl_context *ctx)
>>      }
>>
>>      const char *direction;
>> -    error = parse_direction(ctx->argv[2], &direction);
>> -    if (error) {
>> -        ctx->error = error;
>> -        return;
>> +    const struct uuid *qos_rule_uuid = NULL;
>> +    struct uuid uuid_from_cmd;
>> +    if (uuid_from_string(&uuid_from_cmd, ctx->argv[2])) {
>> +        qos_rule_uuid = &uuid_from_cmd;
>> +    } else {
>> +        error = parse_direction(ctx->argv[2], &direction);
>> +        if (error) {
>> +            ctx->error = error;
>> +            return;
>> +        }
>>      }
>>
>> -    /* If priority and match are not specified, delete all qos_rules with the
>> -     * specified direction. */
>> +    /* If uuid was specified, delete qos_rule with the
>> +     * specified uuid. */
>>      if (ctx->argc == 3) {
>>          struct nbrec_qos **new_qos_rules
>>              = xmalloc(sizeof *new_qos_rules * ls->n_qos_rules);
>>
>>          int n_qos_rules = 0;
>> -        for (size_t i = 0; i < ls->n_qos_rules; i++) {
>> -            if (strcmp(direction, ls->qos_rules[i]->direction)) {
>> -                new_qos_rules[n_qos_rules++] = ls->qos_rules[i];
>> +        if (qos_rule_uuid) {
>> +            for (size_t i = 0; i < ls->n_qos_rules; i++) {
>> +                if (!uuid_equals(qos_rule_uuid,
>> +                                 &(ls->qos_rules[i]->header_.uuid))) {
>> +                    new_qos_rules[n_qos_rules++] = ls->qos_rules[i];
>> +                }
>> +            }
>> +        /* If priority and match are not specified, delete all qos_rules
>> +         * with the specified direction. */
>> +        } else {
>> +            for (size_t i = 0; i < ls->n_qos_rules; i++) {
>> +                if (strcmp(direction, ls->qos_rules[i]->direction)) {
>> +                    new_qos_rules[n_qos_rules++] = ls->qos_rules[i];
>> +                }
>>              }
>>          }
>>
>> @@ -6030,7 +6047,7 @@ static const struct ctl_command_syntax nbctl_commands[] = {
>>      { "qos-add", 5, 7,
>>        "SWITCH DIRECTION PRIORITY MATCH [rate=RATE [burst=BURST]] [dscp=DSCP]",
>>        NULL, nbctl_qos_add, NULL, "--may-exist", RW },
>> -    { "qos-del", 1, 4, "SWITCH [DIRECTION [PRIORITY MATCH]]", NULL,
>> +    { "qos-del", 1, 4, "SWITCH [{DIRECTION | UUID} [PRIORITY MATCH]]", NULL,
>>        nbctl_qos_del, NULL, "", RW },
>>      { "qos-list", 1, 1, "SWITCH", NULL, nbctl_qos_list, NULL, "", RO },
>>
>> --
>> 2.17.1
>>
>>
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
taoyunxiang May 7, 2020, 8:42 a.m. UTC | #4 
Sorry, 
         Acutally, I summited a new change for [0], and the patchwork address is [1]

[0] [ovs-dev,OVN,v2] ovn-nbctl.c: Add an optional way to delete router policy by uuid
[1] http://patchwork.ozlabs.org/project/openvswitch/patch/20200324014951.9521-1-taoyunxiang@cmss.chinamobile.com/

Regards,
Yun

--------------
taoyunxiang@cmss.chinamobile.com
>Hi  Numan,
>I have add a new test, and summit v3, please help to review it.
>
>Thanks,
>Yun
>
>--------------
>taoyunxiang@cmss.chinamobile.com
>>On Thu, Mar 19, 2020 at 11:23 AM Tao YunXiang
>><taoyunxiang@cmss.chinamobile.com> wrote:
>>>
>>> We can delete qos by specify ls and more parameters.
>>> If CMS want to delete it exactly, it must specify detailed "match" field.
>>> It's not an easy way, also maybe deleted by mistake.
>>> This change adds a way to specify ls and uuid, which is optional.
>>> You can still use the previous method to delete.
>>>
>>> usage:
>>> ovn-nbctl qos-del ls0 [UUID0]
>>>
>>> Author: Tao YunXiang <taoyunxiang@cmss.chinamobile.com>
>>> Co-authored-by: Liu Chang <liuchang@cmss.chinamobile.com>
>>> Co-authored-by: Rong Yin <rongyin@cmss.chinamobile.com>
>>> Signed-off-by: Tao YunXiang <taoyunxiang@cmss.chinamobile.com>
>>> Signed-off-by: Liu Chang <liuchang@cmss.chinamobile.com>
>>> Signed-off-by: Rong Yin <rongyin@cmss.chinamobile.com>
>>
>>Can you please add a few tests in the tests/ovn-nbctl.at which would help
>>in regressions ?
>>
>>Thanks
>>Numan
>>
>>>
>>> ---
>>> v4: Add a way to delete QoS by its name or uuid
>>> v3: ovn-nbctl.c: Add a way to delete QoS by its name or uuid
>>>
>>> ---
>>>  utilities/ovn-nbctl.c | 39 ++++++++++++++++++++++++++++-----------
>>>  1 file changed, 28 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
>>> index e80058e61..5b2fa6084 100644
>>> --- a/utilities/ovn-nbctl.c
>>> +++ b/utilities/ovn-nbctl.c
>>> @@ -604,7 +604,7 @@ ACL commands:\n\
>>>  QoS commands:\n\
>>>    qos-add SWITCH DIRECTION PRIORITY MATCH [rate=RATE [burst=BURST]] [dscp=DSCP]\n\
>>>                              add an QoS rule to SWITCH\n\
>>> -  qos-del SWITCH [DIRECTION [PRIORITY MATCH]]\n\
>>> +  qos-del SWITCH [{DIRECTION | UUID} [PRIORITY MATCH]]\n\
>>>                              remove QoS rules from SWITCH\n\
>>>    qos-list SWITCH           print QoS rules for SWITCH\n\
>>>  \n\
>>> @@ -2521,22 +2521,39 @@ nbctl_qos_del(struct ctl_context *ctx)
>>>      }
>>>
>>>      const char *direction;
>>> -    error = parse_direction(ctx->argv[2], &direction);
>>> -    if (error) {
>>> -        ctx->error = error;
>>> -        return;
>>> +    const struct uuid *qos_rule_uuid = NULL;
>>> +    struct uuid uuid_from_cmd;
>>> +    if (uuid_from_string(&uuid_from_cmd, ctx->argv[2])) {
>>> +        qos_rule_uuid = &uuid_from_cmd;
>>> +    } else {
>>> +        error = parse_direction(ctx->argv[2], &direction);
>>> +        if (error) {
>>> +            ctx->error = error;
>>> +            return;
>>> +        }
>>>      }
>>>
>>> -    /* If priority and match are not specified, delete all qos_rules with the
>>> -     * specified direction. */
>>> +    /* If uuid was specified, delete qos_rule with the
>>> +     * specified uuid. */
>>>      if (ctx->argc == 3) {
>>>          struct nbrec_qos **new_qos_rules
>>>              = xmalloc(sizeof *new_qos_rules * ls->n_qos_rules);
>>>
>>>          int n_qos_rules = 0;
>>> -        for (size_t i = 0; i < ls->n_qos_rules; i++) {
>>> -            if (strcmp(direction, ls->qos_rules[i]->direction)) {
>>> -                new_qos_rules[n_qos_rules++] = ls->qos_rules[i];
>>> +        if (qos_rule_uuid) {
>>> +            for (size_t i = 0; i < ls->n_qos_rules; i++) {
>>> +                if (!uuid_equals(qos_rule_uuid,
>>> +                                 &(ls->qos_rules[i]->header_.uuid))) {
>>> +                    new_qos_rules[n_qos_rules++] = ls->qos_rules[i];
>>> +                }
>>> +            }
>>> +        /* If priority and match are not specified, delete all qos_rules
>>> +         * with the specified direction. */
>>> +        } else {
>>> +            for (size_t i = 0; i < ls->n_qos_rules; i++) {
>>> +                if (strcmp(direction, ls->qos_rules[i]->direction)) {
>>> +                    new_qos_rules[n_qos_rules++] = ls->qos_rules[i];
>>> +                }
>>>              }
>>>          }
>>>
>>> @@ -6030,7 +6047,7 @@ static const struct ctl_command_syntax nbctl_commands[] = {
>>>      { "qos-add", 5, 7,
>>>        "SWITCH DIRECTION PRIORITY MATCH [rate=RATE [burst=BURST]] [dscp=DSCP]",
>>>        NULL, nbctl_qos_add, NULL, "--may-exist", RW },
>>> -    { "qos-del", 1, 4, "SWITCH [DIRECTION [PRIORITY MATCH]]", NULL,
>>> +    { "qos-del", 1, 4, "SWITCH [{DIRECTION | UUID} [PRIORITY MATCH]]", NULL,
>>>        nbctl_qos_del, NULL, "", RW },
>>>      { "qos-list", 1, 1, "SWITCH", NULL, nbctl_qos_list, NULL, "", RO },
>>>
>>> --
>>> 2.17.1
>>>
>>>
>>>
>>> _______________________________________________
>>> dev mailing list
>>> dev@openvswitch.org
>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>>
Numan Siddique May 7, 2020, 6 p.m. UTC | #5 
On Thu, May 7, 2020 at 2:13 PM taoyunxiang@cmss.chinamobile.com <
taoyunxiang@cmss.chinamobile.com> wrote:

> Sorry,
>          Acutally, I summited a new change for [0], and the patchwork
> address is [1]
>
> [0] [ovs-dev,OVN,v2] ovn-nbctl.c: Add an optional way to delete router
> policy by uuid
> [1]
> http://patchwork.ozlabs.org/project/openvswitch/patch/20200324014951.9521-1-taoyunxiang@cmss.chinamobile.com/
>
> Regards,
> Yun
>
> --------------
> taoyunxiang@cmss.chinamobile.com
> >Hi  Numan,
> >I have add a new test, and summit v3, please help to review it.


Thanks for adding the test case.

I applied this patch to master with one small change (below).


diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index 4195844d8..1187fe3e1 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -1621,7 +1621,7 @@ Routing Policies


 dnl Delete policy by specified uuid
-AT_CHECK([ovn-nbctl lr-policy-del lr0 `ovn-nbctl --columns=_uuid list
logical-router-policy | awk -F ':'  '{print $2}'`])
+AT_CHECK([ovn-nbctl lr-policy-del lr0 $(ovn-nbctl --bare --column _uuid
list logical_router_policy)])
 AT_CHECK([ovn-nbctl list logical-router-policy], [0], [dnl
 ])

Thanks
Numan



>
> >Thanks,
> >Yun
> >
> >--------------
> >taoyunxiang@cmss.chinamobile.com
> >>On Thu, Mar 19, 2020 at 11:23 AM Tao YunXiang
> >><taoyunxiang@cmss.chinamobile.com> wrote:
> >>>
> >>> We can delete qos by specify ls and more parameters.
> >>> If CMS want to delete it exactly, it must specify detailed "match"
> field.
> >>> It's not an easy way, also maybe deleted by mistake.
> >>> This change adds a way to specify ls and uuid, which is optional.
> >>> You can still use the previous method to delete.
> >>>
> >>> usage:
> >>> ovn-nbctl qos-del ls0 [UUID0]
> >>>
> >>> Author: Tao YunXiang <taoyunxiang@cmss.chinamobile.com>
> >>> Co-authored-by: Liu Chang <liuchang@cmss.chinamobile.com>
> >>> Co-authored-by: Rong Yin <rongyin@cmss.chinamobile.com>
> >>> Signed-off-by: Tao YunXiang <taoyunxiang@cmss.chinamobile.com>
> >>> Signed-off-by: Liu Chang <liuchang@cmss.chinamobile.com>
> >>> Signed-off-by: Rong Yin <rongyin@cmss.chinamobile.com>
> >>
> >>Can you please add a few tests in the tests/ovn-nbctl.at which would
> help
> >>in regressions ?
> >>
> >>Thanks
> >>Numan
> >>
> >>>
> >>> ---
> >>> v4: Add a way to delete QoS by its name or uuid
> >>> v3: ovn-nbctl.c: Add a way to delete QoS by its name or uuid
> >>>
> >>> ---
> >>>  utilities/ovn-nbctl.c | 39 ++++++++++++++++++++++++++++-----------
> >>>  1 file changed, 28 insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> >>> index e80058e61..5b2fa6084 100644
> >>> --- a/utilities/ovn-nbctl.c
> >>> +++ b/utilities/ovn-nbctl.c
> >>> @@ -604,7 +604,7 @@ ACL commands:\n\
> >>>  QoS commands:\n\
> >>>    qos-add SWITCH DIRECTION PRIORITY MATCH [rate=RATE [burst=BURST]]
> [dscp=DSCP]\n\
> >>>                              add an QoS rule to SWITCH\n\
> >>> -  qos-del SWITCH [DIRECTION [PRIORITY MATCH]]\n\
> >>> +  qos-del SWITCH [{DIRECTION | UUID} [PRIORITY MATCH]]\n\
> >>>                              remove QoS rules from SWITCH\n\
> >>>    qos-list SWITCH           print QoS rules for SWITCH\n\
> >>>  \n\
> >>> @@ -2521,22 +2521,39 @@ nbctl_qos_del(struct ctl_context *ctx)
> >>>      }
> >>>
> >>>      const char *direction;
> >>> -    error = parse_direction(ctx->argv[2], &direction);
> >>> -    if (error) {
> >>> -        ctx->error = error;
> >>> -        return;
> >>> +    const struct uuid *qos_rule_uuid = NULL;
> >>> +    struct uuid uuid_from_cmd;
> >>> +    if (uuid_from_string(&uuid_from_cmd, ctx->argv[2])) {
> >>> +        qos_rule_uuid = &uuid_from_cmd;
> >>> +    } else {
> >>> +        error = parse_direction(ctx->argv[2], &direction);
> >>> +        if (error) {
> >>> +            ctx->error = error;
> >>> +            return;
> >>> +        }
> >>>      }
> >>>
> >>> -    /* If priority and match are not specified, delete all qos_rules
> with the
> >>> -     * specified direction. */
> >>> +    /* If uuid was specified, delete qos_rule with the
> >>> +     * specified uuid. */
> >>>      if (ctx->argc == 3) {
> >>>          struct nbrec_qos **new_qos_rules
> >>>              = xmalloc(sizeof *new_qos_rules * ls->n_qos_rules);
> >>>
> >>>          int n_qos_rules = 0;
> >>> -        for (size_t i = 0; i < ls->n_qos_rules; i++) {
> >>> -            if (strcmp(direction, ls->qos_rules[i]->direction)) {
> >>> -                new_qos_rules[n_qos_rules++] = ls->qos_rules[i];
> >>> +        if (qos_rule_uuid) {
> >>> +            for (size_t i = 0; i < ls->n_qos_rules; i++) {
> >>> +                if (!uuid_equals(qos_rule_uuid,
> >>> +                                 &(ls->qos_rules[i]->header_.uuid))) {
> >>> +                    new_qos_rules[n_qos_rules++] = ls->qos_rules[i];
> >>> +                }
> >>> +            }
> >>> +        /* If priority and match are not specified, delete all
> qos_rules
> >>> +         * with the specified direction. */
> >>> +        } else {
> >>> +            for (size_t i = 0; i < ls->n_qos_rules; i++) {
> >>> +                if (strcmp(direction, ls->qos_rules[i]->direction)) {
> >>> +                    new_qos_rules[n_qos_rules++] = ls->qos_rules[i];
> >>> +                }
> >>>              }
> >>>          }
> >>>
> >>> @@ -6030,7 +6047,7 @@ static const struct ctl_command_syntax
> nbctl_commands[] = {
> >>>      { "qos-add", 5, 7,
> >>>        "SWITCH DIRECTION PRIORITY MATCH [rate=RATE [burst=BURST]]
> [dscp=DSCP]",
> >>>        NULL, nbctl_qos_add, NULL, "--may-exist", RW },
> >>> -    { "qos-del", 1, 4, "SWITCH [DIRECTION [PRIORITY MATCH]]", NULL,
> >>> +    { "qos-del", 1, 4, "SWITCH [{DIRECTION | UUID} [PRIORITY
> MATCH]]", NULL,
> >>>        nbctl_qos_del, NULL, "", RW },
> >>>      { "qos-list", 1, 1, "SWITCH", NULL, nbctl_qos_list, NULL, "", RO
> },
> >>>
> >>> --
> >>> 2.17.1
> >>>
> >>>
> >>>
> >>> _______________________________________________
> >>> dev mailing list
> >>> dev@openvswitch.org
> >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >>>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
taoyunxiang May 9, 2020, 7:22 a.m. UTC | #6 
Hi  Numan,
I have add a test, and summit v6, please help to review it.

Thanks,
Yun
--------------
taoyunxiang@cmss.chinamobile.com
>On Thu, Mar 19, 2020 at 11:23 AM Tao YunXiang
><taoyunxiang@cmss.chinamobile.com> wrote:
>>
>> We can delete qos by specify ls and more parameters.
>> If CMS want to delete it exactly, it must specify detailed "match" field.
>> It's not an easy way, also maybe deleted by mistake.
>> This change adds a way to specify ls and uuid, which is optional.
>> You can still use the previous method to delete.
>>
>> usage:
>> ovn-nbctl qos-del ls0 [UUID0]
>>
>> Author: Tao YunXiang <taoyunxiang@cmss.chinamobile.com>
>> Co-authored-by: Liu Chang <liuchang@cmss.chinamobile.com>
>> Co-authored-by: Rong Yin <rongyin@cmss.chinamobile.com>
>> Signed-off-by: Tao YunXiang <taoyunxiang@cmss.chinamobile.com>
>> Signed-off-by: Liu Chang <liuchang@cmss.chinamobile.com>
>> Signed-off-by: Rong Yin <rongyin@cmss.chinamobile.com>
>
>Can you please add a few tests in the tests/ovn-nbctl.at which would help
>in regressions ?
>
>Thanks
>Numan
>
>>
>> ---
>> v4: Add a way to delete QoS by its name or uuid
>> v3: ovn-nbctl.c: Add a way to delete QoS by its name or uuid
>>
>> ---
>>  utilities/ovn-nbctl.c | 39 ++++++++++++++++++++++++++++-----------
>>  1 file changed, 28 insertions(+), 11 deletions(-)
>>
>> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
>> index e80058e61..5b2fa6084 100644
>> --- a/utilities/ovn-nbctl.c
>> +++ b/utilities/ovn-nbctl.c
>> @@ -604,7 +604,7 @@ ACL commands:\n\
>>  QoS commands:\n\
>>    qos-add SWITCH DIRECTION PRIORITY MATCH [rate=RATE [burst=BURST]] [dscp=DSCP]\n\
>>                              add an QoS rule to SWITCH\n\
>> -  qos-del SWITCH [DIRECTION [PRIORITY MATCH]]\n\
>> +  qos-del SWITCH [{DIRECTION | UUID} [PRIORITY MATCH]]\n\
>>                              remove QoS rules from SWITCH\n\
>>    qos-list SWITCH           print QoS rules for SWITCH\n\
>>  \n\
>> @@ -2521,22 +2521,39 @@ nbctl_qos_del(struct ctl_context *ctx)
>>      }
>>
>>      const char *direction;
>> -    error = parse_direction(ctx->argv[2], &direction);
>> -    if (error) {
>> -        ctx->error = error;
>> -        return;
>> +    const struct uuid *qos_rule_uuid = NULL;
>> +    struct uuid uuid_from_cmd;
>> +    if (uuid_from_string(&uuid_from_cmd, ctx->argv[2])) {
>> +        qos_rule_uuid = &uuid_from_cmd;
>> +    } else {
>> +        error = parse_direction(ctx->argv[2], &direction);
>> +        if (error) {
>> +            ctx->error = error;
>> +            return;
>> +        }
>>      }
>>
>> -    /* If priority and match are not specified, delete all qos_rules with the
>> -     * specified direction. */
>> +    /* If uuid was specified, delete qos_rule with the
>> +     * specified uuid. */
>>      if (ctx->argc == 3) {
>>          struct nbrec_qos **new_qos_rules
>>              = xmalloc(sizeof *new_qos_rules * ls->n_qos_rules);
>>
>>          int n_qos_rules = 0;
>> -        for (size_t i = 0; i < ls->n_qos_rules; i++) {
>> -            if (strcmp(direction, ls->qos_rules[i]->direction)) {
>> -                new_qos_rules[n_qos_rules++] = ls->qos_rules[i];
>> +        if (qos_rule_uuid) {
>> +            for (size_t i = 0; i < ls->n_qos_rules; i++) {
>> +                if (!uuid_equals(qos_rule_uuid,
>> +                                 &(ls->qos_rules[i]->header_.uuid))) {
>> +                    new_qos_rules[n_qos_rules++] = ls->qos_rules[i];
>> +                }
>> +            }
>> +        /* If priority and match are not specified, delete all qos_rules
>> +         * with the specified direction. */
>> +        } else {
>> +            for (size_t i = 0; i < ls->n_qos_rules; i++) {
>> +                if (strcmp(direction, ls->qos_rules[i]->direction)) {
>> +                    new_qos_rules[n_qos_rules++] = ls->qos_rules[i];
>> +                }
>>              }
>>          }
>>
>> @@ -6030,7 +6047,7 @@ static const struct ctl_command_syntax nbctl_commands[] = {
>>      { "qos-add", 5, 7,
>>        "SWITCH DIRECTION PRIORITY MATCH [rate=RATE [burst=BURST]] [dscp=DSCP]",
>>        NULL, nbctl_qos_add, NULL, "--may-exist", RW },
>> -    { "qos-del", 1, 4, "SWITCH [DIRECTION [PRIORITY MATCH]]", NULL,
>> +    { "qos-del", 1, 4, "SWITCH [{DIRECTION | UUID} [PRIORITY MATCH]]", NULL,
>>        nbctl_qos_del, NULL, "", RW },
>>      { "qos-list", 1, 1, "SWITCH", NULL, nbctl_qos_list, NULL, "", RO },
>>
>> --
>> 2.17.1
>>
>>
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
diff mbox series

Patch

diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index e80058e61..5b2fa6084 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -604,7 +604,7 @@  ACL commands:\n\
 QoS commands:\n\
   qos-add SWITCH DIRECTION PRIORITY MATCH [rate=RATE [burst=BURST]] [dscp=DSCP]\n\
                             add an QoS rule to SWITCH\n\
-  qos-del SWITCH [DIRECTION [PRIORITY MATCH]]\n\
+  qos-del SWITCH [{DIRECTION | UUID} [PRIORITY MATCH]]\n\
                             remove QoS rules from SWITCH\n\
   qos-list SWITCH           print QoS rules for SWITCH\n\
 \n\
@@ -2521,22 +2521,39 @@  nbctl_qos_del(struct ctl_context *ctx)
     }
 
     const char *direction;
-    error = parse_direction(ctx->argv[2], &direction);
-    if (error) {
-        ctx->error = error;
-        return;
+    const struct uuid *qos_rule_uuid = NULL;
+    struct uuid uuid_from_cmd;
+    if (uuid_from_string(&uuid_from_cmd, ctx->argv[2])) {
+        qos_rule_uuid = &uuid_from_cmd;
+    } else {
+        error = parse_direction(ctx->argv[2], &direction);
+        if (error) {
+            ctx->error = error;
+            return;
+        }
     }
 
-    /* If priority and match are not specified, delete all qos_rules with the
-     * specified direction. */
+    /* If uuid was specified, delete qos_rule with the
+     * specified uuid. */
     if (ctx->argc == 3) {
         struct nbrec_qos **new_qos_rules
             = xmalloc(sizeof *new_qos_rules * ls->n_qos_rules);
 
         int n_qos_rules = 0;
-        for (size_t i = 0; i < ls->n_qos_rules; i++) {
-            if (strcmp(direction, ls->qos_rules[i]->direction)) {
-                new_qos_rules[n_qos_rules++] = ls->qos_rules[i];
+        if (qos_rule_uuid) {
+            for (size_t i = 0; i < ls->n_qos_rules; i++) {
+                if (!uuid_equals(qos_rule_uuid,
+                                 &(ls->qos_rules[i]->header_.uuid))) {
+                    new_qos_rules[n_qos_rules++] = ls->qos_rules[i];
+                }
+            }
+        /* If priority and match are not specified, delete all qos_rules
+         * with the specified direction. */
+        } else {
+            for (size_t i = 0; i < ls->n_qos_rules; i++) {
+                if (strcmp(direction, ls->qos_rules[i]->direction)) {
+                    new_qos_rules[n_qos_rules++] = ls->qos_rules[i];
+                }
             }
         }
 
@@ -6030,7 +6047,7 @@  static const struct ctl_command_syntax nbctl_commands[] = {
     { "qos-add", 5, 7,
       "SWITCH DIRECTION PRIORITY MATCH [rate=RATE [burst=BURST]] [dscp=DSCP]",
       NULL, nbctl_qos_add, NULL, "--may-exist", RW },
-    { "qos-del", 1, 4, "SWITCH [DIRECTION [PRIORITY MATCH]]", NULL,
+    { "qos-del", 1, 4, "SWITCH [{DIRECTION | UUID} [PRIORITY MATCH]]", NULL,
       nbctl_qos_del, NULL, "", RW },
     { "qos-list", 1, 1, "SWITCH", NULL, nbctl_qos_list, NULL, "", RO },