Logging and Code clean-up for protocol_plugins.core

Change-Id: Ia34dd373ba16d7a70be0440d7f076e7dab02ffdd

Signed-off-by: Pramila Singh <pramisin@cisco.com>

diff --git a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/Controller.java b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/Controller.java
index 9ec5b10ea8ee82624fd0f8702b33cc110e6c756a..8e6a30fceed5e0961e9c52660cfd77bb5c9ea71f 100644 (file)
--- a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/Controller.java
+++ b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/Controller.java
@@ -1,4 +1,3 @@
-
 /*
  * Copyright (c) 2013 Cisco Systems, Inc. and others.  All rights reserved.
  *
@@ -53,7 +52,8 @@ public class Controller implements IController, CommandProvider {
     private AtomicInteger switchInstanceNumber;
 
     /*
-     * this thread monitors the switchEvents queue for new incoming events from switch
+     * this thread monitors the switchEvents queue for new incoming events from
+     * switch
      */
     private class EventHandler implements Runnable {
         @Override
@@ -64,17 +64,13 @@ public class Controller implements IController, CommandProvider {
                     SwitchEvent ev = switchEvents.take();
                     SwitchEvent.SwitchEventType eType = ev.getEventType();
                     ISwitch sw = ev.getSwitch();
-                    if (eType != SwitchEvent.SwitchEventType.SWITCH_MESSAGE) {
-                        //logger.debug("Received " + ev.toString() + " from " + sw.toString());
-                    }
                     switch (eType) {
                     case SWITCH_ADD:
                         Long sid = sw.getId();
                         ISwitch existingSwitch = switches.get(sid);
                         if (existingSwitch != null) {
-                            logger.info(" Replacing existing "
-                                    + existingSwitch.toString() + " with New "
-                                    + sw.toString());
+                            logger.info("Replacing existing {} with New {}",
+                                    existingSwitch.toString(), sw.toString());
                             disconnectSwitch(existingSwitch);
                         }
                         switches.put(sid, sw);
@@ -97,7 +93,7 @@ public class Controller implements IController, CommandProvider {
                         }
                         break;
                     default:
-                        logger.error("unknow switch event " + eType.ordinal());
+                        logger.error("Unknown switch event {}", eType.ordinal());
                     }
                 } catch (InterruptedException e) {
                     switchEvents.clear();
@@ -111,10 +107,10 @@ public class Controller implements IController, CommandProvider {
     /**
      * Function called by the dependency manager when all the required
      * dependencies are satisfied
-     *
+     * 
      */
     public void init() {
-        logger.debug("OpenFlowCore init");
+        logger.debug("Initializing!");
         this.switches = new ConcurrentHashMap<Long, ISwitch>();
         this.switchEvents = new LinkedBlockingQueue<SwitchEvent>();
         this.messageListeners = new ConcurrentHashMap<OFType, IMessageListener>();
@@ -124,13 +120,12 @@ public class Controller implements IController, CommandProvider {
     }
 
     /**
-     * Function called by dependency manager after "init ()" is called
-     * and after the services provided by the class are registered in
-     * the service registry
-     *
+     * Function called by dependency manager after "init ()" is called and after
+     * the services provided by the class are registered in the service registry
+     * 
      */
     public void start() {
-        logger.debug("OpenFlowCore start() is called");
+        logger.debug("Starting!");
         /*
          * start a thread to handle event coming from the switch
          */
@@ -142,15 +137,15 @@ public class Controller implements IController, CommandProvider {
         try {
             controllerIO.start();
         } catch (IOException ex) {
-            logger.error("Caught exception: " + ex + " during start");
+            logger.error("Caught exception while starting:", ex);
         }
     }
-    
+
     /**
-     * Function called by the dependency manager before the services
-     * exported by the component are unregistered, this will be
-     * followed by a "destroy ()" calls
-     *
+     * Function called by the dependency manager before the services exported by
+     * the component are unregistered, this will be followed by a "destroy ()"
+     * calls
+     * 
      */
     public void stop() {
         for (Iterator<Entry<Long, ISwitch>> it = switches.entrySet().iterator(); it
@@ -161,17 +156,17 @@ public class Controller implements IController, CommandProvider {
         }
         switchEventThread.interrupt();
         try {
-               controllerIO.shutDown();
+            controllerIO.shutDown();
         } catch (IOException ex) {
-               logger.error("Caught exception: " + ex + " during stop");
+            logger.error("Caught exception while stopping:", ex);
         }
     }
 
     /**
-     * Function called by the dependency manager when at least one
-     * dependency become unsatisfied or when the component is shutting
-     * down because for example bundle is being stopped.
-     *
+     * Function called by the dependency manager when at least one dependency
+     * become unsatisfied or when the component is shutting down because for
+     * example bundle is being stopped.
+     * 
      */
     public void destroy() {
     }
@@ -180,18 +175,20 @@ public class Controller implements IController, CommandProvider {
     public void addMessageListener(OFType type, IMessageListener listener) {
         IMessageListener currentListener = this.messageListeners.get(type);
         if (currentListener != null) {
-            logger.warn(type.toString() + " already listened by "
-                    + currentListener.toString());
+            logger.warn("{} is already listened by {}", type.toString(),
+                    currentListener.toString());
         }
         this.messageListeners.put(type, listener);
-        logger.debug(type.toString() + " is now listened by "
-                + listener.toString());
+        logger.debug("{} is now listened by {}", type.toString(),
+                listener.toString());
     }
 
     @Override
     public void removeMessageListener(OFType type, IMessageListener listener) {
         IMessageListener currentListener = this.messageListeners.get(type);
         if ((currentListener != null) && (currentListener == listener)) {
+            logger.debug("{} listener {} is Removed", type.toString(),
+                    listener.toString());
             this.messageListeners.remove(type);
         }
     }
@@ -199,17 +196,20 @@ public class Controller implements IController, CommandProvider {
     @Override
     public void addSwitchStateListener(ISwitchStateListener listener) {
         if (this.switchStateListener != null) {
-            logger.warn(this.switchStateListener.toString()
-                    + "already listened to switch events");
+            logger.warn("Switch events are already listened by {}",
+                    this.switchStateListener.toString());
         }
         this.switchStateListener = listener;
-        logger.debug(listener.toString() + " now listens to switch events");
+        logger.debug("Switch events are now listened by {}",
+                listener.toString());
     }
 
     @Override
     public void removeSwitchStateListener(ISwitchStateListener listener) {
         if ((this.switchStateListener != null)
                 && (this.switchStateListener == listener)) {
+            logger.debug("SwitchStateListener {} is Removed",
+                    listener.toString());
             this.switchStateListener = null;
         }
     }
@@ -227,7 +227,11 @@ public class Controller implements IController, CommandProvider {
             SwitchHandler switchHandler = new SwitchHandler(this, sc,
                     instanceName);
             switchHandler.start();
-            logger.info(instanceName + " connected: " + sc.toString());
+            if (sc.isConnected()) {
+                logger.info("Switch:{} is connected to the Controller", sc
+                        .getRemoteAddress().toString().split("/")[1]);
+            }
+
         } catch (IOException e) {
             return;
         }
@@ -237,11 +241,8 @@ public class Controller implements IController, CommandProvider {
         if (((SwitchHandler) sw).isOperational()) {
             Long sid = sw.getId();
             if (this.switches.remove(sid, sw)) {
-                logger.warn(sw.toString() + " is disconnected");
+                logger.warn("{} is Disconnected", sw.toString());
                 notifySwitchDeleted(sw);
-            } else {
-                //logger.warn(sw.toString() + " has been replaced by " +
-                //     this.switches.get(sid));
             }
         }
         ((SwitchHandler) sw).stop();
@@ -268,7 +269,7 @@ public class Controller implements IController, CommandProvider {
         }
     }
 
-    public void takeSwtichEventAdd(ISwitch sw) {
+    public void takeSwitchEventAdd(ISwitch sw) {
         SwitchEvent ev = new SwitchEvent(
                 SwitchEvent.SwitchEventType.SWITCH_ADD, sw, null);
         addSwitchEvent(ev);
@@ -317,7 +318,8 @@ public class Controller implements IController, CommandProvider {
         while (iter.hasNext()) {
             Long sid = iter.next();
             Date date = switches.get(sid).getConnectedDate();
-            String switchInstanceName = ((SwitchHandler) switches.get(sid)).getInstanceName();
+            String switchInstanceName = ((SwitchHandler) switches.get(sid))
+                    .getInstanceName();
             s.append(switchInstanceName + "/" + HexString.toHexString(sid)
                     + " connected since " + date.toString() + "\n");
         }
@@ -369,7 +371,7 @@ public class Controller implements IController, CommandProvider {
     @Override
     public String getHelp() {
         StringBuffer help = new StringBuffer();
-        help.append("-- Open Flow Controller --\n");
+        help.append("---Open Flow Controller---\n");
         help.append("\t controllerShowSwitches\n");
         help.append("\t controllerReset\n");
         help.append("\t controllerShowConnConfig\n");

diff --git a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/ControllerIO.java b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/ControllerIO.java
index a4b75843376071ba402729f63e591c2e7bb0b7fe..868e2086511b5cd7926ccb08275cbda6bb270c9d 100644 (file)
--- a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/ControllerIO.java
+++ b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/ControllerIO.java
@@ -1,4 +1,3 @@
-
 /*
  * Copyright (c) 2013 Cisco Systems, Inc. and others.  All rights reserved.
  *
@@ -40,8 +39,8 @@ public class ControllerIO {
             try {
                 openFlowPort = Short.decode(portString).shortValue();
             } catch (NumberFormatException e) {
-                logger.warn("Invalid port:" + portString + ", use default("
-                        + openFlowPort + ")");
+                logger.warn("Invalid port:{}, use default({})", portString,
+                        openFlowPort);
             }
         }
     }
@@ -72,8 +71,8 @@ public class ControllerIO {
                             SelectionKey skey = selectedKeys.next();
                             selectedKeys.remove();
                             if (skey.isValid() && skey.isAcceptable()) {
-                                 ((Controller) listener).handleNewConnection(selector,
-                                        serverSelectionKey);
+                                ((Controller) listener).handleNewConnection(
+                                        selector, serverSelectionKey);
                             }
                         }
                     } catch (Exception e) {
@@ -83,7 +82,7 @@ public class ControllerIO {
             }
         }, "ControllerI/O Thread");
         controllerIOThread.start();
-        logger.info("Controller is now listening on port " + openFlowPort);
+        logger.info("Controller is now listening on port {}", openFlowPort);
     }
 
     public void shutDown() throws IOException {

diff --git a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/SwitchHandler.java b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/SwitchHandler.java
index bbf3e91462a0756417c18fabed8af2e52abea09f..ea659c82e12929283d766a08e5eaa442b3e06b55 100644 (file)
--- a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/SwitchHandler.java
+++ b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/SwitchHandler.java
@@ -356,11 +356,6 @@ public class SwitchHandler implements ISwitch {
         }
         for (OFMessage msg : msgs) {
             logger.trace("Message received: {}", msg.toString());
-            /*
-             * if ((msg.getType() != OFType.ECHO_REQUEST) && (msg.getType() !=
-             * OFType.ECHO_REPLY)) { logger.debug(msg.getType().toString() +
-             * " received from sw " + toString()); }
-             */
             this.lastMsgReceivedTimeStamp = System.currentTimeMillis();
             OFType type = msg.getType();
             switch (type) {
@@ -422,21 +417,14 @@ public class SwitchHandler implements ISwitch {
     }
 
     private void processPortStatusMsg(OFPortStatus msg) {
-        // short portNumber = msg.getDesc().getPortNumber();
         OFPhysicalPort port = msg.getDesc();
         if (msg.getReason() == (byte) OFPortReason.OFPPR_MODIFY.ordinal()) {
             updatePhysicalPort(port);
-            // logger.debug("Port " + portNumber + " on " + toString() +
-            // " modified");
         } else if (msg.getReason() == (byte) OFPortReason.OFPPR_ADD.ordinal()) {
             updatePhysicalPort(port);
-            // logger.debug("Port " + portNumber + " on " + toString() +
-            // " added");
         } else if (msg.getReason() == (byte) OFPortReason.OFPPR_DELETE
                 .ordinal()) {
             deletePhysicalPort(port);
-            // logger.debug("Port " + portNumber + " on " + toString() +
-            // " deleted");
         }
 
     }
@@ -457,8 +445,9 @@ public class SwitchHandler implements ISwitch {
                             reportSwitchStateChange(false);
                         } else {
                             // send a probe to see if the switch is still alive
-                            // logger.debug("Send idle probe (Echo Request) to "
-                            // + switchName());
+                            logger.debug(
+                                    "Send idle probe (Echo Request) to {}",
+                                    toString());
                             probeSent = true;
                             OFMessage echo = factory
                                     .getMessage(OFType.ECHO_REQUEST);
@@ -500,8 +489,7 @@ public class SwitchHandler implements ISwitch {
     private void reportError(Exception e) {
         if (e instanceof AsynchronousCloseException
                 || e instanceof InterruptedException
-                || e instanceof SocketException
-                || e instanceof IOException) {
+                || e instanceof SocketException || e instanceof IOException) {
             logger.debug("Caught exception {}", e.getMessage());
         } else {
             logger.warn("Caught exception ", e);
@@ -512,7 +500,7 @@ public class SwitchHandler implements ISwitch {
 
     private void reportSwitchStateChange(boolean added) {
         if (added) {
-            ((Controller) core).takeSwtichEventAdd(this);
+            ((Controller) core).takeSwitchEventAdd(this);
         } else {
             ((Controller) core).takeSwitchEventDelete(this);
         }
@@ -567,7 +555,7 @@ public class SwitchHandler implements ISwitch {
                                                 .getValue()
                                         | OFPortFeatures.OFPPF_1GB_HD
                                                 .getValue() | OFPortFeatures.OFPPF_10GB_FD
-                                        .getValue()));
+                                            .getValue()));
     }
 
     private void deletePhysicalPort(OFPhysicalPort port) {
@@ -583,11 +571,16 @@ public class SwitchHandler implements ISwitch {
 
     @Override
     public String toString() {
-        return ("["
-                + this.socket.toString()
-                + " SWID "
-                + (isOperational() ? HexString.toHexString(this.sid)
-                        : "unkbown") + "]");
+        try {
+            return ("Switch:"
+                    + socket.getRemoteAddress().toString().split("/")[1]
+                    + " SWID:" + (isOperational() ? HexString
+                    .toHexString(this.sid) : "unknown"));
+        } catch (Exception e) {
+            return (isOperational() ? HexString.toHexString(this.sid)
+                    : "unknown");
+        }
+
     }
 
     @Override
@@ -823,14 +816,14 @@ public class SwitchHandler implements ISwitch {
     }
 
     /**
-     * Sends synchronous Barrier message 
+     * Sends synchronous Barrier message
      */
     @Override
     public Object sendBarrierMessage() {
         OFBarrierRequest barrierMsg = new OFBarrierRequest();
-        return syncSend(barrierMsg);        
+        return syncSend(barrierMsg);
     }
-    
+
     /**
      * This method returns the switch liveness timeout value. If controller did
      * not receive any message from the switch for such a long period,