From 19c6d8054ba4f79b70ba7acbf67dce5a57856f73 Mon Sep 17 00:00:00 2001 From: Hiroaki Kawai Date: Wed, 6 Mar 2013 10:22:44 -0800 Subject: [PATCH] CLOUDSTACK-1535: kvm agent will silently ignore many exceptions Current kvm agent will silently ignore many exception, and there's no way to see what really happened. This patch will log in trace level log that was silently ignored. And also, it will fix huge bare Exception catch, which is very harmful because it also catches RuntimeException. --- .../VirtualRoutingResource.java | 46 +++--- .../hypervisor/kvm/resource/KVMHABase.java | 6 +- .../resource/LibvirtComputingResource.java | 146 ++++++++---------- .../kvm/resource/LibvirtXMLParser.java | 8 +- 4 files changed, 97 insertions(+), 109 deletions(-) diff --git a/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java b/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java index fc7f08f76a2..9dd471d6efa 100755 --- a/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java +++ b/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java @@ -859,35 +859,29 @@ public class VirtualRoutingResource implements Manager { } public void assignVpcIpToRouter(final String routerIP, final boolean add, final String pubIP, - final String nicname, final String gateway, final String netmask, final String subnet) throws Exception { - try { - String args = ""; + final String nicname, final String gateway, final String netmask, final String subnet) throws InternalErrorException { + String args = ""; - if (add) { - args += " -A "; - } else { - args += " -D "; - } + if (add) { + args += " -A "; + } else { + args += " -D "; + } - args += " -l "; - args += pubIP; - args += " -c "; - args += nicname; - args += " -g "; - args += gateway; - args += " -m "; - args += netmask; - args += " -n "; - args += subnet; + args += " -l "; + args += pubIP; + args += " -c "; + args += nicname; + args += " -g "; + args += gateway; + args += " -m "; + args += netmask; + args += " -n "; + args += subnet; - String result = routerProxy("vpc_ipassoc.sh", routerIP, args); - if (result != null) { - throw new InternalErrorException("KVM plugin \"vpc_ipassoc\" failed:"+result); - } - } catch (Exception e) { - String msg = "Unable to assign public IP address due to " + e.toString(); - s_logger.warn(msg, e); - throw new Exception(msg); + String result = routerProxy("vpc_ipassoc.sh", routerIP, args); + if (result != null) { + throw new InternalErrorException("KVM plugin \"vpc_ipassoc\" failed:"+result); } } diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/KVMHABase.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/KVMHABase.java index af89d9b18a9..d067b35902f 100644 --- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/KVMHABase.java +++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/KVMHABase.java @@ -23,6 +23,7 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; +import org.apache.log4j.Logger; import org.libvirt.LibvirtException; import org.libvirt.StoragePool; import org.libvirt.StoragePoolInfo; @@ -33,6 +34,7 @@ import com.cloud.utils.script.OutputInterpreter.AllLinesParser; import com.cloud.utils.script.Script; public class KVMHABase { + private static final Logger s_logger = Logger.getLogger(KVMHABase.class); private long _timeout = 60000; /* 1 minutes */ protected static String _heartBeatPath; protected long _heartBeatUpdateTimeout = 60000; @@ -124,14 +126,14 @@ public class KVMHABase { } poolName = pool.getName(); } catch (LibvirtException e) { - + s_logger.debug("Ignoring libvirt error.", e); } finally { try { if (pool != null) { pool.free(); } } catch (LibvirtException e) { - + s_logger.debug("Ignoring libvirt error.", e); } } 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 3c848dead1e..aa4acbb851b 100755 --- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -692,7 +692,7 @@ ServerResource { _hvVersion = conn.getVersion(); _hvVersion = (_hvVersion % 1000000) / 1000; } catch (LibvirtException e) { - + s_logger.trace("Ignoring libvirt error.", e); } String[] info = NetUtils.getNetworkParams(_privateNic); @@ -764,8 +764,8 @@ ServerResource { if (tokens.length == 2) { try { _migrateSpeed = Integer.parseInt(tokens[0]); - } catch (Exception e) { - + } catch (NumberFormatException e) { + s_logger.trace("Ignoring migrateSpeed extraction error.", e); } s_logger.debug("device " + _pifs.get("public") + " has speed: " + String.valueOf(_migrateSpeed)); } @@ -846,8 +846,8 @@ ServerResource { throw new ConfigurationException("Unable to find class for libvirt.vif.driver " + e); } catch (InstantiationException e) { throw new ConfigurationException("Unable to instantiate class for libvirt.vif.driver " + e); - } catch (Exception e) { - throw new ConfigurationException("Failed to initialize libvirt.vif.driver " + e); + } catch (IllegalAccessException e) { + throw new ConfigurationException("Unable to instantiate class for libvirt.vif.driver " + e); } return vifDriver; } @@ -1043,7 +1043,6 @@ ServerResource { protected String startVM(Connect conn, String vmName, String domainXML) throws LibvirtException, InternalErrorException { - Domain dm = null; try { /* We create a transient domain here. When this method gets @@ -1053,12 +1052,11 @@ ServerResource { This also makes sure we never have any old "garbage" defined in libvirt which might haunt us. */ - dm = conn.domainCreateXML(domainXML, 0); + conn.domainCreateXML(domainXML, 0); } catch (final LibvirtException e) { s_logger.warn("Failed to start domain " + vmName + ": " - + e.getMessage()); + + e.getMessage(), e); } - return null; } @@ -1068,6 +1066,7 @@ ServerResource { Connect conn = LibvirtConnection.getConnection(); conn.close(); } catch (LibvirtException e) { + s_logger.trace("Ignoring libvirt error.", e); } return true; @@ -1516,11 +1515,10 @@ ServerResource { } private PlugNicAnswer execute(PlugNicCommand cmd) { - Connect conn; NicTO nic = cmd.getNic(); String vmName = cmd.getVmName(); try { - conn = LibvirtConnection.getConnection(); + Connect conn = LibvirtConnection.getConnection(); Domain vm = getDomain(conn, vmName); List pluggedNics = getInterfaces(conn, vmName); Integer nicnum = 0; @@ -1533,7 +1531,11 @@ ServerResource { } vm.attachDevice(getVifDriver(nic.getType()).plug(nic, "Other PV (32-bit)").toString()); return new PlugNicAnswer(cmd, true, "success"); - } catch (Exception e) { + } catch (LibvirtException e) { + String msg = " Plug Nic failed due to " + e.toString(); + s_logger.warn(msg, e); + return new PlugNicAnswer(cmd, false, msg); + } catch (InternalErrorException e) { String msg = " Plug Nic failed due to " + e.toString(); s_logger.warn(msg, e); return new PlugNicAnswer(cmd, false, msg); @@ -1555,7 +1557,7 @@ ServerResource { } } return new UnPlugNicAnswer(cmd, true, "success"); - } catch (Exception e) { + } catch (LibvirtException e) { String msg = " Unplug Nic failed due to " + e.toString(); s_logger.warn(msg, e); return new UnPlugNicAnswer(cmd, false, msg); @@ -1609,7 +1611,7 @@ ServerResource { return new SetupGuestNetworkAnswer(cmd, false, "Creating guest network failed due to " + result); } return new SetupGuestNetworkAnswer(cmd, true, "success"); - } catch (Exception e) { + } catch (LibvirtException e) { String msg = "Creating guest network failed due to " + e.toString(); s_logger.warn(msg, e); return new SetupGuestNetworkAnswer(cmd, false, msg); @@ -1649,7 +1651,7 @@ ServerResource { } return new SetNetworkACLAnswer(cmd, true, results); - } catch (Exception e) { + } catch (LibvirtException e) { String msg = "SetNetworkACL failed due to " + e.toString(); s_logger.error(msg, e); return new SetNetworkACLAnswer(cmd, false, results); @@ -1694,7 +1696,7 @@ ServerResource { return new SetSourceNatAnswer(cmd, false, "KVM plugin \"vpc_snat\" failed:"+result); } return new SetSourceNatAnswer(cmd, true, "success"); - } catch (Exception e) { + } catch (LibvirtException e) { String msg = "Ip SNAT failure due to " + e.toString(); s_logger.error(msg, e); return new SetSourceNatAnswer(cmd, false, msg); @@ -1739,7 +1741,10 @@ ServerResource { results[i++] = ip.getPublicIp() + " - success"; } - } catch (Exception e) { + } catch (LibvirtException e) { + s_logger.error("Ip Assoc failure on applying one ip due to exception: ", e); + results[i++] = IpAssocAnswer.errorResult; + } catch (InternalErrorException e) { s_logger.error("Ip Assoc failure on applying one ip due to exception: ", e); results[i++] = IpAssocAnswer.errorResult; } @@ -1819,7 +1824,7 @@ ServerResource { vm = getDomain(conn, cmd.getVmName()); state = vm.getInfo().state; } catch (LibvirtException e) { - + s_logger.trace("Ignoring libvirt error.", e); } } @@ -1938,7 +1943,7 @@ ServerResource { vm = getDomain(conn, cmd.getVmName()); state = vm.getInfo().state; } catch (LibvirtException e) { - + s_logger.trace("Ignoring libvirt error.", e); } } @@ -2380,7 +2385,7 @@ ServerResource { Connect conn = LibvirtConnection.getConnection(); Integer vncPort = getVncPort(conn, cmd.getName()); return new GetVncPortAnswer(cmd, _privateIp, 5900 + vncPort); - } catch (Exception e) { + } catch (LibvirtException e) { return new GetVncPortAnswer(cmd, e.toString()); } } @@ -2501,16 +2506,13 @@ ServerResource { } catch (final LibvirtException e) { s_logger.warn("Can't get vm state " + vmName + e.getMessage() + "retry:" + retry); - } catch (Exception e) { - s_logger.warn("Can't get vm state " + vmName + e.getMessage() - + "retry:" + retry); } finally { try { if (vms != null) { vms.free(); } - } catch (final LibvirtException e) { - + } catch (final LibvirtException l) { + s_logger.trace("Ignoring libvirt error.", l); } } } @@ -2603,9 +2605,6 @@ ServerResource { } catch (LibvirtException e) { s_logger.debug("Can't migrate domain: " + e.getMessage()); result = e.getMessage(); - } catch (Exception e) { - s_logger.debug("Can't migrate domain: " + e.getMessage()); - result = e.getMessage(); } finally { try { if (dm != null) { @@ -2618,7 +2617,7 @@ ServerResource { destDomain.free(); } } catch (final LibvirtException e) { - + s_logger.trace("Ignoring libvirt error.", e); } } @@ -2795,8 +2794,8 @@ ServerResource { Integer vncPort = null; try { vncPort = getVncPort(conn, cmd.getVmName()); - } catch (Exception e) { - + } catch (LibvirtException e) { + s_logger.trace("Ignoring libvirt error.", e); } get_rule_logs_for_vms(); return new RebootAnswer(cmd, null, vncPort); @@ -3124,8 +3123,20 @@ ServerResource { state = State.Running; return new StartAnswer(cmd); - } catch (Exception e) { - s_logger.warn("Exception ", e); + } catch (LibvirtException e) { + s_logger.warn("LibvirtException ", e); + if (conn != null) { + handleVmStartFailure(conn, vmName, vm); + } + return new StartAnswer(cmd, e.getMessage()); + } catch (InternalErrorException e) { + s_logger.warn("InternalErrorException ", e); + if (conn != null) { + handleVmStartFailure(conn, vmName, vm); + } + return new StartAnswer(cmd, e.getMessage()); + } catch (URISyntaxException e) { + s_logger.warn("URISyntaxException ", e); if (conn != null) { handleVmStartFailure(conn, vmName, vm); } @@ -3325,14 +3336,10 @@ ServerResource { s_logger.debug("Ping command port, " + privateIp + ":" + cmdPort); } - try { - String result = _virtRouterResource.connect(privateIp, cmdPort); - if (result != null) { - return new CheckSshAnswer(cmd, "Can not ping System vm " - + vmName + "due to:" + result); - } - } catch (Exception e) { - return new CheckSshAnswer(cmd, e); + String result = _virtRouterResource.connect(privateIp, cmdPort); + if (result != null) { + return new CheckSshAnswer(cmd, "Can not ping System vm " + + vmName + "due to:" + result); } if (s_logger.isDebugEnabled()) { @@ -3479,14 +3486,12 @@ ServerResource { + e.getMessage()); } throw e; - } catch (Exception e) { - throw new InternalErrorException(e.toString()); } finally { if (dm != null) { try { dm.free(); } catch (LibvirtException l) { - + s_logger.trace("Ignoring libvirt error.", l); } } } @@ -3704,22 +3709,21 @@ ServerResource { return convertToState(vps); } } catch (final LibvirtException e) { - s_logger.trace(e.getMessage()); - } catch (Exception e) { - s_logger.trace(e.getMessage()); + s_logger.trace("Ignoring libvirt error.", e); } finally { try { if (dm != null) { dm.free(); } - } catch (final LibvirtException e) { - + } catch (final LibvirtException l) { + s_logger.trace("Ignoring libvirt error.", l); } } try { Thread.sleep(1000); } catch (InterruptedException e) { + s_logger.trace("Ignoring InterruptedException.", e); } } return State.Stopped; @@ -3757,7 +3761,7 @@ ServerResource { dm.free(); } } catch (final LibvirtException e) { - + s_logger.trace("Ignoring libvirt error.", e); } } } @@ -3812,7 +3816,7 @@ ServerResource { dm.free(); } } catch (LibvirtException e) { - + s_logger.trace("Ignoring libvirt error.", e); } } } @@ -3832,15 +3836,13 @@ ServerResource { vmStates.put(vmName, state); } catch (final LibvirtException e) { s_logger.warn("Unable to get vms", e); - } catch (Exception e) { - s_logger.warn("Unable to get vms", e); } finally { try { if (dm != null) { dm.free(); } } catch (LibvirtException e) { - + s_logger.trace("Ignoring libvirt error.", e); } } } @@ -3891,7 +3893,7 @@ ServerResource { } } } catch (LibvirtException e) { - + s_logger.trace("Ignoring libvirt error.", e); } if (isSnapshotSupported()) { @@ -3938,7 +3940,7 @@ ServerResource { } catch (LibvirtException e) { s_logger.warn("Failed to create vm", e); msg = e.getMessage(); - } catch (Exception e) { + } catch (InternalErrorException e) { s_logger.warn("Failed to create vm", e); msg = e.getMessage(); } finally { @@ -3947,7 +3949,7 @@ ServerResource { dm.free(); } } catch (LibvirtException e) { - + s_logger.trace("Ignoring libvirt error.", e); } } @@ -3977,15 +3979,13 @@ ServerResource { break; } catch (LibvirtException e) { s_logger.debug("Failed to get vm status:" + e.getMessage()); - } catch (Exception e) { - s_logger.debug("Failed to get vm status:" + e.getMessage()); } finally { try { if (dm != null) { dm.free(); } } catch (LibvirtException l) { - + s_logger.trace("Ignoring libvirt error.", l); } } } @@ -4040,15 +4040,13 @@ ServerResource { } catch (InterruptedException ie) { s_logger.debug("Interrupted sleep"); return ie.getMessage(); - } catch (Exception e) { - s_logger.debug("Failed to stop VM :" + vmName + " :", e); - return e.getMessage(); } finally { try { if (dm != null) { dm.free(); } } catch (LibvirtException e) { + s_logger.trace("Ignoring libvirt error.", e); } } @@ -4094,7 +4092,7 @@ ServerResource { dm.free(); } } catch (LibvirtException l) { - + s_logger.trace("Ignoring libvirt error.", l); } } } @@ -4110,7 +4108,7 @@ ServerResource { } } } catch (LibvirtException e) { - + s_logger.trace("Ignoring libvirt error.", e); } return false; } @@ -4135,8 +4133,7 @@ ServerResource { parser.parseDomainXML(xmlDesc); return parser.getDescription(); } catch (LibvirtException e) { - return null; - } catch (Exception e) { + s_logger.trace("Ignoring libvirt error.", e); return null; } finally { try { @@ -4144,7 +4141,7 @@ ServerResource { dm.free(); } } catch (LibvirtException l) { - + s_logger.trace("Ignoring libvirt error.", l); } } } @@ -4242,16 +4239,13 @@ ServerResource { } catch (LibvirtException e) { s_logger.debug("Failed to get dom xml: " + e.toString()); return new ArrayList(); - } catch (Exception e) { - s_logger.debug("Failed to get dom xml: " + e.toString()); - return new ArrayList(); } finally { try { if (dm != null) { dm.free(); } } catch (LibvirtException e) { - + s_logger.trace("Ignoring libvirt error.", e); } } } @@ -4268,16 +4262,13 @@ ServerResource { } catch (LibvirtException e) { s_logger.debug("Failed to get dom xml: " + e.toString()); return new ArrayList(); - } catch (Exception e) { - s_logger.debug("Failed to get dom xml: " + e.toString()); - return new ArrayList(); } finally { try { if (dm != null) { dm.free(); } } catch (LibvirtException e) { - + s_logger.trace("Ignoring libvirt error.", e); } } } @@ -4625,8 +4616,7 @@ ServerResource { conn = LibvirtConnection.getConnection(); success = default_network_rules_for_systemvm(conn, cmd.getVmName()); } catch (LibvirtException e) { - // TODO Auto-generated catch block - e.printStackTrace(); + s_logger.trace("Ignoring libvirt error.", e); } return new Answer(cmd, success, ""); diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtXMLParser.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtXMLParser.java index 3a614037d85..dd0d7724c4a 100644 --- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtXMLParser.java +++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtXMLParser.java @@ -19,6 +19,7 @@ package com.cloud.hypervisor.kvm.resource; import java.io.IOException; import java.io.StringReader; +import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; import javax.xml.parsers.SAXParserFactory; @@ -40,13 +41,14 @@ public class LibvirtXMLParser extends DefaultHandler { protected boolean _initialized = false; public LibvirtXMLParser() { - try { _sp = s_spf.newSAXParser(); _initialized = true; - } catch (Exception ex) { + } catch (ParserConfigurationException e) { + s_logger.trace("Ignoring xml parser error.", e); + } catch (SAXException e) { + s_logger.trace("Ignoring xml parser error.", e); } - } public boolean parseDomainXML(String domXML) {