You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Daan Hoogland <da...@gmail.com> on 2014/04/22 22:14:49 UTC

Re: git commit: updated refs/heads/master to 4cb3e55

Anthony,

Are you sure it is needed to catch Exception in these cases? It seems
unnecessary and bad practice. Can you revert, please?

regards,
Daan

On Tue, Apr 22, 2014 at 8:44 PM,  <an...@apache.org> wrote:
> Repository: cloudstack
> Updated Branches:
>   refs/heads/master 1bb31412f -> 4cb3e553d
>
>
> enable XS event for XS 6.2 + SP1 + FOX
>
>
> Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
> Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/4cb3e553
> Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/4cb3e553
> Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/4cb3e553
>
> Branch: refs/heads/master
> Commit: 4cb3e553d5b48e9ee7d6267562084ac1cdd528c8
> Parents: 1bb3141
> Author: Anthony Xu <an...@citrix.com>
> Authored: Tue Apr 22 11:43:01 2014 -0700
> Committer: Anthony Xu <an...@citrix.com>
> Committed: Tue Apr 22 11:43:43 2014 -0700
>
> ----------------------------------------------------------------------
>  .../xen/resource/CitrixResourceBase.java        | 26 +++++++++-----------
>  .../xenserver/XenServerResourceNewBase.java     | 20 +++++++++++----
>  2 files changed, 26 insertions(+), 20 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4cb3e553/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
> ----------------------------------------------------------------------
> diff --git a/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java b/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
> index 6226b2e..ba8be91 100644
> --- a/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
> +++ b/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
> @@ -214,6 +214,7 @@ import org.w3c.dom.Node;
>  import org.w3c.dom.NodeList;
>  import org.xml.sax.InputSource;
>  import org.xml.sax.SAXException;
> +import java.util.concurrent.TimeoutException;
>
>  import javax.ejb.Local;
>  import javax.naming.ConfigurationException;
> @@ -664,16 +665,11 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
>                      + " with args " + getArgsString(args)
>                      + " due to HandleInvalid clazz:" + e.clazz + ", handle:"
>                      + e.handle);
> -        } catch (XenAPIException e) {
> +        } catch (Exception e) {
>              s_logger.warn(
>                      "callHostPlugin failed for cmd: " + cmd + " with args "
>                              + getArgsString(args) + " due to " + e.toString(),
>                              e);
> -        } catch (XmlRpcException e) {
> -            s_logger.warn(
> -                    "callHostPlugin failed for cmd: " + cmd + " with args "
> -                            + getArgsString(args) + " due to " + e.getMessage(),
> -                            e);
>          } finally {
>              if (task != null) {
>                  try {
> @@ -3310,7 +3306,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
>          return false;
>      }
>
> -    protected void waitForTask(Connection c, Task task, long pollInterval, long timeout) throws XenAPIException, XmlRpcException {
> +    protected void waitForTask(Connection c, Task task, long pollInterval, long timeout) throws XenAPIException, XmlRpcException, TimeoutException {
>          long beginTime = System.currentTimeMillis();
>          if (s_logger.isTraceEnabled()) {
>              s_logger.trace("Task " + task.getNameLabel(c) + " (" + task.getUuid(c) + ") sent to " + c.getSessionReference() + " is pending completion with a " + timeout +
> @@ -3329,7 +3325,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
>                  s_logger.warn(msg);
>                  task.cancel(c);
>                  task.destroy(c);
> -                throw new Types.BadAsyncResult(msg);
> +                throw new TimeoutException(msg);
>              }
>          }
>      }
> @@ -3349,7 +3345,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
>          }
>      }
>
> -    void rebootVM(Connection conn, VM vm, String vmName) throws XmlRpcException {
> +    void rebootVM(Connection conn, VM vm, String vmName) throws Exception {
>          Task task = null;
>          try {
>              task = vm.cleanRebootAsync(conn);
> @@ -3405,7 +3401,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
>                  //poll every 1 seconds , timeout after 10 minutes
>                  waitForTask(conn, task, 1000, 10 * 60 * 1000);
>                  checkForSuccess(conn, task);
> -            } catch (Types.HandleInvalid e) {
> +            } catch (TimeoutException e) {
>                  if (vm.getPowerState(conn) == Types.VmPowerState.HALTED) {
>                      task = null;
>                      return;
> @@ -3450,7 +3446,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
>          }
>      }
>
> -    void startVM(Connection conn, Host host, VM vm, String vmName) throws XmlRpcException {
> +    void startVM(Connection conn, Host host, VM vm, String vmName) throws Exception {
>          Task task = null;
>          try {
>              task = vm.startOnAsync(conn, host, false, true);
> @@ -3465,7 +3461,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
>                      return;
>                  }
>                  throw new CloudRuntimeException("Start VM " + vmName + " catch HandleInvalid and VM is not in RUNNING state");
> -            } catch (Types.BadAsyncResult e) {
> +            } catch (TimeoutException e) {
>                  if (vm.getPowerState(conn) == Types.VmPowerState.RUNNING) {
>                      s_logger.debug("VM " + vmName + " is in Running status");
>                      task = null;
> @@ -3488,7 +3484,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
>          }
>      }
>
> -    private void migrateVM(Connection conn, Host destHost, VM vm, String vmName) throws XmlRpcException {
> +    private void migrateVM(Connection conn, Host destHost, VM vm, String vmName) throws Exception {
>          Task task = null;
>          try {
>              Map<String, String> other = new HashMap<String, String>();
> @@ -3521,7 +3517,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
>          }
>      }
>
> -    protected VDI cloudVDIcopy(Connection conn, VDI vdi, SR sr, int wait) throws XenAPIException, XmlRpcException {
> +    protected VDI cloudVDIcopy(Connection conn, VDI vdi, SR sr, int wait) throws Exception {
>          Task task = null;
>          if (wait == 0) {
>              wait = 2 * 60 * 60;
> @@ -3570,7 +3566,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
>                      e.handle);
>          } catch (XenAPIException e) {
>              s_logger.warn("callHostPlugin failed for cmd: " + cmd + " with args " + getArgsString(args) + " due to " + e.toString(), e);
> -        } catch (XmlRpcException e) {
> +        } catch (Exception e) {
>              s_logger.warn("callHostPlugin failed for cmd: " + cmd + " with args " + getArgsString(args) + " due to " + e.getMessage(), e);
>          } finally {
>              if (task != null) {
>
> http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4cb3e553/plugins/hypervisors/xen/src/org/apache/cloudstack/hypervisor/xenserver/XenServerResourceNewBase.java
> ----------------------------------------------------------------------
> diff --git a/plugins/hypervisors/xen/src/org/apache/cloudstack/hypervisor/xenserver/XenServerResourceNewBase.java b/plugins/hypervisors/xen/src/org/apache/cloudstack/hypervisor/xenserver/XenServerResourceNewBase.java
> index c67fb33..1883371 100644
> --- a/plugins/hypervisors/xen/src/org/apache/cloudstack/hypervisor/xenserver/XenServerResourceNewBase.java
> +++ b/plugins/hypervisors/xen/src/org/apache/cloudstack/hypervisor/xenserver/XenServerResourceNewBase.java
> @@ -99,14 +99,14 @@ public class XenServerResourceNewBase extends XenServer620SP1Resource {
>          return cmds;
>      }
>
> -    protected void waitForTask2(Connection c, Task task, long pollInterval, long timeout) throws XenAPIException, XmlRpcException, TimeoutException {
> +    protected void waitForTask(Connection c, Task task, long pollInterval, long timeout) throws XenAPIException, XmlRpcException, TimeoutException {
>          long beginTime = System.currentTimeMillis();
>          if (s_logger.isTraceEnabled()) {
>              s_logger.trace("Task " + task.getNameLabel(c) + " (" + task.getType(c) + ") sent to " + c.getSessionReference() + " is pending completion with a " + timeout +
>                             "ms timeout");
>          }
>          Set<String> classes = new HashSet<String>();
> -        classes.add("Task/" + task.toString());
> +        classes.add("Task/" + task.toWireString());
>          String token = "";
>          Double t = new Double(timeout / 1000);
>          while (true) {
> @@ -115,7 +115,7 @@ public class XenServerResourceNewBase extends XenServer620SP1Resource {
>              @SuppressWarnings("unchecked")
>              Set<Event.Record> events = map.events;
>              if (events.size() == 0) {
> -                String msg = "Async " + timeout / 1000 + " seconds timeout for task " + task.toString();
> +                String msg = "No event for task " + task.toWireString();
>                  s_logger.warn(msg);
>                  task.cancel(c);
>                  throw new TimeoutException(msg);
> @@ -132,16 +132,26 @@ public class XenServerResourceNewBase extends XenServer620SP1Resource {
>
>                  if (taskRecord.status != Types.TaskStatusType.PENDING) {
>                      if (s_logger.isDebugEnabled()) {
> -                        s_logger.debug("Task is done " + taskRecord.status);
> +                        s_logger.debug("Task, ref:" + task.toWireString() + ", UUID:" + taskRecord.uuid + " is done " + taskRecord.status);
>                      }
>                      return;
>                  } else {
> -                    s_logger.debug("Task is not done " + taskRecord);
> +                    if (s_logger.isDebugEnabled()) {
> +                        s_logger.debug("Task: ref:" + task.toWireString() + ", UUID:" + taskRecord.uuid +  " progress: " + taskRecord.progress);
> +                    }
> +
>                  }
>              }
> +            if (System.currentTimeMillis() - beginTime > timeout) {
> +                String msg = "Async " + timeout / 1000 + " seconds timeout for task " + task.toString();
> +                s_logger.warn(msg);
> +                task.cancel(c);
> +                throw new TimeoutException(msg);
> +            }
>          }
>      }
>
> +
>      @Override
>      protected Answer execute(final ClusterSyncCommand cmd) {
>          if (!_listener.isListening()) {
>



-- 
Daan