Re: Review Request 28965: CLOUDSTACK-7881: Allow VPN IP range to be specified when creating a VPN

2014-12-13 Thread Logan B

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

(Updated Dec. 13, 2014, 3:20 p.m.)


Review request for cloudstack, Brian Federle, Gabor Apati-Nagy, Jessica Wang, 
and Mihaela Stoica.


Changes
---

Had to look up Gabor


Repository: cloudstack-git


Description
---

Added optional "Start IP" and "End IP" fields to the Create Remote Access VPN 
confirmation in the UI.


Diffs
-

  ui/scripts/network.js f934f21 

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


Testing
---

Tested creating VPN with several inputs, including leaving the fields blank.

Results:

- No input: VPN deployed with globally specified VPN IP range.
- IP in only one field: VPN deployed with globally specified VPN IP range.
- IP in both fields: VPN deployed with client specified VPN IP range.
- Random data in one or both fields: VPN deployed with globally specified VPN 
IP range.


Thanks,

Logan B



Re: Review Request 28965: CLOUDSTACK-7881: Allow VPN IP range to be specified when creating a VPN

2014-12-13 Thread Logan B

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

(Updated Dec. 13, 2014, 3:02 p.m.)


Review request for cloudstack, Brian Federle, Jessica Wang, and Mihaela Stoica.


Changes
---

added some people that can judge thi change more then I. LTGM


Repository: cloudstack-git


Description
---

Added optional "Start IP" and "End IP" fields to the Create Remote Access VPN 
confirmation in the UI.


Diffs
-

  ui/scripts/network.js f934f21 

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


Testing
---

Tested creating VPN with several inputs, including leaving the fields blank.

Results:

- No input: VPN deployed with globally specified VPN IP range.
- IP in only one field: VPN deployed with globally specified VPN IP range.
- IP in both fields: VPN deployed with client specified VPN IP range.
- Random data in one or both fields: VPN deployed with globally specified VPN 
IP range.


Thanks,

Logan B



Review Request 28965: CLOUDSTACK-7881: Allow VPN IP range to be specified when creating a VPN

2014-12-11 Thread Logan B

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

Review request for cloudstack.


Repository: cloudstack-git


Description
---

Added optional "Start IP" and "End IP" fields to the Create Remote Access VPN 
confirmation in the UI.


Diffs
-

  ui/scripts/network.js f934f21 

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


Testing
---

Tested creating VPN with several inputs, including leaving the fields blank.

Results:

- No input: VPN deployed with globally specified VPN IP range.
- IP in only one field: VPN deployed with globally specified VPN IP range.
- IP in both fields: VPN deployed with client specified VPN IP range.
- Random data in one or both fields: VPN deployed with globally specified VPN 
IP range.


Thanks,

Logan B



Re: Review Request 27393: Fix for Resize RBD Root Disk on Deploy

2014-11-03 Thread Logan B

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

Ship it!


Ship It!

- Logan B


On Oct. 30, 2014, 8:37 p.m., Logan B wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27393/
> ---
> 
> (Updated Oct. 30, 2014, 8:37 p.m.)
> 
> 
> Review request for cloudstack and Wido den Hollander.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> ---
> 
> Currently if you specify a root disk size when deploying an instance using 
> Ceph RBD as the Primary Storage backend the disk will not be resized.
> 
> This patch adds a check after deploying an instance that checks the specified 
> root disk volume size against the size of the template it was deployed from.  
> If the specified volume size is larger than the template it will resize the 
> root disk volume to the specified size.
> 
> If the specified volume size is equal to or smaller than the template size 
> then no resizing occurs, and the instance will be deployed with a root disk 
> size equal to the size of the template.
> 
> 
> Diffs
> -
> 
>   
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java
>  af7a0e0 
> 
> Diff: https://reviews.apache.org/r/27393/diff/
> 
> 
> Testing
> ---
> 
> Test: Created new instance from 5GB template with 20GB root disk size 
> specified.
> Result: Instance created successfully with 20GB volume size.
> 
> Test: Created new instance from 5GB template with 5GB root disk size 
> specified.
> Result: Instance created successfully with 5GB volume size.
> 
> Test: Created new instance from 5GB template with 1GB root disk size 
> specified.
> Result: Instance created successfully with 5GB volume size.
> 
> Test: Created new instance from 5GB template with no root disk size specified.
> Result: Instance created successfully with 5GB volume size.
> 
> Patch was deployed to our production infrastructure on July 25th, 2014.  It's 
> been running stable since that time.
> 
> 
> Thanks,
> 
> Logan B
> 
>



Review Request 27393: Fix for Resize RBD Root Disk on Deploy

2014-10-30 Thread Logan B

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

Review request for cloudstack and Wido den Hollander.


Repository: cloudstack-git


Description
---

Currently if you specify a root disk size when deploying an instance using Ceph 
RBD as the Primary Storage backend the disk will not be resized.

This patch adds a check after deploying an instance that checks the specified 
root disk volume size against the size of the template it was deployed from.  
If the specified volume size is larger than the template it will resize the 
root disk volume to the specified size.

If the specified volume size is equal to or smaller than the template size then 
no resizing occurs, and the instance will be deployed with a root disk size 
equal to the size of the template.


Diffs
-

  
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java
 af7a0e0 

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


Testing
---

Test: Created new instance from 5GB template with 20GB root disk size specified.
Result: Instance created successfully with 20GB volume size.

Test: Created new instance from 5GB template with 5GB root disk size specified.
Result: Instance created successfully with 5GB volume size.

Test: Created new instance from 5GB template with 1GB root disk size specified.
Result: Instance created successfully with 5GB volume size.

Test: Created new instance from 5GB template with no root disk size specified.
Result: Instance created successfully with 5GB volume size.

Patch was deployed to our production infrastructure on July 25th, 2014.  It's 
been running stable since that time.


Thanks,

Logan B



Review Request 22820: Fix: CLOUDSTACK-6938 -> Cannot create template from snapshot when using S3 secondary storage

2014-06-20 Thread Logan B

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

Review request for cloudstack.


Repository: cloudstack-git


Description
---

This is a proposed fix for: 
https://issues.apache.org/jira/browse/CLOUDSTACK-6938

Previously when attempting to create a template from a snapshot stored in S3 
the task would fail with "Unable to create directory."  This was due to the 
fact that Java's 'mkdirs()' directive returns 'false' if the directory already 
exists.  I just changed the logic to check for an existing directory first, 
then attempt to create it if one doesn't already exist.  The task will still 
fail if it legitimately can't create a folder, but should work if one already 
exists now.

Original code:
if (!downloadDirectory.mkdirs()) { 
final String errMsg = "Unable to create directory " + downloadPath + " to 
copy from S3 to cache.";
s_logger.error(errMsg);
return new CopyCmdAnswer(errMsg);
} else { 
   s_logger.debug("Directory " + downloadPath + " already exists"); 
}

New code:
if (downloadDirectory.exists()) { 
s_logger.debug("Directory " + downloadPath + " already exists"); 
} else {
if (!downloadDirectory.mkdirs()) {
final String errMsg = "Unable to create directory " + downloadPath + " 
to copy from S3 to cache.";
s_logger.error(errMsg);
return new CopyCmdAnswer(errMsg);
}


Diffs
-

  
services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
 6927f02 

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


Testing
---

Tested this on the CloudStack 4.4-SNAPSHOT.  Original code produced a failure 
with an existing directory.  I replaced the 
'cloud-secondary-storage-4.4.0-SNAPSHOT.jar' on the SSVM, and it no longer 
failed.

The only 'bug' I can see is that the 's_logger.debug' message for the .exists() 
check doesn't seem to appear in the SSVM logs.


Thanks,

Logan B