[jboss-cvs] JBossAS SVN: r110498 - branches/snmp4j-int/varia/src/main/java/org/jboss/jmx/adaptor/snmp/agent.

jboss-cvs-commits at lists.jboss.org jboss-cvs-commits at lists.jboss.org
Fri Jan 28 17:03:16 EST 2011


Author: thauser at redhat.com
Date: 2011-01-28 17:03:16 -0500 (Fri, 28 Jan 2011)
New Revision: 110498

Modified:
   branches/snmp4j-int/varia/src/main/java/org/jboss/jmx/adaptor/snmp/agent/RequestHandlerImpl.java
Log:
minor code cleanup and variable renaming for better indication of purpose.

Modified: branches/snmp4j-int/varia/src/main/java/org/jboss/jmx/adaptor/snmp/agent/RequestHandlerImpl.java
===================================================================
--- branches/snmp4j-int/varia/src/main/java/org/jboss/jmx/adaptor/snmp/agent/RequestHandlerImpl.java	2011-01-28 21:16:45 UTC (rev 110497)
+++ branches/snmp4j-int/varia/src/main/java/org/jboss/jmx/adaptor/snmp/agent/RequestHandlerImpl.java	2011-01-28 22:03:16 UTC (rev 110498)
@@ -64,7 +64,8 @@
 
 /**
  * Implement RequestHandler with mapping of snmp get/set requests
- * to JMX mbean attribute gets/sets
+ * to JMX mbean attribute gets/sets. Currently only v1 / v2 PDUs are supported by the 
+ * agent service. 
  *
  * @author <a href="mailto:hwr at pilhuhn.de>">Heiko W. Rupp</a>
  * @author <a href="mailto:dimitris at jboss.org">Dimitris Andreadis</a>
@@ -83,8 +84,10 @@
 	/** Bindings from oid to mbean */
 	protected SortedMap bindings = new TreeMap();
 	
+	/** keep track of the instances of variables */
 	private SortedSet oidKeys = null;
-	/** keep track of the objects created **/
+	
+	/** keep track of the objects created */
 	private SortedSet objectKeys = null;
 
 	/** Has this RequestHandler instance been initialized? */
@@ -178,10 +181,6 @@
     * 
     */
    
-   // TODO: find out the proper getBulk behavior, for certain. 
-   //       - should getBulk fail if the total amount of attributes in a tree is < mR for a repeater?
-   //		- or should all values in that tree be returned, with no message?
-   // this method is a work in progress.
    	public PDU snmpReceivedGetBulk(PDU pdu){
    		PDU response;
    		
@@ -201,108 +200,106 @@
 			log.trace("requestID=" + pdu.getRequestID() + ", elementCount="
 					+ pdu.size());
 		}
-	//	if (pdu != null){ this is already checked in SnmpRequest.
-			int counter = 1; 
-			int nonRepeaters = pdu.getNonRepeaters();
-			if (nonRepeaters<0)
-				nonRepeaters=0;
+
+		int errorIndex = 1; 
+		int nonRepeaters = pdu.getNonRepeaters();
+		if (nonRepeaters<0)
+			nonRepeaters=0;
+		
+		int maxRepetitions = pdu.getMaxRepetitions();
+		if (maxRepetitions<0)
+			maxRepetitions=0;
+		Vector<VariableBinding> vbList = pdu.getVariableBindings();
+		VariableBinding vb = null;
+		Variable var = null;
+		
+		if (vbList.size() == 0){ // we were given an empty list in the PDU
+			log.debug("snmpReceivedGetBulk: No VariableBindings in received PDU");
+			makeErrorPdu(response, pdu, errorIndex, PDU.genErr);
+			return response;
+		}
+		for (int i=0; i < Math.min(nonRepeaters, vbList.size());i++){
+			vb = (VariableBinding)vbList.get(i);
+			OID oid = vb.getOid();
+			OID noid = null;
 			
-			int maxRepetitions = pdu.getMaxRepetitions();
-			if (maxRepetitions<0)
-				maxRepetitions=0;
-			Vector<VariableBinding> vbList = pdu.getVariableBindings();
-			VariableBinding vb = null;
-			Variable var = null;
-			
-			if (vbList.size() == 0){ // we were given an empty list in the PDU
-				log.debug("snmpReceivedGetBulk: No VariableBindings in received PDU");
-				makeErrorPdu(response, pdu, counter, PDU.genErr);
+			try { 
+				noid = getNextOid(oid);
+			}
+			catch (EndOfMibViewException e){
+				log.debug("snmpReceivedGetBulk: End of MIB View in NonRepeaters.");
+				var = Null.endOfMibView;
+				response.add(new VariableBinding(oid, var));
+				continue;
+			}
+			try {
+				var = getValueFor(noid);
+			} 
+			catch (NoSuchInstanceException e){ 
+				// this won't happen because of using getnext;
+				log.debug("snmpReceivedGetBulk: An instance of an OID didn't exist.");
+				response.add(new VariableBinding(oid, Null.noSuchInstance));
+				continue;
+			} 
+			catch (VariableTypeException e){
+				log.debug("snmpReceivedGetBulk: Couldn't convert a Variable to a correct type.");
+				makeErrorPdu(response, pdu, errorIndex, PDU.genErr);
 				return response;
 			}
-
-			for (int i=0; i < Math.min(nonRepeaters, vbList.size());i++){
-				vb = (VariableBinding)vbList.get(i);
-				OID oid = vb.getOid();
-				OID noid = null;
-				
-				try { 
-					noid = getNextOid(oid);
-				}
-				catch (EndOfMibViewException e){
-					log.debug("snmpReceivedGetBulk: End of MIB View in NonRepeaters.");
-					var = Null.endOfMibView;
-					response.add(new VariableBinding(oid, var));
-					continue;
-				}
+			response.add(new VariableBinding(noid,var));	
+			errorIndex++;
+		}
+		
+			// for the remaining it.size() bindings, we perform maxRepetitions successive getNext calls
+		for (int i = nonRepeaters; i < vbList.size(); i++){
+			vb = (VariableBinding)vbList.get(i);
+			OID oid = vb.getOid();
+			OID noid = null;
+			try {
+				noid = getNextOid(oid);
+			}
+			
+			catch (EndOfMibViewException e) {
+				// we know that doing more getnext in this case will just give more end of MIB view results. so we try the next VB.
+				response.add(new VariableBinding(oid, Null.endOfMibView));
+				continue;
+			}
+							
+			for (int j = 0; j < maxRepetitions; j++){
 				try {
 					var = getValueFor(noid);
 				} 
-				catch (NoSuchInstanceException e){ 
-					// this won't happen because of using getnext;
-					log.debug("snmpReceivedGetBulk: An instance of an OID didn't exist.");
-					response.add(new VariableBinding(oid, Null.noSuchInstance));
-					continue;
+				// since we're using getNext to retrieve the OIDs, this can't happen. we'll always have an instance.
+				catch (NoSuchInstanceException e) {
+					var = Null.noSuchInstance;						
 				} 
-				catch (VariableTypeException e){
-					log.debug("snmpReceivedGetBulk: Couldn't convert a Variable to a correct type.");
-					makeErrorPdu(response, pdu, counter, PDU.genErr);
-					return response;
+				catch (VariableTypeException e) {
+					makeErrorPdu(response, pdu, errorIndex, PDU.genErr);
+					var = Null.instance;						
 				}
-				response.add(new VariableBinding(noid,var));	
-				counter++;
-			}
-			
-			// for the remaining it.size() bindings, we perform maxRepetitions successive getNext calls
-			for (int i = nonRepeaters; i < vbList.size(); i++){
-				vb = (VariableBinding)vbList.get(i);
-				OID oid = vb.getOid();
-				OID noid = null;
-				try {
-					noid = getNextOid(oid);
-				}
 				
+				response.add(new VariableBinding(noid,var));
+				
+				try {
+					noid = getNextOid(noid);
+				} 
 				catch (EndOfMibViewException e) {
-					// we know that doing more getnext in this case will just give more end of MIB view results. so we try the next VB.
-					response.add(new VariableBinding(oid, Null.endOfMibView));
-					continue;
+					// doing more repetitions here is a waste of time. 
+					response.add(new VariableBinding(noid, Null.endOfMibView));
+					break;
 				}
-								
-				for (int j = 0; j < maxRepetitions; j++){
-					try {
-						var = getValueFor(noid);
-					} 
-					// since we're using getNext to retrieve the OIDs, this can't happen. we'll always have an instance.
-					catch (NoSuchInstanceException e) {
-						var = Null.noSuchInstance;						
-					} 
-					catch (VariableTypeException e) {
-						makeErrorPdu(response, pdu, counter, PDU.genErr);
-						var = Null.instance;						
-					}
-					
-					response.add(new VariableBinding(noid,var));
-					
-					try {
-						noid = getNextOid(noid);
-					} 
-					catch (EndOfMibViewException e) {
-						// doing more repetitions here is a waste of time. 
-						response.add(new VariableBinding(noid, Null.endOfMibView));
-						break;
-					}
-				}
 			}
-			counter++;		
-	//	}	
+			errorIndex++;	
+		}
 		return response;
    	}
    	   
     /**
 	 * <P>
-	 * This method is defined to handle SNMP Get requests that are received by
+	 * This method is defined to handle SNMP GET and GETNEXT requests that are received by
 	 * the session. The request has already been validated by the system. This
 	 * routine will build a response and pass it back to the caller.
-	 * The behaviour is defined in RFC-3416
+	 * The behaviour is defined in RFC-3416 for v2 protocols.
 	 * </P>
 	 * 
 	 * @param pdu
@@ -316,15 +313,20 @@
 	{
 		PDU response;
 		// this counts the number of VariableBindings and indicates which one caused a problem, if any
-		int counter = 1;
+		int errorIndex = 1;
 		// flags the end of the MIB 
 		boolean endOfMib = false;
 		
 		if (pdu instanceof ScopedPDU){
+			/* ScopedPDUs are not fully supported. this check is here so that when
+			 * SnmpAgentService is updated to support v3 correctly, we won't have to 
+			 * add it in.*/
 			response = DefaultPDUFactory.createPDU(SnmpConstants.version3);
 		} else if (pdu instanceof PDUv1){
+			// v1
 			response = DefaultPDUFactory.createPDU(SnmpConstants.version1);
 		} else {
+			// v2c
 			response = new PDU();
 		} 
 		response.setType(PDU.RESPONSE);
@@ -337,85 +339,77 @@
 						+ pdu.size());
 			}
 		
-		if (pdu != null){
-				
-			Iterator it = pdu.getVariableBindings().iterator();
-			VariableBinding newVB;
-			Variable var;
-			OID noid = null;
+		Iterator it = pdu.getVariableBindings().iterator();
+		VariableBinding newVB;
+		Variable var;
+		OID noid = null;
 			
-			while (it.hasNext()){
-				VariableBinding vb = (VariableBinding)it.next();
-				OID oid = vb.getOid();
+		while (it.hasNext()){
+			VariableBinding vb = (VariableBinding)it.next();
+			OID oid = vb.getOid();
+			
+			if (pdu.getType()==PDU.GETNEXT){
+				// we get the lexicographically next OID from the given one.
+				try{
+				noid = getNextOid(oid);
+				}
+				// if there are no lexicographically larger OIDs than this one
+				catch(EndOfMibViewException e){
+					log.debug("snmpReceivedGet: GETNEXT operation left the MIB View");
+					endOfMib=true;
+				}
 				
-				if (pdu.getType()==PDU.GETNEXT){
-					// we get the lexically next OID from the given one.
-					try{
-					noid = getNextOid(oid);
+				if (endOfMib){
+					newVB = new VariableBinding(oid);
+					var = Null.endOfMibView;
+				}
+				else{
+					newVB = new VariableBinding(noid);
+					try {
+						var = getValueFor(noid);
 					}
-					// if this OID is the last one possible, we throw this exception.
-					catch(EndOfMibViewException e){
-						log.debug("snmpReceivedGet: GETNEXT operation left the MIB View");
-						endOfMib=true;
-						//makeErrorPdu(oid,PDU.noAccess, counter,response);
-
+					catch (NoSuchInstanceException e) {
+						log.debug("snmpReceivedGet: GETNEXT operation returned null. No such OID.");
+						var = Null.noSuchInstance;
 					}
-					
-					if (endOfMib){
-						newVB = new VariableBinding(oid);
-						var = Null.endOfMibView;
+					catch (VariableTypeException e) {
+						log.debug("snmpReceivedGet: GETNEXT operation could not convert the returned value for " + noid + " into an appropriate type.");
+						makeErrorPdu(response, pdu, errorIndex, PDU.genErr);
+						return response;
 					}
-					else{
-						newVB = new VariableBinding(noid);
-						try {
-							var = getValueFor(noid);
-						}
-						catch (NoSuchInstanceException e) {
-							log.debug("snmpReceivedGet: GETNEXT operation returned null. No such OID.");
-							var = Null.noSuchInstance;
-							//makeErrorPdu(noid, PDU.noSuchName, counter,response);
-
-						}
-						catch (VariableTypeException e) {
-							log.debug("snmpReceivedGet: GETNEXT operation could not convert the returned value for " + noid + " into an appropriate type.");
-							makeErrorPdu(response, pdu, counter, PDU.genErr);
-							return response;
-						}
-					}
 				}
-				else {
-					newVB = new VariableBinding(oid);
-					var = null;
-					if (checkObject(oid)){				
-						try {
-							var = getValueFor(oid);
-						}
-						catch (NoSuchInstanceException e) {
-							log.debug("snmpReceivedGet: GET operation returned null. No such Instance.");
-							var = Null.noSuchInstance;
-							//makeErrorPdu(oid, PDU.noSuchName, counter, response);
-						}
-						catch (VariableTypeException e) {
-							log.debug("snmpReceivedGet: GET operation could not convert the returned value for " + oid + " into an appropriate type.");
-							makeErrorPdu(response, pdu, counter, PDU.genErr);
-							return response;
-							
-
-						}
-						
+			}
+			else {
+				newVB = new VariableBinding(oid);
+				var = null;
+				// check the existence of the object for the requested instance
+				// the object is the OID with the last number removed.
+				if (checkObject(oid)){				
+					try {
+						var = getValueFor(oid);
 					}
-					// if we get here, there's no such object.
-					else{
-						var = Null.noSuchObject;						
+					catch (NoSuchInstanceException e) {
+						log.debug("snmpReceivedGet: GET operation returned null. No such Instance.");
+						var = Null.noSuchInstance;
 					}
+					catch (VariableTypeException e) {
+						log.debug("snmpReceivedGet: GET operation could not convert the returned value for " + oid + " into an appropriate type.");
+						makeErrorPdu(response, pdu, errorIndex, PDU.genErr);
+						return response;
 						
+						}
+					
 				}
-					newVB.setVariable(var);
-					response.add(newVB);
-					counter++;
-		 	}
-			
-		}
+				// if we get here, there's no such object.
+				else{
+					var = Null.noSuchObject;						
+				}
+					
+			}
+			newVB.setVariable(var);
+			response.add(newVB);
+			errorIndex++;
+	 	}
 		//TODO: check size constraints of the sender
 		return response;
 	}	
@@ -424,7 +418,9 @@
 	 * <P>
 	 * This method is defined to handle SNMP Set requests that are received by
 	 * the session. The request has already been validated by the system. This
-	 * routine will build a response and pass it back to the caller.
+	 * routine will build a response and pass it back to the caller. 
+	 * Upon any error, this method will rollback all sets that were made before the 
+	 * failure occured.
 	 * The behaviour is defined in RFC-3416
 	 * </P>
 	 * 
@@ -440,9 +436,8 @@
    {
 		PDU response;
 		// the modified OID entries so far.  
-		HashSet<VariableBinding> modified = new HashSet<VariableBinding>(); // modified variable values so far
-		// this counter is so we can indicate which entry in the given VB list caused an error, if any
-		int counter = 1;
+		HashSet<VariableBinding> modified = new HashSet<VariableBinding>(); 
+		int errorIndex = 1;
 		Variable var, oldVar = null; // oldVar variable for storing into modified
 		
 		if (pdu instanceof ScopedPDU){
@@ -461,115 +456,65 @@
 					+ pdu.size());
 		}
 		
-		if (pdu != null){
-			//iterate through the VB in this PDU
-			Iterator<VariableBinding> it = pdu.getVariableBindings().iterator();
+		//iterate through the VB in this PDU
+		Iterator<VariableBinding> it = pdu.getVariableBindings().iterator();
+		
+		while (it.hasNext()){
+			// if any set fails for any reason, changes must be undone. 
+			// so changes that pass all tests should be stored, and all applied
+			// after each VB has been checked. 
+			VariableBinding vb = it.next();
+			OID oid = vb.getOid();
+			Variable newVal = vb.getVariable();
+			//setup the new variable binding to put into the response pdu
+			VariableBinding newVB = new VariableBinding(oid,newVal);
 			
-			while (it.hasNext()){
-				// if any set fails for any reason, changes must be undone. 
-				// so changes that pass all tests should be stored, and all applied
-				// after each VB has been checked. 
-				VariableBinding vb = it.next();
-				OID oid = vb.getOid();
-				Variable newVal = vb.getVariable();
-				//setup the new variable binding to put into the response pdu
-				VariableBinding newVB = new VariableBinding(oid,newVal);
-				
-				//some errorchecking ....
-				//else is good and do:
-				try{
+			try{
 				oldVar = getValueFor(oid);
 				modified.add(new VariableBinding(oid, oldVar)); // keep a record of the old variable binding.
 				if (checkObject(oid)){
 					var = setValueFor(oid,newVal);
 				}
 				else{
-					log.debug("snmpReceivedSet: no object for setting");
+					log.debug("snmpReceivedSet: no object for SET request");
 					undoSets(modified);
-					makeErrorPdu(response, pdu, counter, PDU.noAccess);
+					makeErrorPdu(response, pdu, errorIndex, PDU.noAccess);
 					return response;
 				}
-				}
-				catch (NoSuchInstanceException e){
-					log.debug("snmpReceivedSet: attempt to set a non-existent OID " + oid);
-					undoSets(modified);
-					makeErrorPdu(response, pdu, counter, PDU.noCreation);
-					return response;
-				}
-				catch (VariableTypeException e){
-					log.debug("snmpReceievedSet: could not convert the given value into an appropriate type: " +newVal);
-					undoSets(modified);
-					makeErrorPdu(response, pdu, counter, PDU.wrongType);
-					return response;
-				}
-				catch (ReadOnlyException e){
-					log.debug("snmpReceivedSet: attempt to set a read-only attribute: " + newVB);
-					undoSets(modified);
-					makeErrorPdu(response, pdu, counter, PDU.notWritable);
-					return response;
-				}
-				catch (Exception e){
-					log.debug("snmpReceivedSet: catastrophe!!! General variable validation error." + " " + e);
-					e.printStackTrace();
-					undoSets(modified);
-					makeErrorPdu(response, pdu, counter, PDU.genErr);
-					return response;
-				}
-				
-				// if we get here, we modified the value successfully. 
-				response.add(newVB);
-				counter++;
 			}
+			catch (NoSuchInstanceException e){
+				log.debug("snmpReceivedSet: attempt to set a non-existent instance: " + oid.last() + " of object: " + oid.trim());
+				undoSets(modified);
+				makeErrorPdu(response, pdu, errorIndex, PDU.noCreation);
+				return response;
+			}
+			catch (VariableTypeException e){
+				log.debug("snmpReceievedSet: could not convert the given value into an appropriate type: " +newVal);
+				undoSets(modified);
+				makeErrorPdu(response, pdu, errorIndex, PDU.wrongType);
+				return response;
+			}
+			catch (ReadOnlyException e){
+				log.debug("snmpReceivedSet: attempt to set a read-only attribute: " + newVB);
+				undoSets(modified);
+				makeErrorPdu(response, pdu, errorIndex, PDU.notWritable);
+				return response;
+			}
+			catch (Exception e){
+				log.debug("snmpReceivedSet: catastrophe!!! General variable validation error." + " " + e);
+				e.printStackTrace();
+				undoSets(modified);
+				makeErrorPdu(response, pdu, errorIndex, PDU.genErr);
+				return response;
+			}
+			
+			// if we get here, we modified the value successfully. 
+			response.add(newVB);
+			errorIndex++;
 		}
-
-		return response;
-	}		
+	return response;
+   }		
 			
-		//return null;
-//		final boolean trace = log.isTraceEnabled();
-//		SnmpPduRequest response = null;
-//		int errorStatus = SnmpPduPacket.ErrNoError;
-//		int errorIndex = 0;
-//		int k = pdu.getLength();
-//		SnmpVarBind[] vblist = new SnmpVarBind[k];
-//
-//		for (int i = 0; i < k; i++)
-//      {
-//			SnmpVarBind vb = pdu.getVarBindAt(i);
-//			vblist[i] = new SnmpVarBind(vb);
-//			SnmpObjectId oid = vb.getName();
-//			SnmpSyntax newVal = vb.getValue();
-//			if (trace)
-//				log.trace("set: received oid " + oid.toString() + " with value " + newVal.toString());
-//			SnmpSyntax result = null;
-//			try
-//         {
-//				result = setValueFor(oid,newVal);
-//			}
-//         catch (ReadOnlyException e)
-//         {
-//				errorStatus = SnmpPduPacket.ErrReadOnly;
-//				errorIndex = i + 1;
-//			}
-//
-//			 if (result != null)
-//			 {
-//				errorStatus = SnmpPduPacket.ErrReadOnly;
-//				errorIndex = i + 1;
-//				log.debug("Error occured " + vb.getName().toString());
-//			 }
-//
-//			 if (trace)
-//          {
-//				 log.trace("Varbind[" + i + "] := " + vb.getName().toString());
-//				 log.trace(" --> " + vb.getValue().toString());
-//			 }
-//		}
-//		response = new SnmpPduRequest(SnmpPduPacket.RESPONSE, vblist);
-//		response.setErrorStatus(errorStatus);
-//		response.setErrorIndex(errorIndex);
-//
-
 	/**
 	 * <P>
 	 * This method is defined to handle SNMP requests that are received by the
@@ -1115,10 +1060,10 @@
 	 * @param errNo The error number defined in the PDU class that indicates a given failure
 	 * @param errInd the VariableBinding in the PDU that caused the error. 
 	 */
-	private void makeErrorPdu(PDU response, PDU pdu, int counter, int err){
+	private void makeErrorPdu(PDU response, PDU pdu, int errorIndex, int err){
 		response.clear();
 		response.addAll(pdu.toArray());
-		response.setErrorIndex(counter);
+		response.setErrorIndex(errorIndex);
 		response.setErrorStatus(err);
 	}
 



More information about the jboss-cvs-commits mailing list