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

jboss-cvs-commits at lists.jboss.org jboss-cvs-commits at lists.jboss.org
Wed Dec 8 17:09:55 EST 2010


Author: thauser at redhat.com
Date: 2010-12-08 17:09:54 -0500 (Wed, 08 Dec 2010)
New Revision: 109807

Modified:
   branches/snmp4j-integration-1.11.1/varia/src/main/java/org/jboss/jmx/adaptor/snmp/agent/RequestHandlerImpl.java
Log:
modifications to how snmpreceivedset handles errors. 

Modified: branches/snmp4j-integration-1.11.1/varia/src/main/java/org/jboss/jmx/adaptor/snmp/agent/RequestHandlerImpl.java
===================================================================
--- branches/snmp4j-integration-1.11.1/varia/src/main/java/org/jboss/jmx/adaptor/snmp/agent/RequestHandlerImpl.java	2010-12-08 22:06:08 UTC (rev 109806)
+++ branches/snmp4j-integration-1.11.1/varia/src/main/java/org/jboss/jmx/adaptor/snmp/agent/RequestHandlerImpl.java	2010-12-08 22:09:54 UTC (rev 109807)
@@ -28,6 +28,8 @@
 import java.util.SortedMap;
 import java.util.SortedSet;
 import java.util.TreeMap;
+import java.util.HashMap;
+import java.util.HashSet;
 import java.util.TreeSet;
 import java.util.Vector;
 
@@ -164,7 +166,7 @@
     * 		a list of OIDs 
     * 		an Integer > 0 indicating the number of non-repeaters (set with PDU.setNonRepeaters())
     * 		an Integer >= 0 indicating the number of repetitions   (set with PDU.setMaxRepetitions())
-    * @return a PDU filled with the appropriate values.
+    * @return a PDU filled with the appropriate values, and statuses indicating errors.
     * 
     */
    
@@ -211,22 +213,10 @@
 				var = getValueFor(oid);
 				
 				if (var instanceof Null){
-					//no bind entry found. so the OID doesn't have a binding / doesn't exist
-					//so we don't add it, and move to the next binding.
-					//response.add(new VariableBinding(new OID(), Null.noSuchObject));
-					//response.setErrorStatus(PDU.noSuchName);
-					//response.setErrorIndex(i+1);
-					//return response;
 					continue;
 				}
 				
 				response.add(new VariableBinding(oid,var));	
-					/*if (oid == null){ // OID can never be null here. See VariableBinding.java
-						log.debug("snmpReceivedGetBulk: Attempt to GETBULK on non-existent scalar OID.");
-						response.setErrorStatus(PDU.noSuchName);
-						response.setErrorIndex(i);
-						return response;
-					}*/
 			}
 			
 			// for the remaining it.size() bindings, we perform maxRepetitions successive getNext calls
@@ -271,6 +261,7 @@
 	public PDU snmpReceivedGet(PDU pdu)
 	{
 		PDU response;
+		// this counts the number of VariableBindings and indicates which one caused a problem, if any
 		int counter = 1;
 		
 		if (pdu instanceof ScopedPDU){
@@ -303,11 +294,12 @@
 					// we get the lexically next OID from the given one.
 					OID noid = getNextOid(oid,true);
 					// if noid is null, there is no OID > the one given. so we change response's errorStatus.
+					// we also abort here. 
 					if (noid == null){
 						log.debug("snmpReceivedGet: getNextOid operation returned null. No such OID.");
 						response.setErrorStatus(PDU.noSuchName);
 						response.setErrorIndex(counter);
-						response.add(new VariableBinding(oid, Null.noSuchObject));
+						response.add(new VariableBinding(noid, Null.noSuchObject));
 						return response;
 					}
 					newVB = new VariableBinding(noid);
@@ -316,6 +308,14 @@
 				else {
 					newVB = new VariableBinding(oid);
 					var = getValueFor(oid);
+					// the OID we are looking for has no BindEntry. Indicate this in the returned pdu.
+					// and abort immediately.
+					if (var instanceof Null){
+						response.setErrorStatus(PDU.noSuchName);
+						response.setErrorIndex(counter);
+						response.add(newVB);
+						return response;
+					}
 				}
 					newVB.setVariable(var);
 					response.add(newVB);
@@ -336,12 +336,15 @@
 	 * @param pdu
 	 *           The SNMP pdu
 	 * 
-	 * @return SnmpPduRequest filled in with the proper response, or null if
-	 *         cannot process NOTE: this might be changed to throw an exception.
+	 * @return PDU filled with the new value of the given OID and an error of 0 on success,
+	 * 		   or a PDU with an empty VB List and an error status indicating why the operation failed,
+	 * 		   and which PDU was problematic. 
 	 */
 	public PDU snmpReceivedSet(PDU pdu)
    {
 		PDU response;
+		HashMap modified = new HashMap(); // the modified OID entries so far. 
+		// this counter is so we can indicate which entry in the given VB list caused an error, if any
 		int counter = 1;
 		Variable var = null;
 		if (pdu instanceof ScopedPDU){
@@ -365,6 +368,9 @@
 			Iterator 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 = (VariableBinding)it.next();
 				OID oid = vb.getOid();
 				Variable newVal = vb.getVariable();
@@ -377,11 +383,12 @@
 				var = setValueFor(oid,newVal);
 				}
 				catch (ReadOnlyException e){
-					// this reaction needs to be turned into an exception in all cases. 
+					// this reaction needs to be turned into an exception in all cases.
 					log.debug("snmpReceivedSet: attempt to set a read-only attribute: " + newVB);
 					response.setErrorIndex(counter);
-					response.setErrorStatus(PDU.readOnly); 
-					response.add(newVB);
+					response.setErrorStatus(PDU.readOnly);
+					undoSets(modified);
+		//			response.add(newVB);
 					return response;
 				}
 				
@@ -390,8 +397,11 @@
 					response.setErrorStatus(PDU.noSuchName);
 					response.setErrorIndex(counter);
 					response.add(new VariableBinding(oid, Null.noSuchObject));
+					undoSets(modified);
 					return response;
 				}
+				// if we get here, we modified the value successfully. 
+				modified.put(oid,newVal);
 				response.add(newVB);
 				counter++;
 			}
@@ -637,72 +647,34 @@
      }
      else
      {
-			ssy = Null.noSuchInstance;
+			ssy = Null.noSuchObject;
 			log.info("getValueFor: " + NO_ENTRY_FOUND_FOR_OID + oid);
 		}
 		return ssy;
 	}
 
-//	/**
-//	 * Return the current value for the given oid
-//	 * 
-//	 * @param oid
-//	 *            The oid we want a value for
-//	 * @return SnmpNull if no value present
-//	 */
-//	private SnmpSyntax getValueFor(final SnmpObjectId oid) {
-//
-//		BindEntry be = findBindEntryForOid(oid);
-//		SnmpSyntax ssy = null;
-//		if (be != null)
-//      {
-//			if (log.isTraceEnabled())
-//				log.trace("Found entry " + be.toString() + " for oid " + oid);
-//         
-//			try
-//         {
-//			   Object val = server.getAttribute(be.mbean, be.attr.getName());
-//
-//				if (val instanceof Long)
-//            {
-//					Long uin = (Long) val;
-//					ssy = new SnmpUInt32(uin);
-//				}
-//            else if (val instanceof String)
-//            {
-//					String in = (String) val;
-//					ssy = new SnmpOctetString(in.getBytes());
-//				}
-//            else if (val instanceof Integer)
-//            {
-//					Integer in = (Integer) val;
-//					ssy = new SnmpInt32(in);
-//				}
-//            else if (val instanceof SnmpObjectId)
-//            {
-//					ssy = (SnmpObjectId)val;
-//				}
-//            else if (val instanceof SnmpTimeTicks)
-//            {
-//               ssy = (SnmpTimeTicks)val;
-//            }
-//            else
-//					log.info("Unknown type for " + be);
-//			}
-//         catch (Exception e)
-//         {
-//				log.warn("getValueFor (" + be.mbean.toString() + ", "
-//						+ be.attr.getName() + ": " + e.toString());
-//         }
-//      }
-//      else
-//      {
-//			ssy = new SnmpNull();
-//			log.info(NO_ENTRY_FOUND_FOR_OID + oid);
-//		}
-//		return ssy;
-//	}
+	/** This method is used by snmpReceivedSet to reverse any changes in a 
+	 * SET PDU if an error is encountered before finishing.
+	 * 
+	 * @param modified HashMap containing OID,Val mappings
+	 */
+	private void undoSets(HashMap modified){
+		HashSet keys = (HashSet)modified.keySet();
+		Iterator iter = keys.iterator();
+		
+		while (iter.hasNext()){
+			try{
+			OID oid = (OID) iter.next();
+			setValueFor(oid,(Variable)modified.get(oid));// this will not fail, because it succeeded earlier.
+			}
+			catch(Exception e){
+				//impossible;
+			}
+		}		
+	}
 	
+	
+	
 	/**This method takes an Object that is typically going to be 
 	 * put into a VariableBinding for use in a PDU, and thus must be converted
 	 * into an SNMP type based on it's type. This Object is usually read 
@@ -788,6 +760,11 @@
 //	 * @return null on success, non-null on failure
 //	 * @throws ReadOnlyException If the referred entry is read only.
 //	 */
+	
+	//TODO: currently, if one accesses a String and attempts to set it to an Int or Double value,
+	// the response is success, but the result is failure. This must be changed by investigating 
+	// what value is given from this method when such a situation arises, and then either throwing an exception 
+	// or handling that value in the caller.
 	private Variable setValueFor(final OID oid, final Variable newVal) throws ReadOnlyException
    {
 		final boolean trace = log.isTraceEnabled();



More information about the jboss-cvs-commits mailing list