This is an automated email from the ASF dual-hosted git repository.
rohit pushed a commit to branch 4.11
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.11 by this push:
new f02e402 kvm: send unsupported answer only when applicable (#2714)
f02e402 is described below
commit f02e402ebb86d9ab5b6f34c04e57460ed53b7827
Author: dahn <[email protected]>
AuthorDate: Thu Jun 21 07:33:43 2018 +0200
kvm: send unsupported answer only when applicable (#2714)
Throw specific NPE child when command is known not to be known. Add unit
tests.
---
core/src/com/cloud/resource/RequestWrapper.java | 23 +++++++++++-------
.../kvm/resource/LibvirtComputingResource.java | 11 ++++++++-
.../resource/wrapper/LibvirtRequestWrapper.java | 3 +++
.../kvm/resource/LibvirtComputingResourceTest.java | 27 ++++++++++++++++++++++
4 files changed, 55 insertions(+), 9 deletions(-)
diff --git a/core/src/com/cloud/resource/RequestWrapper.java
b/core/src/com/cloud/resource/RequestWrapper.java
index 4e754d6..d1a3c1f 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.Answer;
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 @@ public abstract class RequestWrapper {
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 @@ public abstract class RequestWrapper {
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 @@ public abstract class RequestWrapper {
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 @@ public abstract class RequestWrapper {
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 424280c..8a94b05 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.DocumentBuilder;
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 class LibvirtComputingResource extends
ServerResourceBase implements Serv
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 0b413bc..436a130 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 class LibvirtRequestWrapper extends RequestWrapper {
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 795b961..be191f5 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.XPathConstants;
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 class LibvirtComputingResourceTest {
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);
+ }
}
