diff mbox

[ovs-dev] datapath_windows: Set isActivated flag only on success

Message ID 1475703225-31768-1-git-send-email-rams@vmware.com
State Changes Requested
Headers show

Commit Message

Shashank Ram Oct. 5, 2016, 9:33 p.m. UTC
@Switch.c: Modifies OvsActivateSwitch() function
to mark the switch as activated only if the
the status is success. The callers itself
only call this method when the isActivated
flag is unset.

Signed-off-by: Shashank Ram <rams@vmware.com>
---
 datapath-windows/ovsext/Switch.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--
2.6.2

Comments

Sairam Venugopal Oct. 10, 2016, 10:02 p.m. UTC | #1
Thanks for the patch. Usually we append Œdatapath-windows: Brief
description¹ for Windows datapath commmits.

Had a comment which is inlined.

Thanks,
Sairam

On 10/5/16, 2:33 PM, "Shashank Ram" <rams@vmware.com> wrote:

>@Switch.c: Modifies OvsActivateSwitch() function
>to mark the switch as activated only if the
>the status is success. The callers itself
>only call this method when the isActivated
>flag is unset.
>
>Signed-off-by: Shashank Ram <rams@vmware.com>
>---
> datapath-windows/ovsext/Switch.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
>diff --git a/datapath-windows/ovsext/Switch.c
>b/datapath-windows/ovsext/Switch.c
>index 825fa3c..7913cd5 100644
>--- a/datapath-windows/ovsext/Switch.c
>+++ b/datapath-windows/ovsext/Switch.c
>@@ -556,7 +556,6 @@ OvsActivateSwitch(POVS_SWITCH_CONTEXT switchContext)
>     OVS_LOG_TRACE("Enter: activate switch %p, dpNo: %ld",
>                   switchContext, switchContext->dpNo);
>
>-    switchContext->isActivated = TRUE;
>     status = OvsAddConfiguredSwitchPorts(switchContext);
>
>     if (status != NDIS_STATUS_SUCCESS) {
>@@ -573,7 +572,7 @@ OvsActivateSwitch(POVS_SWITCH_CONTEXT switchContext)
>     }

Sai: You can add switchContext->isActivated = TRUE; before the cleanup
label.
This way you can get rid of the check for status==NDIS_STATUS_SUCCESS.

>
> cleanup:
>-    if (status != NDIS_STATUS_SUCCESS) {
>+    if (status == NDIS_STATUS_SUCCESS) {
>         switchContext->isActivated = TRUE;
>     }
>
>--
>2.6.2
>
>_______________________________________________
>dev mailing list
>dev@openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma
>n_listinfo_dev&d=CwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Dc
>ruz40PROJ40ROzSpxyQSLw6fcrOWpJgEcEmNR3JEQ&m=JEXx0nmOWA23mhKMOcUJzNcIULZ8kH
>WeAtQcmFjGo6k&s=5tUWl0geJxE6uBwW6svE_7kChh5iUVlo8kVnpFUrZSA&e=
diff mbox

Patch

diff --git a/datapath-windows/ovsext/Switch.c b/datapath-windows/ovsext/Switch.c
index 825fa3c..7913cd5 100644
--- a/datapath-windows/ovsext/Switch.c
+++ b/datapath-windows/ovsext/Switch.c
@@ -556,7 +556,6 @@  OvsActivateSwitch(POVS_SWITCH_CONTEXT switchContext)
     OVS_LOG_TRACE("Enter: activate switch %p, dpNo: %ld",
                   switchContext, switchContext->dpNo);

-    switchContext->isActivated = TRUE;
     status = OvsAddConfiguredSwitchPorts(switchContext);

     if (status != NDIS_STATUS_SUCCESS) {
@@ -573,7 +572,7 @@  OvsActivateSwitch(POVS_SWITCH_CONTEXT switchContext)
     }

 cleanup:
-    if (status != NDIS_STATUS_SUCCESS) {
+    if (status == NDIS_STATUS_SUCCESS) {
         switchContext->isActivated = TRUE;
     }