rhtyd closed pull request #2714: send unsupported answer only when applicable
URL: https://github.com/apache/cloudstack/pull/2714
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/core/src/com/cloud/resource/RequestWrapper.java
b/core/src/com/cloud/resource/RequestWrapper.java
index 4e754d60a29..d1a3c1f18cc 100644
--- a/core/src/com/cloud/resource/RequestWrapper.java
+++ b/core/src/com/cloud/resource/RequestWrapper.java
@@ -29,6 +29,15 @@
import com.cloud.agent.api.Command;
public abstract class RequestWrapper {
+ static public class CommandNotSupported extends NullPointerException {
+ public CommandNotSupported(String msg) {
+ super(msg);
+ }
+ public CommandNotSupported(String msg, Throwable cause) {
+ super(msg);
+ initCause(cause);
+ }
+ }
private static final Logger s_logger =
Logger.getLogger(RequestWrapper.class);
@@ -52,7 +61,7 @@
keepResourceClass = keepResourceClass2;
} catch (final ClassCastException e) {
- throw new NullPointerException("No key found for '" +
command.getClass() + "' in the Map!");
+ throw new CommandNotSupported("No key found for '" +
command.getClass() + "' in the Map!");
}
}
return resource;
@@ -69,14 +78,14 @@
final Class<? extends Command> commandClass2 = (Class<?
extends Command>) keepCommandClass.getSuperclass();
if (commandClass2 == null) {
- throw new NullPointerException("All the COMMAND hierarchy
tree has been visited but no compliant key has been found for '" + commandClass
+ "'.");
+ throw new CommandNotSupported("All the COMMAND hierarchy
tree has been visited but no compliant key has been found for '" + commandClass
+ "'.");
}
commandWrapper = resourceCommands.get(commandClass2);
keepCommandClass = commandClass2;
} catch (final ClassCastException e) {
- throw new NullPointerException("No key found for '" +
keepCommandClass.getClass() + "' in the Map!");
+ throw new CommandNotSupported("No key found for '" +
keepCommandClass.getClass() + "' in the Map!");
} catch (final NullPointerException e) {
// Will now traverse all the resource hierarchy. Returning null
// is not a problem.
@@ -102,7 +111,7 @@
final Class<? extends ServerResource> resourceClass2 =
(Class<? extends ServerResource>) keepResourceClass.getSuperclass();
if (resourceClass2 == null) {
- throw new NullPointerException("All the SERVER-RESOURCE
hierarchy tree has been visited but no compliant key has been found for '" +
command.getClass() + "'.");
+ throw new CommandNotSupported("All the SERVER-RESOURCE
hierarchy tree has been visited but no compliant key has been found for '" +
command.getClass() + "'.");
}
final Hashtable<Class<? extends Command>, CommandWrapper>
resourceCommands2 = retrieveResource(command,
@@ -110,10 +119,8 @@
keepResourceClass = resourceClass2;
commandWrapper = retrieveCommands(command.getClass(),
resourceCommands2);
- } catch (final ClassCastException e) {
- throw new NullPointerException("No key found for '" +
command.getClass() + "' in the Map!");
- } catch (final NullPointerException e) {
- throw e;
+ } catch (final ClassCastException | NullPointerException e) {
+ throw new CommandNotSupported("No key found for '" +
command.getClass() + "' in the Map!", e);
}
}
return commandWrapper;
diff --git
a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
index 424280cc66c..8a94b058655 100644
---
a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
+++
b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
@@ -47,6 +47,7 @@
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
+import com.cloud.resource.RequestWrapper;
import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
import org.apache.cloudstack.storage.to.VolumeObjectTO;
import org.apache.cloudstack.utils.hypervisor.HypervisorUtils;
@@ -1432,13 +1433,21 @@ public boolean stop() {
return true;
}
+ /**
+ * This finds a command wrapper to handle the command and executes it.
+ * If no wrapper is found an {@see UnsupportedAnswer} is sent back.
+ * Any other exceptions are to be caught and wrapped in an generic {@see
Answer}, marked as failed.
+ *
+ * @param cmd the instance of a {@see Command} to execute.
+ * @return the for the {@see Command} appropriate {@see Answer} or {@see
UnsupportedAnswer}
+ */
@Override
public Answer executeRequest(final Command cmd) {
final LibvirtRequestWrapper wrapper =
LibvirtRequestWrapper.getInstance();
try {
return wrapper.execute(cmd, this);
- } catch (final Exception e) {
+ } catch (final RequestWrapper.CommandNotSupported cmde) {
return Answer.createUnsupportedCommandAnswer(cmd);
}
}
diff --git
a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRequestWrapper.java
b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRequestWrapper.java
index 0b413bc6e9f..436a1308316 100644
---
a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRequestWrapper.java
+++
b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRequestWrapper.java
@@ -72,6 +72,9 @@ public Answer execute(final Command command, final
ServerResource serverResource
commandWrapper = retryWhenAllFails(command, resourceClass,
resourceCommands);
}
+ if (commandWrapper == null) {
+ throw new CommandNotSupported("No way to handle " +
command.getClass());
+ }
return commandWrapper.execute(command, serverResource);
}
}
\ No newline at end of file
diff --git
a/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java
b/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java
index 795b96175ab..be191f5e9a5 100644
---
a/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java
+++
b/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java
@@ -38,6 +38,8 @@
import javax.xml.xpath.XPathExpressionException;
import javax.xml.xpath.XPathFactory;
+import com.cloud.agent.api.Command;
+import com.cloud.agent.api.UnsupportedAnswer;
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.CpuTuneDef;
import org.apache.commons.lang.SystemUtils;
import org.joda.time.Duration;
@@ -5191,4 +5193,29 @@ public void testSetQuotaAndPeriodMinQuota() {
Assert.assertEquals(CpuTuneDef.MIN_QUOTA, cpuTuneDef.getQuota());
Assert.assertEquals((int) (CpuTuneDef.MIN_QUOTA / pct),
cpuTuneDef.getPeriod());
}
+
+ @Test
+ public void testUnknownCommand() {
+ libvirtComputingResource = new LibvirtComputingResource();
+ Command cmd = new Command() {
+ @Override public boolean executeInSequence() {
+ return false;
+ }
+ };
+ Answer ans = libvirtComputingResource.executeRequest(cmd);
+ assertTrue(ans instanceof UnsupportedAnswer);
+ }
+
+ @Test
+ public void testKnownCommand() {
+ libvirtComputingResource = new LibvirtComputingResource();
+ Command cmd = new PingTestCommand() {
+ @Override public boolean executeInSequence() {
+ throw new NullPointerException("test succeeded");
+ }
+ };
+ Answer ans = libvirtComputingResource.executeRequest(cmd);
+ assertFalse(ans instanceof UnsupportedAnswer);
+ assertTrue(ans instanceof Answer);
+ }
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
