[jboss-cvs] JBossAS SVN: r109726 - in branches/snmp4j-integration-1.11.1/varia/src/main/java/org/jboss/jmx/adaptor/snmp: test and 1 other directory.

jboss-cvs-commits at lists.jboss.org jboss-cvs-commits at lists.jboss.org
Mon Dec 6 09:19:31 EST 2010


Author: thauser at redhat.com
Date: 2010-12-06 09:19:31 -0500 (Mon, 06 Dec 2010)
New Revision: 109726

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/test/NotificationProducerService.java
Log:
commit changes to error handling


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-06 13:21:37 UTC (rev 109725)
+++ branches/snmp4j-integration-1.11.1/varia/src/main/java/org/jboss/jmx/adaptor/snmp/agent/RequestHandlerImpl.java	2010-12-06 14:19:31 UTC (rev 109726)
@@ -74,7 +74,7 @@
 {
 	// Protected Data ------------------------------------------------
 
-	private static final String NO_ENTRY_FOUND_FOR_OID = "No entry found for oid ";
+	private static final String NO_ENTRY_FOUND_FOR_OID = "No bind entry found for oid ";
 	private static final String SKIP_ENTRY = " - skipping entry";
 
 
@@ -121,6 +121,9 @@
 		initialized = true;
 	}
 
+   // TODO: change all error handling to Exceptions. Much more extensible. 
+   //		this must be done because the use of return values could be much more useful if exceptions were thrown instead.
+   // TODO: getValueFor, setValueFor, and getNextOid all need to throw appropriate exceptions. 
    // Reconfigurable Implementation ---------------------------------
    /**
     * Reconfigures the RequestHandler
@@ -165,6 +168,9 @@
     * 
     */
    
+   // 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;
@@ -172,7 +178,7 @@
 		if (pdu instanceof ScopedPDU){
 			response = DefaultPDUFactory.createPDU(SnmpConstants.version3);
 		} else if (pdu instanceof PDUv1){
-			log.debug("snmp-adaptor: cannot getBulk with V1 PDU.");
+			log.debug("snmpReceievedGetBulk: cannot getBulk with V1 PDU.");
 			return null;
 		} else {
 			response = new PDU();
@@ -192,40 +198,54 @@
 			VariableBinding vb;
 			Variable var;
 			
+			if (it.size() == 0){ // we were given an empty list in the PDU
+				log.debug("snmpReceivedGetBulk: No VariableBindings in received PDU");
+				response.setErrorStatus(SnmpConstants.SNMP_ERROR_GENERAL_ERROR);
+				return response;
+			}
 			// for the first nonRepeater Bindings, we simply get their value. if nonRepeaters is 0, 
 			// we do a GET on every element, and ignore the next loop.
 			for (int i=0; i < Math.min(nonRepeaters, it.size());i++){
 				vb = (VariableBinding)it.get(i);
 				OID oid = vb.getOid();
-					
-					if (oid == null){
-						log.debug("Attempt to GETBULK on non-existent scalar OID.");
+				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;
-						// TODO: should we fail out here?
-					}
-					var = getValueFor(oid);
-					response.add(new VariableBinding(oid,var));
-				}
+					}*/
+			}
 			
 			// for the remaining it.size() bindings, we perform maxRepetitions successive getNext calls
 			for (int i = nonRepeaters; i < it.size(); i++){
 				vb = (VariableBinding)it.get(i);
 				OID noid = getNextOid(vb.getOid(),true);
-				
+								
 				//FIXME: returns null when maxReptitions goes into OIDs that don't exist. 
 				for (int j = 0; j < maxRepetitions; j++){
-					var = getValueFor(noid);
-					
-					if (noid == null){
-						log.debug("Attempt to GETBULK on non-existent non-scalar OID.");
-						response.setErrorStatus(PDU.noSuchName);
-						response.setErrorIndex(i);
+					if (noid == null){ // reached the end of the repitions.
+						log.debug("snmpReceivedGetBulk: Attempt to GETBULK on non-existent non-scalar OID.");
+						continue;
+/*						response.setErrorStatus(PDU.noSuchName);
+						response.setErrorIndex(i+1);
+						response.add(new VariableBinding(new OID(), Null.noSuchObject));
 						// TODO: should we fail out here?
-						return response;
+						return response;*/
 					}
-					
+					var = getValueFor(noid);
 					response.add(new VariableBinding(noid,var));
 					noid = getNextOid(noid,true);
 				}
@@ -234,8 +254,7 @@
 		}	
 		return response;
    	}
-   	
-   
+   	   
     /**
 	 * <P>
 	 * This method is defined to handle SNMP Get requests that are received by
@@ -252,6 +271,8 @@
 	public PDU snmpReceivedGet(PDU pdu)
 	{
 		PDU response;
+		int counter = 1;
+		
 		if (pdu instanceof ScopedPDU){
 			response = DefaultPDUFactory.createPDU(SnmpConstants.version3);
 		} else if (pdu instanceof PDUv1){
@@ -278,24 +299,19 @@
 				VariableBinding vb = (VariableBinding)it.next();
 				OID oid = vb.getOid();
 				
-				if (oid == null){
-					log.debug("Attempt to SNMP GET non-existent OID.");
-					response.setErrorStatus(PDU.noSuchName);
-					return response;
-				}
-				//TODO: find a way to properly index problematic PDU variablebindings.
-
 				if (pdu.getType()==PDU.GETNEXT){	
 					// 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.
 					if (noid == null){
-						log.debug("SNMP GETNEXT operation returned null. No such OID.");
+						log.debug("snmpReceivedGet: getNextOid operation returned null. No such OID.");
 						response.setErrorStatus(PDU.noSuchName);
+						response.setErrorIndex(counter);
+						response.add(new VariableBinding(oid, Null.noSuchObject));
 						return response;
 					}
 					newVB = new VariableBinding(noid);
-					var = getValueFor(noid);						
+					var = getValueFor(noid);	
 				}
 				else {
 					newVB = new VariableBinding(oid);
@@ -303,95 +319,13 @@
 				}
 					newVB.setVariable(var);
 					response.add(newVB);
+					counter++;
 		 	}
 			
 		}
 		return response;
 	}	
 		
-	/** Brian Shim commented this out for reference i'm guessing **/
-//		try
-//		{
-//			SnmpPduRequest response = null;
-//			int pduLength = pdu.getLength();
-//			final boolean trace = log.isTraceEnabled();
-//
-//			if (trace)
-//				log.trace("requestId=" + pdu.getRequestId() + ", pduLength="
-//						+ pduLength + ", getNext=" + getNext);
-//
-//			SnmpVarBind[] vblist = new SnmpVarBind[pduLength];
-//			int errorStatus = SnmpPduPacket.ErrNoError;
-//			int errorIndex = 0;
-//
-//			// Process for each varibind in the request
-//			for (int i = 0; i < pduLength; i++)
-//			{
-//				boolean good = true;
-//				SnmpVarBind vb = pdu.getVarBindAt(i);
-//				SnmpObjectId oid = vb.getName();
-		//START OF GETNEXT
-//				if (getNext) //i don't care about GETNEXT, doing GET first
-//				{
-//					/*
-//					 * We call getNextOid() to find out what is the next valid OID
-//					 * instance in the supported MIB (sub-)tree. Assign that OID to the
-//					 * VB List and then proceed same as that of get request. If the
-//					 * passed oid is already the last, we flag it.
-//					 */
-//					ComparableSnmpObjectId coid = new ComparableSnmpObjectId(oid);
-//					oid = getNextOid(coid, true);
-//					if (oid == null)
-//					{
-//						good = false;
-//					}
-//					else
-//					{
-//						pdu.setVarBindAt(i, new SnmpVarBind(oid));
-//					}
-//				}
-		//End of GETNEXT
-//				if (oid!=null)
-//					vblist[i] = new SnmpVarBind(oid);
-//				else
-//					vblist[i] = new SnmpVarBind(vb.getName()); // oid passed in
-//				
-//
-//				if (trace)
-//					log.trace("oid=" + oid);
-//
-//				SnmpSyntax result = null;
-//				if (good && bindings != null)
-//					result = getValueFor(oid);
-//
-//				if (trace)
-//					log.trace("got result of " + result);
-//
-//				if (result == null || !good)
-//				{
-//					errorStatus = SnmpPduPacket.ErrNoSuchName;
-//					errorIndex = i + 1;
-//					log.debug("Error Occured " + vb.getName().toString());
-//				} 
-//				else
-//				{
-//					vblist[i].setValue(result);
-//					log.debug("Varbind[" + i + "] := "
-//									+ vblist[i].getName().toString());
-//					log.debug(" --> " + vblist[i].getValue().toString());
-//				}
-//			} // for ...
-//			response = new SnmpPduRequest(SnmpPduPacket.RESPONSE, vblist);
-//			response.setErrorStatus(errorStatus);
-//			response.setErrorIndex(errorIndex);
-//			return response;
-//		} catch (Exception e)
-//		{
-//			// TODO Auto-generated catch block
-//			e.printStackTrace();
-//			return null;
-//		}
-
 	/**
 	 * <P>
 	 * This method is defined to handle SNMP Set requests that are received by
@@ -408,6 +342,7 @@
 	public PDU snmpReceivedSet(PDU pdu)
    {
 		PDU response;
+		int counter = 1;
 		Variable var = null;
 		if (pdu instanceof ScopedPDU){
 			response = DefaultPDUFactory.createPDU(SnmpConstants.version3);
@@ -442,17 +377,23 @@
 				var = setValueFor(oid,newVal);
 				}
 				catch (ReadOnlyException e){
-					log.debug("attempt to set a read-only attribute: " + newVB);
-					response.setErrorStatus(pdu.readOnly);
-					//TODO: find a way to indicate the Varbind with setErrorIndex without using a local int variable 
+					// 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);
 					return response;
 				}
 				
 				if (var != null){
-					log.debug("attempt to set value resulted in a null PDU being returned.");
-					response = null;
+					log.debug("snmpReceivedSet: attempt to set value resulted in a null PDU being returned");
+					response.setErrorStatus(PDU.noSuchName);
+					response.setErrorIndex(counter);
+					response.add(new VariableBinding(oid, Null.noSuchObject));
+					return response;
 				}
 				response.add(newVB);
+				counter++;
 			}
 		}
 
@@ -638,7 +579,6 @@
 		      BindEntry be = new BindEntry(oid, mmb.getName(), ma.getName());
 		      be.isReadWrite = ma.isReadWrite();
   			  
-  			  //ComparableSnmpObjectId coid = new ComparableSnmpObjectId(oid);
   			  OID coid = new OID(oid);
 		      if (log.isTraceEnabled())
 		         log.trace("New bind entry   " + be);
@@ -677,7 +617,7 @@
 		if (be != null)
      {
 			if (log.isTraceEnabled())
-				log.trace("Found entry " + be.toString() + " for oid " + oid);
+				log.trace("getValueFor: Found entry " + be.toString() + " for oid " + oid);
         
 			try
         {
@@ -685,20 +625,20 @@
 			   ssy = prepForPdu(val);
 			   
 			   if (val == null){
-					log.info("Unknown type for " + be);
+					log.info("getValueFor: Unknown type for " + be);
 				}
 		}
 
         catch (Exception e)
         {
-				log.warn("getValueFor (" + be.mbean.toString() + ", "
+				log.warn("getValueFor: (" + be.mbean.toString() + ", "
 						+ be.attr.getName() + ": " + e.toString());
         }
      }
      else
      {
-			ssy = new Null();
-			log.info(NO_ENTRY_FOUND_FOR_OID + oid);
+			ssy = Null.noSuchInstance;
+			log.info("getValueFor: " + NO_ENTRY_FOUND_FOR_OID + oid);
 		}
 		return ssy;
 	}
@@ -804,13 +744,13 @@
         }
         else
         {
-        	result = null;
+        	result = Null.noSuchInstance; // no instance of an SNMP Variable could be crated for type
         }
 		return result;
 	}	
 	
 	/** 
-	 * Takes an instance of org.snmp4j.smi.Variable and returns a Java Class
+	 * Takes an instance of org.snmp4j.smi.Variable and returns a Java primitive
 	 * representation. This is used to set mbean values given a PDU.
 	 * @param val the value to be converted
 	 * @return an Object created with the proper type. null if failure
@@ -834,7 +774,7 @@
 			result = new Long(((Counter64)val).getValue());
 		}
 		else{
-			result = null;
+			result = Null.noSuchInstance; //no instance could be created.
 		}
 		// TODO do more mumbo jumbo for type casting / changing
 		return result;
@@ -859,46 +799,44 @@
 			log.trace("setValueFor: found bind entry for " + oid);
 		
 		if (be != null)
-      {
+		{
 			if (trace)
 				log.trace("setValueFor: " + be.toString());
          
 			if (be.isReadWrite == false)
-         {
+			{
 				if (trace)
 					log.trace("setValueFor: this is marked read only");
             
 				throw new ReadOnlyException(oid);
 			}
 			try
-         {		
-				
-			Object val = convertVariableToValue(newVal);
+			{		
+				Object val = convertVariableToValue(newVal);
 		
-				if (val != null)
-            { 
-					Attribute at = new Attribute(be.attr.getName(), val);
-					server.setAttribute(be.mbean, at);
-					if (trace)
-						log.trace("setValueFor: set attribute in mbean-Server");
+				if (val instanceof Null)
+				{ 
+					log.debug("setValueFor: did not find a suitable data type for newVal " + newVal);
+					ssy = newVal;
+
 				}
-				else
-            {
-					log.debug("Did not find a suitable data type for newVal " + newVal);
-					ssy = newVal;
+				
+				Attribute at = new Attribute(be.attr.getName(), val);
+				server.setAttribute(be.mbean, at);
+			
+				if (trace)
+					log.trace("setValueFor: set attribute in mbean server");
 			}
-				// TODO
-			}
 			catch (Exception e )
-         {
+			{
 				log.debug("setValueFor: exception " + e.getMessage());
 				ssy = null;
 			}
 		}
 		else
-      {
+		{
 			ssy = newVal;
-			log.info(NO_ENTRY_FOUND_FOR_OID + oid);
+			log.info("setValueFor: " + NO_ENTRY_FOUND_FOR_OID + oid + " on the mbean server");
 		}
 		return ssy;
 	}
@@ -969,38 +907,43 @@
 		 *   SortedSet.tailSet() , then get next, which is the 
 		 *   one we look for.
 		 */
-		if (it.hasNext()) 
-		{
+		if (it.hasNext()){
+			
 			roid = (OID)it.next(); // oid
 		}
+		//catch (NoSuchElementException e){
+		//log.debug("getNextOid: Placeholder. The OID did not exist");
+		else{
+			log.debug("getNextOid: Placeholder. The OID did not exist");
+			return null;
+		}	
 		
-		if (roid == null)
-		{
-			return null; // roid is null, 
-		}
-		
 		if (roid.compareTo(coid)==0) // input elment
 		{
 			// if there is a next element, then it is ours.
+			// perhaps change this to try/catch also
 			if (it.hasNext()) 
 			{
 				roid = (OID)it.next();
 			}
 			else
 			{
-				roid = null; // end of list
+				log.debug("getNextOid: Placeholder. There is no lexically larger OID than the input.");
+				// end of list
+				return null;
 			}
 		}
-      
-		
+ 		
 		// * Check if still in subtree if requested to stay within
 		 
 		if (stayInSubtree && roid != null)
 		{
 			//OID parent = coid.removeLast();
 			// this emulates the functionality of the "isRoot" in SnmpObjectId from joesnmp
-			if (coid.leftMostCompare((coid.size()-1), roid) != 0)
+			if (coid.leftMostCompare((coid.size()-1), roid) != 0){
+				log.debug("getNextOid: Placeholder. The traversal has left the subtree.");
 				roid = null;
+			}
 		}
 
 		return roid;
@@ -1122,3 +1065,86 @@
 	}
 
 }
+
+/** Brian Shim commented this out for reference i'm guessing **/
+//try
+//{
+//	SnmpPduRequest response = null;
+//	int pduLength = pdu.getLength();
+//	final boolean trace = log.isTraceEnabled();
+//
+//	if (trace)
+//		log.trace("requestId=" + pdu.getRequestId() + ", pduLength="
+//				+ pduLength + ", getNext=" + getNext);
+//
+//	SnmpVarBind[] vblist = new SnmpVarBind[pduLength];
+//	int errorStatus = SnmpPduPacket.ErrNoError;
+//	int errorIndex = 0;
+//
+//	// Process for each varibind in the request
+//	for (int i = 0; i < pduLength; i++)
+//	{
+//		boolean good = true;
+//		SnmpVarBind vb = pdu.getVarBindAt(i);
+//		SnmpObjectId oid = vb.getName();
+//START OF GETNEXT
+//		if (getNext) //i don't care about GETNEXT, doing GET first
+//		{
+//			/*
+//			 * We call getNextOid() to find out what is the next valid OID
+//			 * instance in the supported MIB (sub-)tree. Assign that OID to the
+//			 * VB List and then proceed same as that of get request. If the
+//			 * passed oid is already the last, we flag it.
+//			 */
+//			ComparableSnmpObjectId coid = new ComparableSnmpObjectId(oid);
+//			oid = getNextOid(coid, true);
+//			if (oid == null)
+//			{
+//				good = false;
+//			}
+//			else
+//			{
+//				pdu.setVarBindAt(i, new SnmpVarBind(oid));
+//			}
+//		}
+//End of GETNEXT
+//		if (oid!=null)
+//			vblist[i] = new SnmpVarBind(oid);
+//		else
+//			vblist[i] = new SnmpVarBind(vb.getName()); // oid passed in
+//		
+//
+//		if (trace)
+//			log.trace("oid=" + oid);
+//
+//		SnmpSyntax result = null;
+//		if (good && bindings != null)
+//			result = getValueFor(oid);
+//
+//		if (trace)
+//			log.trace("got result of " + result);
+//
+//		if (result == null || !good)
+//		{
+//			errorStatus = SnmpPduPacket.ErrNoSuchName;
+//			errorIndex = i + 1;
+//			log.debug("Error Occured " + vb.getName().toString());
+//		} 
+//		else
+//		{
+//			vblist[i].setValue(result);
+//			log.debug("Varbind[" + i + "] := "
+//							+ vblist[i].getName().toString());
+//			log.debug(" --> " + vblist[i].getValue().toString());
+//		}
+//	} // for ...
+//	response = new SnmpPduRequest(SnmpPduPacket.RESPONSE, vblist);
+//	response.setErrorStatus(errorStatus);
+//	response.setErrorIndex(errorIndex);
+//	return response;
+//} catch (Exception e)
+//{
+//	// TODO Auto-generated catch block
+//	e.printStackTrace();
+//	return null;
+//}

Modified: branches/snmp4j-integration-1.11.1/varia/src/main/java/org/jboss/jmx/adaptor/snmp/test/NotificationProducerService.java
===================================================================
--- branches/snmp4j-integration-1.11.1/varia/src/main/java/org/jboss/jmx/adaptor/snmp/test/NotificationProducerService.java	2010-12-06 13:21:37 UTC (rev 109725)
+++ branches/snmp4j-integration-1.11.1/varia/src/main/java/org/jboss/jmx/adaptor/snmp/test/NotificationProducerService.java	2010-12-06 14:19:31 UTC (rev 109726)
@@ -102,7 +102,58 @@
                           "V3 test notifications"));        
    }
    
+   public void getBulk(){
+		PDU pdu = new PDU();
+		pdu.setType(PDU.GETBULK);
+		pdu.add(new VariableBinding(new OID("1.2.3.4.1.1")));
+		pdu.add(new VariableBinding(new OID("1.3.6.1.2.1.1.0")));
+		pdu.setMaxRepetitions(7);
+		pdu.setNonRepeaters(1);
+		CommunityTarget target = new CommunityTarget();
+		OctetString community = new OctetString("public");
+		target.setCommunity(community);
+		target.setVersion(SnmpConstants.version2c);
+		Address targetAddress = GenericAddress.parse("udp:127.0.0.1/1161");
+		target.setAddress(targetAddress);
+		target.setRetries(2);
+		target.setTimeout(2000);
+	
+		try {
+			DefaultUdpTransportMapping transport = new DefaultUdpTransportMapping();
+			transport.listen();
+			System.out.println("READY: "+System.currentTimeMillis());
+			Snmp snmp = new Snmp(transport);
+			long t1 = System.currentTimeMillis();
+			System.out.println("SENDING: "+t1);
+			System.out.println("PDU: "+pdu);
+			ResponseEvent responseEvent = snmp.send(pdu, target);
+			long t2=System.currentTimeMillis();
+			System.out.println("SENT: "+t2);
+			System.out.println("ELAPSED: "+(t2-t1));
+			System.out.println("response " + responseEvent.toString());
+
+			PDU responsePDU = responseEvent.getResponse();
+			if (responsePDU == null){
+				System.out.println("Request timed out");
+			}
+			else{
+				System.out.println("Received response "+responsePDU);
+			}
+				System.out.println("Peer Address: "+responseEvent.getPeerAddress());
+		} catch (UnknownHostException e1) {
+			// TODO Auto-generated catch block
+			e1.printStackTrace();
+		} catch (IOException e1) {
+			// TODO Auto-generated catch block
+			e1.printStackTrace();
+		} catch (Exception e) {
+			System.out.println("Some Other exception!!");
+		}
+	   
+	   
+   }
    
+   
    /**
     * Sends a test GET request
     * 



More information about the jboss-cvs-commits mailing list