You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Donal Lafferty <do...@citrix.com> on 2013/06/27 12:27:56 UTC

Review Request 12131: CLOUDSTACK-3215 Cannot Deploy VM when using S3 object store without NFS Cache

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12131/
-----------------------------------------------------------

Review request for cloudstack, edison su and Min Chen.


Bugs: CLOUDSTACK-3215


Repository: cloudstack-git


Description
-------

Fix https://issues.apache.org/jira/browse/CLOUDSTACK-3215 by changing code to not use a cache for image transfer if one can't be found.  Previously, the management server entered a failure state.
Also, added addition debug logging.


Diffs
-----

  engine/storage/cache/src/org/apache/cloudstack/storage/cache/manager/StorageCacheManagerImpl.java 4b4e52106ffbf70bcf2f6a656a8b8e4cacd6f91e 
  engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java 631de6a47a3eff510c84aa275fd87f8fa2f7780b 

Diff: https://reviews.apache.org/r/12131/diff/


Testing
-------

Code executed on deployement using S3 and no NFS cache.  Did not have facilities to test on S3 with a cache. 


Thanks,

Donal Lafferty


Re: Review Request 12131: CLOUDSTACK-3215 Cannot Deploy VM when using S3 object store without NFS Cache

Posted by Donal Lafferty <do...@citrix.com>.

> On June 27, 2013, 6:36 p.m., edison su wrote:
> > engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java, line 170
> > <https://reviews.apache.org/r/12131/diff/1/?file=312666#file312666line170>
> >
> >     
> >     In most of the cases(vmware/xen/kvm), must to have cache storage if S3 is used, in the current code. We can't say, if there is no cache storage available in the system for those hypervisors, we should throw exception immediately. 
> >     
> >     Better to add code in needCacheStorage(), or subclass ancientDataMotionStrategy for hyperV.
> >     
> >     For example, you can add following code in needcachestorage():
> >     
> >     if (srcData.getType() == DataObjectType.Template) {
> >        TemplateInfo template = (TemplateInfo)srcData;
> >        if (template.getHypervisorType() == HypervisorType.HperV) {
> >           return false; 
> >        }
> >     }
> >     }
> >     
> >

Sure, what do you want to throw?  E.g. what object type do you want to throw.


- Donal


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12131/#review22478
-----------------------------------------------------------


On June 27, 2013, 10:27 a.m., Donal Lafferty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12131/
> -----------------------------------------------------------
> 
> (Updated June 27, 2013, 10:27 a.m.)
> 
> 
> Review request for cloudstack, edison su and Min Chen.
> 
> 
> Bugs: CLOUDSTACK-3215
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Fix https://issues.apache.org/jira/browse/CLOUDSTACK-3215 by changing code to not use a cache for image transfer if one can't be found.  Previously, the management server entered a failure state.
> Also, added addition debug logging.
> 
> 
> Diffs
> -----
> 
>   engine/storage/cache/src/org/apache/cloudstack/storage/cache/manager/StorageCacheManagerImpl.java 4b4e52106ffbf70bcf2f6a656a8b8e4cacd6f91e 
>   engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java 631de6a47a3eff510c84aa275fd87f8fa2f7780b 
> 
> Diff: https://reviews.apache.org/r/12131/diff/
> 
> 
> Testing
> -------
> 
> Code executed on deployement using S3 and no NFS cache.  Did not have facilities to test on S3 with a cache. 
> 
> 
> Thanks,
> 
> Donal Lafferty
> 
>


Re: Review Request 12131: CLOUDSTACK-3215 Cannot Deploy VM when using S3 object store without NFS Cache

Posted by edison su <ed...@citrix.com>.

> On June 27, 2013, 6:36 p.m., edison su wrote:
> > engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java, line 170
> > <https://reviews.apache.org/r/12131/diff/1/?file=312666#file312666line170>
> >
> >     
> >     In most of the cases(vmware/xen/kvm), must to have cache storage if S3 is used, in the current code. We can't say, if there is no cache storage available in the system for those hypervisors, we should throw exception immediately. 
> >     
> >     Better to add code in needCacheStorage(), or subclass ancientDataMotionStrategy for hyperV.
> >     
> >     For example, you can add following code in needcachestorage():
> >     
> >     if (srcData.getType() == DataObjectType.Template) {
> >        TemplateInfo template = (TemplateInfo)srcData;
> >        if (template.getHypervisorType() == HypervisorType.HperV) {
> >           return false; 
> >        }
> >     }
> >     }
> >     
> >
> 
> Donal Lafferty wrote:
>     Sure, what do you want to throw?  E.g. what object type do you want to throw.

In StorageCacheManagerImpl->createCacheObject(DataObject data, Scope scope): we should have the following code:
DataStore cacheStore = this.getCacheStorage(scope);
if (cacheStore == null) {
   s_logger.debug(); throw new CloudRuntimeException("something");
}


- edison


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12131/#review22478
-----------------------------------------------------------


On June 27, 2013, 10:27 a.m., Donal Lafferty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12131/
> -----------------------------------------------------------
> 
> (Updated June 27, 2013, 10:27 a.m.)
> 
> 
> Review request for cloudstack, edison su and Min Chen.
> 
> 
> Bugs: CLOUDSTACK-3215
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Fix https://issues.apache.org/jira/browse/CLOUDSTACK-3215 by changing code to not use a cache for image transfer if one can't be found.  Previously, the management server entered a failure state.
> Also, added addition debug logging.
> 
> 
> Diffs
> -----
> 
>   engine/storage/cache/src/org/apache/cloudstack/storage/cache/manager/StorageCacheManagerImpl.java 4b4e52106ffbf70bcf2f6a656a8b8e4cacd6f91e 
>   engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java 631de6a47a3eff510c84aa275fd87f8fa2f7780b 
> 
> Diff: https://reviews.apache.org/r/12131/diff/
> 
> 
> Testing
> -------
> 
> Code executed on deployement using S3 and no NFS cache.  Did not have facilities to test on S3 with a cache. 
> 
> 
> Thanks,
> 
> Donal Lafferty
> 
>


Re: Review Request 12131: CLOUDSTACK-3215 Cannot Deploy VM when using S3 object store without NFS Cache

Posted by edison su <ed...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12131/#review22478
-----------------------------------------------------------



engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java
<https://reviews.apache.org/r/12131/#comment46022>

    
    In most of the cases(vmware/xen/kvm), must to have cache storage if S3 is used, in the current code. We can't say, if there is no cache storage available in the system for those hypervisors, we should throw exception immediately. 
    
    Better to add code in needCacheStorage(), or subclass ancientDataMotionStrategy for hyperV.
    
    For example, you can add following code in needcachestorage():
    
    if (srcData.getType() == DataObjectType.Template) {
       TemplateInfo template = (TemplateInfo)srcData;
       if (template.getHypervisorType() == HypervisorType.HperV) {
          return false; 
       }
    }
    }
    
    


- edison su


On June 27, 2013, 10:27 a.m., Donal Lafferty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12131/
> -----------------------------------------------------------
> 
> (Updated June 27, 2013, 10:27 a.m.)
> 
> 
> Review request for cloudstack, edison su and Min Chen.
> 
> 
> Bugs: CLOUDSTACK-3215
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Fix https://issues.apache.org/jira/browse/CLOUDSTACK-3215 by changing code to not use a cache for image transfer if one can't be found.  Previously, the management server entered a failure state.
> Also, added addition debug logging.
> 
> 
> Diffs
> -----
> 
>   engine/storage/cache/src/org/apache/cloudstack/storage/cache/manager/StorageCacheManagerImpl.java 4b4e52106ffbf70bcf2f6a656a8b8e4cacd6f91e 
>   engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java 631de6a47a3eff510c84aa275fd87f8fa2f7780b 
> 
> Diff: https://reviews.apache.org/r/12131/diff/
> 
> 
> Testing
> -------
> 
> Code executed on deployement using S3 and no NFS cache.  Did not have facilities to test on S3 with a cache. 
> 
> 
> Thanks,
> 
> Donal Lafferty
> 
>


Re: Review Request 12131: CLOUDSTACK-3215 Cannot Deploy VM when using S3 object store without NFS Cache

Posted by edison su <ed...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12131/#review22548
-----------------------------------------------------------

Ship it!


Ship It!

- edison su


On June 28, 2013, 7:46 p.m., Donal Lafferty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12131/
> -----------------------------------------------------------
> 
> (Updated June 28, 2013, 7:46 p.m.)
> 
> 
> Review request for cloudstack, edison su and Min Chen.
> 
> 
> Bugs: CLOUDSTACK-3215
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Fix https://issues.apache.org/jira/browse/CLOUDSTACK-3215 by changing code to not use a cache for image transfer if one can't be found.  Previously, the management server entered a failure state.
> Also, added addition debug logging.
> 
> 
> Diffs
> -----
> 
>   engine/storage/cache/src/org/apache/cloudstack/storage/cache/manager/StorageCacheManagerImpl.java 4b4e52106ffbf70bcf2f6a656a8b8e4cacd6f91e 
>   engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java 631de6a47a3eff510c84aa275fd87f8fa2f7780b 
> 
> Diff: https://reviews.apache.org/r/12131/diff/
> 
> 
> Testing
> -------
> 
> Code executed on deployement using S3 and no NFS cache.  Did not have facilities to test on S3 with a cache. 
> 
> 
> Thanks,
> 
> Donal Lafferty
> 
>


Re: Review Request 12131: CLOUDSTACK-3215 Cannot Deploy VM when using S3 object store without NFS Cache

Posted by ASF Subversion and Git Services <as...@urd.zones.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12131/#review24922
-----------------------------------------------------------


Commit 6b905528559f1f1b6ae472edc550c31d59e8461d in branch refs/heads/4.1 from Rajesh Battala
[ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=6b90552 ]

CLOUDSTACK-3215 Logs in the Software router are not being rotated


- ASF Subversion and Git Services


On June 28, 2013, 7:46 p.m., Donal Lafferty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12131/
> -----------------------------------------------------------
> 
> (Updated June 28, 2013, 7:46 p.m.)
> 
> 
> Review request for cloudstack, edison su and Min Chen.
> 
> 
> Bugs: CLOUDSTACK-3215
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Fix https://issues.apache.org/jira/browse/CLOUDSTACK-3215 by changing code to not use a cache for image transfer if one can't be found.  Previously, the management server entered a failure state.
> Also, added addition debug logging.
> 
> 
> Diffs
> -----
> 
>   engine/storage/cache/src/org/apache/cloudstack/storage/cache/manager/StorageCacheManagerImpl.java 4b4e52106ffbf70bcf2f6a656a8b8e4cacd6f91e 
>   engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java 631de6a47a3eff510c84aa275fd87f8fa2f7780b 
> 
> Diff: https://reviews.apache.org/r/12131/diff/
> 
> 
> Testing
> -------
> 
> Code executed on deployement using S3 and no NFS cache.  Did not have facilities to test on S3 with a cache. 
> 
> 
> Thanks,
> 
> Donal Lafferty
> 
>


Re: Review Request 12131: CLOUDSTACK-3215 Cannot Deploy VM when using S3 object store without NFS Cache

Posted by ASF Subversion and Git Services <as...@urd.zones.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12131/#review24115
-----------------------------------------------------------


Commit 8edf6b346789f826847245472977913b56bce17e in branch refs/heads/master from Rajesh Battala
[ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=8edf6b3 ]

CLOUDSTACK-3215 Logs in the Software router are not being rotated


- ASF Subversion and Git Services


On June 28, 2013, 7:46 p.m., Donal Lafferty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12131/
> -----------------------------------------------------------
> 
> (Updated June 28, 2013, 7:46 p.m.)
> 
> 
> Review request for cloudstack, edison su and Min Chen.
> 
> 
> Bugs: CLOUDSTACK-3215
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Fix https://issues.apache.org/jira/browse/CLOUDSTACK-3215 by changing code to not use a cache for image transfer if one can't be found.  Previously, the management server entered a failure state.
> Also, added addition debug logging.
> 
> 
> Diffs
> -----
> 
>   engine/storage/cache/src/org/apache/cloudstack/storage/cache/manager/StorageCacheManagerImpl.java 4b4e52106ffbf70bcf2f6a656a8b8e4cacd6f91e 
>   engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java 631de6a47a3eff510c84aa275fd87f8fa2f7780b 
> 
> Diff: https://reviews.apache.org/r/12131/diff/
> 
> 
> Testing
> -------
> 
> Code executed on deployement using S3 and no NFS cache.  Did not have facilities to test on S3 with a cache. 
> 
> 
> Thanks,
> 
> Donal Lafferty
> 
>


Re: Review Request 12131: CLOUDSTACK-3215 Cannot Deploy VM when using S3 object store without NFS Cache

Posted by ASF Subversion and Git Services <as...@urd.zones.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12131/#review22547
-----------------------------------------------------------


Commit 58f9202818ffe78f3cd6748fc8710433d11bd5a2 in branch refs/heads/master from Donal Lafferty
[ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=58f9202 ]

CLOUDSTACK-3215: Cannot Deploy VM when using S3 object store without NFS Cache

Signed-off-by: Edison Su <su...@gmail.com>


- ASF Subversion and Git Services


On June 28, 2013, 7:46 p.m., Donal Lafferty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12131/
> -----------------------------------------------------------
> 
> (Updated June 28, 2013, 7:46 p.m.)
> 
> 
> Review request for cloudstack, edison su and Min Chen.
> 
> 
> Bugs: CLOUDSTACK-3215
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Fix https://issues.apache.org/jira/browse/CLOUDSTACK-3215 by changing code to not use a cache for image transfer if one can't be found.  Previously, the management server entered a failure state.
> Also, added addition debug logging.
> 
> 
> Diffs
> -----
> 
>   engine/storage/cache/src/org/apache/cloudstack/storage/cache/manager/StorageCacheManagerImpl.java 4b4e52106ffbf70bcf2f6a656a8b8e4cacd6f91e 
>   engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java 631de6a47a3eff510c84aa275fd87f8fa2f7780b 
> 
> Diff: https://reviews.apache.org/r/12131/diff/
> 
> 
> Testing
> -------
> 
> Code executed on deployement using S3 and no NFS cache.  Did not have facilities to test on S3 with a cache. 
> 
> 
> Thanks,
> 
> Donal Lafferty
> 
>


Re: Review Request 12131: CLOUDSTACK-3215 Cannot Deploy VM when using S3 object store without NFS Cache

Posted by ASF Subversion and Git Services <as...@urd.zones.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12131/#review24113
-----------------------------------------------------------


Commit 50fe9e1cbf1a25cfe6b9d0760cb14d23d1cfbc72 in branch refs/heads/4.2 from Rajesh Battala
[ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=50fe9e1 ]

CLOUDSTACK-3215 Logs in the Software router are not being rotated


- ASF Subversion and Git Services


On June 28, 2013, 7:46 p.m., Donal Lafferty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12131/
> -----------------------------------------------------------
> 
> (Updated June 28, 2013, 7:46 p.m.)
> 
> 
> Review request for cloudstack, edison su and Min Chen.
> 
> 
> Bugs: CLOUDSTACK-3215
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Fix https://issues.apache.org/jira/browse/CLOUDSTACK-3215 by changing code to not use a cache for image transfer if one can't be found.  Previously, the management server entered a failure state.
> Also, added addition debug logging.
> 
> 
> Diffs
> -----
> 
>   engine/storage/cache/src/org/apache/cloudstack/storage/cache/manager/StorageCacheManagerImpl.java 4b4e52106ffbf70bcf2f6a656a8b8e4cacd6f91e 
>   engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java 631de6a47a3eff510c84aa275fd87f8fa2f7780b 
> 
> Diff: https://reviews.apache.org/r/12131/diff/
> 
> 
> Testing
> -------
> 
> Code executed on deployement using S3 and no NFS cache.  Did not have facilities to test on S3 with a cache. 
> 
> 
> Thanks,
> 
> Donal Lafferty
> 
>


Re: Review Request 12131: CLOUDSTACK-3215 Cannot Deploy VM when using S3 object store without NFS Cache

Posted by Donal Lafferty <do...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12131/
-----------------------------------------------------------

(Updated June 28, 2013, 7:46 p.m.)


Review request for cloudstack, edison su and Min Chen.


Changes
-------

Updated code to address comments by Edison Su.


Bugs: CLOUDSTACK-3215


Repository: cloudstack-git


Description
-------

Fix https://issues.apache.org/jira/browse/CLOUDSTACK-3215 by changing code to not use a cache for image transfer if one can't be found.  Previously, the management server entered a failure state.
Also, added addition debug logging.


Diffs (updated)
-----

  engine/storage/cache/src/org/apache/cloudstack/storage/cache/manager/StorageCacheManagerImpl.java 4b4e52106ffbf70bcf2f6a656a8b8e4cacd6f91e 
  engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java 631de6a47a3eff510c84aa275fd87f8fa2f7780b 

Diff: https://reviews.apache.org/r/12131/diff/


Testing
-------

Code executed on deployement using S3 and no NFS cache.  Did not have facilities to test on S3 with a cache. 


Thanks,

Donal Lafferty