Re: Virtual Router : Marvin test gap analysis

[Murali Reddy]

We (Abhi, Boris and me) opened couple of marvin test bugs to automate the gaps 
[1], we will start to work on them. If any one wishes to contribute please feel 
free to pick them up.

[1] 
https://cwiki.apache.org/confluence/display/CLOUDSTACK/Virtual+Router%3A+Smoke+and+Integration+tests+gap+analysis




On 26/08/16, 9:35 PM, "Murali Reddy"  wrote:

>Raja,
>
>Thanks for sharing insightful automation analysis. We are trying to figure out 
>how to convert our efforts in to actionable items. As community we can work on 
>them to get good automated test suite for VR.
>
>
>
>Will be sharing details early next week.
>
>Thanks.
>
>On 19/08/16, 2:25 PM, "Raja Pullela"  wrote:
>
>>Hi Murali, 
>>
>>Great initiative… and VR is one area that could help greatly if we have more 
>>automation.   we have done high-level analysis on VR functionality/automation 
>>recently and have posted our findings to the wiki under “high-level 
>>Automation analysis” section.  Please take a look, 
>>
>>[1] 
>>https://cwiki.apache.org/confluence/display/CLOUDSTACK/Virtual+Router%3A+Smoke+and+Integration+tests+gap+analysis
>>
>>Best,
>>Raja
>>Senior Manager, Product Development
>>Accelerite, www.accelerite.com, @accelerite
>>2055, Laurelwood Road,  Santa Clara, CA 95054, USA
>>Phone: 1-408-216-7010
>>
>>On 8/19/16, 12:33 PM, "Murali Reddy"  wrote:
>>
>>All,
>>
>>We (at ShapeBlue) did a gap analysis to figure if current set of smoke and 
>>component tests sufficiently test the VR functionality for the regressions. I 
>>have posted the analysis at [1]. I went through the test suites, and listed 
>>down all the tests that touch virtual router functionality. There is listing 
>>of general observations on grey areas.
>>
>>One particular area where there were no tests, was related multiple public 
>>IP’s from different public IP ranges associated with a network. From 4.6, all 
>>the way to master only IP’s from one public IP range (eth2 on the VR) is 
>>working, any network services  on the public IP’s on eth3, eth4 etc on VR are 
>>not functional. This is a common use case and is broken for last few 
>>releases. Bug in this area are reported [2] and PR is yet to be merged [3]. I 
>>will be work on the patch to get this fixed in LTS. I have also added Marvin 
>>tests [4] cover multiple public IP scenarios. 
>>
>>Given the flexibility and rich set of network functionality is CloudStack, we 
>>could catch regression only if we have good test suite. If there are any 
>>other areas related to virtual router functionality that you see there are 
>>significant gaps, please chime in share your thoughts or add the the wiki.
>>
>>[1] 
>>https://cwiki.apache.org/confluence/display/CLOUDSTACK/Virtual+Router%3A+Smoke+and+Integration+tests+gap+analysis
>>[2] https://issues.apache.org/jira/browse/CLOUDSTACK-9339
>>[3] https://github.com/apache/cloudstack/pull/1519
>>[4] 
>>https://github.com/murali-reddy/cloudstack/commit/0b6fbc29fcadb39b08d0050ca473680a614dfab4
>>
>>
>>
>>
>>
>>
>>
>>DISCLAIMER
>>==
>>This e-mail may contain privileged and confidential information which is the 
>>property of Accelerite, a Persistent Systems business. It is intended only 
>>for the use of the individual or entity to which it is addressed. If you are 
>>not the intended recipient, you are not authorized to read, retain, copy, 
>>print, distribute or use this message. If you have received this 
>>communication in error, please notify the sender and delete all copies of 
>>this message. Accelerite, a Persistent Systems business does not accept any 
>>liability for virus infected mails.
>

[GitHub] cloudstack pull request #1666: CLOUDSTACK-9480: Egress Firewall: Incorrect u...

[jayapalu]

Github user jayapalu commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1666#discussion_r77116561
  
--- Diff: systemvm/patches/debian/config/opt/cloud/bin/configure.py ---
@@ -145,40 +145,41 @@ def add_rule(self, cidr):
 logging.debug("Current ACL IP direction is ==> %s", 
self.direction)
 if self.direction == 'egress':
 self.fw.append(["filter", "", " -A FW_OUTBOUND -j 
FW_EGRESS_RULES"])
+fwr = " -I FW_EGRESS_RULES"
+
+#In case we have a default rule (accept all or drop all), 
we have to evaluate the action again.
+if self.rule['cidr'] == ['0.0.0.0/0']:
--- End diff --

Here the default rule when egress policy is true is identified using the 
cidr '0.0.0.0/0'. What if we configure a egress rule  (after the network is up 
) with cidr '0.0.0.0/0' (ex: cidr 0.0.0.0/0, tcp, 22 egress policy on 
network:true)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1654: Updating pom.xml version numbers for release 4.8.2.0...

[rhtyd]

Github user rhtyd commented on the issue:

https://github.com/apache/cloudstack/pull/1654
  
Lgtm.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1605: CLOUDSTACK-9428: Fix for CLOUDSTACK-9211 - Improve p...

[nvazquez]

Github user nvazquez commented on the issue:

https://github.com/apache/cloudstack/pull/1605
  
@jburwell I added marvin test, this are results on VMware env:

```
[root@ussarlabcsmgt41 cloudstack]# cat 
/tmp//MarvinLogs/test_deploy_vgpu_enabled_vm_OG63Q9/results.txt
# 1. Register a template for VMware with nicAdapter vmxnet3 and 3D GPU 
details ... === TestName: test_3d_gpu_support | Status : SUCCESS ===
ok
Test Deploy Virtual Machine ... SKIP: This test case is written 
specificallyfor XenServer hypervisor

--
Ran 2 tests in 739.974s

OK (SKIP=1)
```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1600#discussion_r77080602
  
--- Diff: 
engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java
 ---
@@ -212,20 +251,47 @@ public SnapshotInfo takeSnapshot(SnapshotInfo 
snapshotInfo) {
 performSnapshotAndCopyOnHostSide(volumeInfo, snapshotInfo);
 }
 
-markAsBackedUp((SnapshotObject)result.getSnashot());
+snapshotOnPrimary = result.getSnapshot();
+backedUpSnapshot = backupSnapshot(snapshotOnPrimary);
+
+updateLocationTypeInDb(backedUpSnapshot);
+
 }
 finally {
 if (result != null && result.isSuccess()) {
 volumeInfo.stateTransit(Volume.Event.OperationSucceeded);
-}
-else {
+
+Preconditions.checkState(snapshotOnPrimary != null);
--- End diff --

Should an exception be thrown in this ``finally`` block or just skip 
processing?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1600#discussion_r77080424
  
--- Diff: 
engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java
 ---
@@ -92,7 +90,34 @@
 
 @Override
 public SnapshotInfo backupSnapshot(SnapshotInfo snapshotInfo) {
-return snapshotInfo;
+
+if 
(!snapshotInfo.getLocationType().equals(Snapshot.LocationType.SECONDARY)) {
+
+markAsBackedUp((SnapshotObject)snapshotInfo);
+return snapshotInfo;
+}
+
+// At this point the snapshot is either taken as a native
+// snapshot on the storage or exists as a volume on the storage 
(clone).
+// If archive flag is passed, we will copy this snapshot to 
secondary
+// storage and delete it from the primary storage.
+
+
Preconditions.checkState(Snapshot.LocationType.SECONDARY.equals(snapshotInfo.getLocationType()));
--- End diff --

Why is the check necessary given the check on Line 94?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1600#discussion_r77080328
  
--- Diff: 
engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java
 ---
@@ -92,7 +90,34 @@
 
 @Override
 public SnapshotInfo backupSnapshot(SnapshotInfo snapshotInfo) {
-return snapshotInfo;
+
+if 
(!snapshotInfo.getLocationType().equals(Snapshot.LocationType.SECONDARY)) {
--- End diff --

Since Enum values are guaranteed to be singletons, the `==` is that same as 
`equals`.  By changing this check to be ``snapshotInfo.getLocationType() != 
Snapshot.LocationType.SECONDARY``, a potential NPE is avoided and the code is 
more straightforward.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1600#discussion_r77079861
  
--- Diff: 
engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java
 ---
@@ -92,7 +90,34 @@
 
 @Override
 public SnapshotInfo backupSnapshot(SnapshotInfo snapshotInfo) {
-return snapshotInfo;
+
--- End diff --

Please add a ``Preconditions.checkArgument(snapshotInfo != null, 
"")`` to protect against potential NPEs.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1600#discussion_r77079527
  
--- Diff: 
engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java
 ---
@@ -92,7 +90,34 @@
 
 @Override
 public SnapshotInfo backupSnapshot(SnapshotInfo snapshotInfo) {
-return snapshotInfo;
+
+if 
(!snapshotInfo.getLocationType().equals(Snapshot.LocationType.SECONDARY)) {
+
+markAsBackedUp((SnapshotObject)snapshotInfo);
+return snapshotInfo;
+}
+
+// At this point the snapshot is either taken as a native
+// snapshot on the storage or exists as a volume on the storage 
(clone).
+// If archive flag is passed, we will copy this snapshot to 
secondary
+// storage and delete it from the primary storage.
+
+
Preconditions.checkState(Snapshot.LocationType.SECONDARY.equals(snapshotInfo.getLocationType()));
+
+HostVO host = getHost(snapshotInfo.getVolumeId());
+boolean canStorageSystemCreateVolumeFromSnapshot = 
canStorageSystemCreateVolumeFromSnapshot(snapshotInfo.getBaseVolume().getPoolId());
+boolean computeClusterSupportsResign = 
clusterDao.getSupportsResigning(host.getClusterId());
+
+if (!canStorageSystemCreateVolumeFromSnapshot || 
!computeClusterSupportsResign) {
+String mesg = "Cannot archive snapshot: Either 
canStorageSystemCreateVolumeFromSnapshot or " +
+"computeClusterSupportsResign were false.  ";
+
+s_logger.warn(mesg);
--- End diff --

Why is the message being logged to ``WARN`` and not ``ERROR``?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1600#discussion_r77079000
  
--- Diff: 
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
 ---
@@ -380,6 +513,122 @@ private void 
handleCreateTemplateFromSnapshot(SnapshotInfo snapshotInfo, Templat
 }
 
 /**
+ * Creates a volume on the storage from a snapshot that resides on the 
secondary storage (archived snapshot).
+ * @param snapshotInfo snapshot on secondary
+ * @param volumeInfo volume to be created on the storage
+ * @param callback  for async
+ */
+private void 
handleCreateVolumeFromSnapshotOnSecondaryStorage(SnapshotInfo snapshotInfo, 
VolumeInfo volumeInfo, AsyncCompletionCallback callback) {
+
+// at this point, the snapshotInfo and volumeInfo should have the 
same disk offering ID (so either one should be OK to get a DiskOfferingVO 
instance)
+DiskOfferingVO diskOffering = 
_diskOfferingDao.findByIdIncludingRemoved(volumeInfo.getDiskOfferingId());
+SnapshotVO snapshot = _snapshotDao.findById(snapshotInfo.getId());
+DataStore destDataStore = volumeInfo.getDataStore();
+
+// update the volume's hv_ss_reserve (hypervisor snapshot reserve) 
from a disk offering (used for managed storage)
+
_volumeService.updateHypervisorSnapshotReserveForVolume(diskOffering, 
volumeInfo.getId(), snapshot.getHypervisorType());
+
+
+CopyCmdAnswer copyCmdAnswer = null;
+String errMsg = null;
+
+HostVO hostVO = null;
+try {
+
+//create a volume on the storage
+AsyncCallFuture future = 
_volumeService.createVolumeAsync(volumeInfo, volumeInfo.getDataStore());
+VolumeApiResult result = future.get();
+
+if (result.isFailed()) {
+LOGGER.warn("Failed to create a volume: " + 
result.getResult());
+throw new CloudRuntimeException(result.getResult());
+}
+
+volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(), 
volumeInfo.getDataStore());
+
+volumeInfo.processEvent(Event.MigrationRequested);
+
+volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(), 
volumeInfo.getDataStore());
+
+hostVO = getHost(snapshotInfo.getDataCenterId(), false);
+
+//copy the volume from secondary via the hypervisor
+copyCmdAnswer = performCopyOfVdi(volumeInfo, snapshotInfo, 
hostVO);
+
+if (copyCmdAnswer == null || !copyCmdAnswer.getResult()) {
+if (copyCmdAnswer != null && 
!StringUtils.isEmpty(copyCmdAnswer.getDetails())) {
+errMsg = copyCmdAnswer.getDetails();
+}
+else {
+errMsg = "Unable to create volume from snapshot";
+}
+}
+}
+catch (Exception ex) {
+errMsg = ex.getMessage() != null ? ex.getMessage() : "Copy 
operation failed in 
'StorageSystemDataMotionStrategy.handleCreateVolumeFromSnapshotBothOnStorageSystem'";
+} finally {
+
+// detach and remove access after the volume is created
+DiskTO disk = new DiskTO(volumeInfo.getTO(), 
volumeInfo.getDeviceId(), volumeInfo.getPath(), volumeInfo.getVolumeType());
+DettachCommand cmd = new DettachCommand(disk, null);
+long storagePoolId = volumeInfo.getPoolId();
+StoragePoolVO storagePool = 
_storagePoolDao.findById(storagePoolId);
+
+cmd.setManaged(true);
+cmd.setStorageHost(storagePool.getHostAddress());
+cmd.setStoragePort(storagePool.getPort());
+cmd.set_iScsiName(volumeInfo.get_iScsiName());
+
+try {
+DettachAnswer dettachAnswer = (DettachAnswer) 
_agentMgr.send(hostVO.getId(), cmd);
+
+if (!dettachAnswer.getResult()) {
+throw new CloudRuntimeException("Error detaching 
volume:" + dettachAnswer.getDetails());
+}
+
+} catch (Exception e) {
+LOGGER.warn("Error detaching volume " + volumeInfo.getId() 
+ " Error: " + e.getMessage());
+}
+
+_volumeService.revokeAccess(volumeInfo, hostVO, destDataStore);
+}
+
+CopyCommandResult result = new CopyCommandResult(null, 
copyCmdAnswer);
+
+result.setResult(errMsg);
+
+callback.complete(result);
+
+}
+
+private void performCleanupCacheStorage(DataObject destOnStore) {
+

[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1600#discussion_r77078902
  
--- Diff: 
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
 ---
@@ -380,6 +513,122 @@ private void 
handleCreateTemplateFromSnapshot(SnapshotInfo snapshotInfo, Templat
 }
 
 /**
+ * Creates a volume on the storage from a snapshot that resides on the 
secondary storage (archived snapshot).
+ * @param snapshotInfo snapshot on secondary
+ * @param volumeInfo volume to be created on the storage
+ * @param callback  for async
+ */
+private void 
handleCreateVolumeFromSnapshotOnSecondaryStorage(SnapshotInfo snapshotInfo, 
VolumeInfo volumeInfo, AsyncCompletionCallback callback) {
+
+// at this point, the snapshotInfo and volumeInfo should have the 
same disk offering ID (so either one should be OK to get a DiskOfferingVO 
instance)
+DiskOfferingVO diskOffering = 
_diskOfferingDao.findByIdIncludingRemoved(volumeInfo.getDiskOfferingId());
+SnapshotVO snapshot = _snapshotDao.findById(snapshotInfo.getId());
+DataStore destDataStore = volumeInfo.getDataStore();
+
+// update the volume's hv_ss_reserve (hypervisor snapshot reserve) 
from a disk offering (used for managed storage)
+
_volumeService.updateHypervisorSnapshotReserveForVolume(diskOffering, 
volumeInfo.getId(), snapshot.getHypervisorType());
+
+
+CopyCmdAnswer copyCmdAnswer = null;
+String errMsg = null;
+
+HostVO hostVO = null;
+try {
+
+//create a volume on the storage
+AsyncCallFuture future = 
_volumeService.createVolumeAsync(volumeInfo, volumeInfo.getDataStore());
+VolumeApiResult result = future.get();
+
+if (result.isFailed()) {
+LOGGER.warn("Failed to create a volume: " + 
result.getResult());
+throw new CloudRuntimeException(result.getResult());
+}
+
+volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(), 
volumeInfo.getDataStore());
+
+volumeInfo.processEvent(Event.MigrationRequested);
+
+volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(), 
volumeInfo.getDataStore());
+
+hostVO = getHost(snapshotInfo.getDataCenterId(), false);
+
+//copy the volume from secondary via the hypervisor
+copyCmdAnswer = performCopyOfVdi(volumeInfo, snapshotInfo, 
hostVO);
+
+if (copyCmdAnswer == null || !copyCmdAnswer.getResult()) {
+if (copyCmdAnswer != null && 
!StringUtils.isEmpty(copyCmdAnswer.getDetails())) {
+errMsg = copyCmdAnswer.getDetails();
+}
+else {
+errMsg = "Unable to create volume from snapshot";
+}
+}
+}
+catch (Exception ex) {
+errMsg = ex.getMessage() != null ? ex.getMessage() : "Copy 
operation failed in 
'StorageSystemDataMotionStrategy.handleCreateVolumeFromSnapshotBothOnStorageSystem'";
+} finally {
+
+// detach and remove access after the volume is created
+DiskTO disk = new DiskTO(volumeInfo.getTO(), 
volumeInfo.getDeviceId(), volumeInfo.getPath(), volumeInfo.getVolumeType());
+DettachCommand cmd = new DettachCommand(disk, null);
+long storagePoolId = volumeInfo.getPoolId();
+StoragePoolVO storagePool = 
_storagePoolDao.findById(storagePoolId);
+
+cmd.setManaged(true);
+cmd.setStorageHost(storagePool.getHostAddress());
+cmd.setStoragePort(storagePool.getPort());
+cmd.set_iScsiName(volumeInfo.get_iScsiName());
+
+try {
+DettachAnswer dettachAnswer = (DettachAnswer) 
_agentMgr.send(hostVO.getId(), cmd);
+
+if (!dettachAnswer.getResult()) {
+throw new CloudRuntimeException("Error detaching 
volume:" + dettachAnswer.getDetails());
+}
+
+} catch (Exception e) {
+LOGGER.warn("Error detaching volume " + volumeInfo.getId() 
+ " Error: " + e.getMessage());
+}
+
+_volumeService.revokeAccess(volumeInfo, hostVO, destDataStore);
--- End diff --

Lines 571-593 appear to duplicate Line 463-475 in 
``StorageSystemDataMotionStrategy``.  Please consider consolidation of the 
commonalities.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled 

[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1600#discussion_r77077921
  
--- Diff: 
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
 ---
@@ -380,6 +513,122 @@ private void 
handleCreateTemplateFromSnapshot(SnapshotInfo snapshotInfo, Templat
 }
 
 /**
+ * Creates a volume on the storage from a snapshot that resides on the 
secondary storage (archived snapshot).
+ * @param snapshotInfo snapshot on secondary
+ * @param volumeInfo volume to be created on the storage
+ * @param callback  for async
+ */
+private void 
handleCreateVolumeFromSnapshotOnSecondaryStorage(SnapshotInfo snapshotInfo, 
VolumeInfo volumeInfo, AsyncCompletionCallback callback) {
+
+// at this point, the snapshotInfo and volumeInfo should have the 
same disk offering ID (so either one should be OK to get a DiskOfferingVO 
instance)
+DiskOfferingVO diskOffering = 
_diskOfferingDao.findByIdIncludingRemoved(volumeInfo.getDiskOfferingId());
+SnapshotVO snapshot = _snapshotDao.findById(snapshotInfo.getId());
+DataStore destDataStore = volumeInfo.getDataStore();
+
+// update the volume's hv_ss_reserve (hypervisor snapshot reserve) 
from a disk offering (used for managed storage)
+
_volumeService.updateHypervisorSnapshotReserveForVolume(diskOffering, 
volumeInfo.getId(), snapshot.getHypervisorType());
+
+
+CopyCmdAnswer copyCmdAnswer = null;
+String errMsg = null;
+
+HostVO hostVO = null;
+try {
+
+//create a volume on the storage
+AsyncCallFuture future = 
_volumeService.createVolumeAsync(volumeInfo, volumeInfo.getDataStore());
+VolumeApiResult result = future.get();
+
+if (result.isFailed()) {
+LOGGER.warn("Failed to create a volume: " + 
result.getResult());
--- End diff --

Why is this message being logged to ``WARN`` and not ``ERROR``?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1600#discussion_r77077963
  
--- Diff: 
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
 ---
@@ -380,6 +513,122 @@ private void 
handleCreateTemplateFromSnapshot(SnapshotInfo snapshotInfo, Templat
 }
 
 /**
+ * Creates a volume on the storage from a snapshot that resides on the 
secondary storage (archived snapshot).
+ * @param snapshotInfo snapshot on secondary
+ * @param volumeInfo volume to be created on the storage
+ * @param callback  for async
+ */
+private void 
handleCreateVolumeFromSnapshotOnSecondaryStorage(SnapshotInfo snapshotInfo, 
VolumeInfo volumeInfo, AsyncCompletionCallback callback) {
+
+// at this point, the snapshotInfo and volumeInfo should have the 
same disk offering ID (so either one should be OK to get a DiskOfferingVO 
instance)
+DiskOfferingVO diskOffering = 
_diskOfferingDao.findByIdIncludingRemoved(volumeInfo.getDiskOfferingId());
+SnapshotVO snapshot = _snapshotDao.findById(snapshotInfo.getId());
+DataStore destDataStore = volumeInfo.getDataStore();
+
+// update the volume's hv_ss_reserve (hypervisor snapshot reserve) 
from a disk offering (used for managed storage)
+
_volumeService.updateHypervisorSnapshotReserveForVolume(diskOffering, 
volumeInfo.getId(), snapshot.getHypervisorType());
+
+
+CopyCmdAnswer copyCmdAnswer = null;
+String errMsg = null;
+
+HostVO hostVO = null;
+try {
+
+//create a volume on the storage
+AsyncCallFuture future = 
_volumeService.createVolumeAsync(volumeInfo, volumeInfo.getDataStore());
+VolumeApiResult result = future.get();
--- End diff --

Should this call be bounded by a timeout?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1600#discussion_r77074337
  
--- Diff: core/src/org/apache/cloudstack/storage/to/PrimaryDataStoreTO.java 
---
@@ -51,6 +50,7 @@
 private final String url;
 private Map details;
 private static final String pathSeparator = "/";
+private boolean isManaged;
--- End diff --

Since no mutator is exposed and the value gets set in the constructor, 
considering making it ``final``.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1600#discussion_r77074073
  
--- Diff: 
api/test/org/apache/cloudstack/api/command/test/CreateSnapshotCmdTest.java ---
@@ -0,0 +1,130 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.test;
+
+import com.cloud.storage.Snapshot;
+import com.cloud.storage.VolumeApiService;
+import com.cloud.user.Account;
+import com.cloud.user.AccountService;
+import junit.framework.TestCase;
+import org.apache.cloudstack.api.ResponseGenerator;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.command.user.snapshot.CreateSnapshotCmd;
+import org.apache.cloudstack.api.response.SnapshotResponse;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Ignore;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.mockito.Mockito;
+
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyBoolean;
+import static org.mockito.Matchers.anyLong;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Matchers.isNull;
+
+public class CreateSnapshotCmdTest extends TestCase {
+
+private CreateSnapshotCmd createSnapshotCmd;
+private ResponseGenerator responseGenerator;
+
+@Rule
+public ExpectedException expectedException = ExpectedException.none();
+
+@Override
+@Before
+public void setUp() {
+
+createSnapshotCmd = new CreateSnapshotCmd() {
+
+@Override
+public String getCommandName() {
+return "createsnapshotresponse";
+}
+
+@Override
+public Long getVolumeId(){
+return 1L;
+}
+
+@Override
+public long getEntityOwnerId(){
+return 1L;
+}
+};
+
+}
+
+@Test
+public void testCreateSuccess() {
+
+AccountService accountService = Mockito.mock(AccountService.class);
+Account account = Mockito.mock(Account.class);
+
Mockito.when(accountService.getAccount(anyLong())).thenReturn(account);
+
+VolumeApiService volumeApiService = 
Mockito.mock(VolumeApiService.class);
+Snapshot snapshot = Mockito.mock(Snapshot.class);
+try {
+
+Mockito.when(volumeApiService.takeSnapshot(anyLong(), 
anyLong(), anyLong(),
+any(Account.class), anyBoolean(), 
isNull(Snapshot.LocationType.class))).thenReturn(snapshot);
+
+} catch (Exception e) {
+Assert.fail("Received exception when success expected " + 
e.getMessage());
+}
+
+responseGenerator = Mockito.mock(ResponseGenerator.class);
+SnapshotResponse snapshotResponse = 
Mockito.mock(SnapshotResponse.class);
+
Mockito.when(responseGenerator.createSnapshotResponse(snapshot)).thenReturn(snapshotResponse);
+
Mockito.doNothing().when(snapshotResponse).setAccountName(anyString());
+
+createSnapshotCmd._accountService = accountService;
+createSnapshotCmd._responseGenerator = responseGenerator;
+createSnapshotCmd._volumeService = volumeApiService;
+
+createSnapshotCmd.execute();
+}
+
+@Test
+@Ignore
--- End diff --

Why are we adding a new unit test that is being ignored?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1600#discussion_r77073945
  
--- Diff: 
api/test/org/apache/cloudstack/api/command/test/CreateSnapshotCmdTest.java ---
@@ -0,0 +1,130 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.test;
+
+import com.cloud.storage.Snapshot;
+import com.cloud.storage.VolumeApiService;
+import com.cloud.user.Account;
+import com.cloud.user.AccountService;
+import junit.framework.TestCase;
+import org.apache.cloudstack.api.ResponseGenerator;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.command.user.snapshot.CreateSnapshotCmd;
+import org.apache.cloudstack.api.response.SnapshotResponse;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Ignore;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.mockito.Mockito;
+
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyBoolean;
+import static org.mockito.Matchers.anyLong;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Matchers.isNull;
+
+public class CreateSnapshotCmdTest extends TestCase {
+
+private CreateSnapshotCmd createSnapshotCmd;
+private ResponseGenerator responseGenerator;
+
+@Rule
+public ExpectedException expectedException = ExpectedException.none();
+
+@Override
+@Before
+public void setUp() {
+
+createSnapshotCmd = new CreateSnapshotCmd() {
+
+@Override
+public String getCommandName() {
+return "createsnapshotresponse";
+}
+
+@Override
+public Long getVolumeId(){
+return 1L;
+}
+
+@Override
+public long getEntityOwnerId(){
+return 1L;
+}
+};
+
+}
+
+@Test
+public void testCreateSuccess() {
+
+AccountService accountService = Mockito.mock(AccountService.class);
+Account account = Mockito.mock(Account.class);
+
Mockito.when(accountService.getAccount(anyLong())).thenReturn(account);
+
+VolumeApiService volumeApiService = 
Mockito.mock(VolumeApiService.class);
+Snapshot snapshot = Mockito.mock(Snapshot.class);
+try {
+
+Mockito.when(volumeApiService.takeSnapshot(anyLong(), 
anyLong(), anyLong(),
+any(Account.class), anyBoolean(), 
isNull(Snapshot.LocationType.class))).thenReturn(snapshot);
+
+} catch (Exception e) {
+Assert.fail("Received exception when success expected " + 
e.getMessage());
+}
+
+responseGenerator = Mockito.mock(ResponseGenerator.class);
+SnapshotResponse snapshotResponse = 
Mockito.mock(SnapshotResponse.class);
+
Mockito.when(responseGenerator.createSnapshotResponse(snapshot)).thenReturn(snapshotResponse);
+
Mockito.doNothing().when(snapshotResponse).setAccountName(anyString());
+
+createSnapshotCmd._accountService = accountService;
+createSnapshotCmd._responseGenerator = responseGenerator;
+createSnapshotCmd._volumeService = volumeApiService;
+
+createSnapshotCmd.execute();
--- End diff --

Why aren't there any assertions in this test method?  Seems like it should 
at least validate the response returned by ``createSnapshotCmd.execute()``.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...

[jburwell]

Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1600#discussion_r77073521
  
--- Diff: api/src/org/apache/cloudstack/api/response/SnapshotResponse.java 
---
@@ -82,6 +82,10 @@
 @Param(description = "valid types are hourly, daily, weekly, monthy, 
template, and none.")
 private String intervalType;
 
+@SerializedName(ApiConstants.LOCATION_TYPE)
+@Param(description = "valid location types are primary and archive.")
+private String locationType;
--- End diff --

+1 @karuturi -- types should be symmetric between request and response.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1624: Fixes regarding VOLUME_DELETE events resulting from ...

[jburwell]

Github user jburwell commented on the issue:

https://github.com/apache/cloudstack/pull/1624
  
LGTM for code review.  Do we have someone testing for the second LGTM?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1624: Fixes regarding VOLUME_DELETE events resulting from ...

[jburwell]

Github user jburwell commented on the issue:

https://github.com/apache/cloudstack/pull/1624
  
@ProjectMoon we have a code review LGTM on #1654, and have someone lined up 
to test it and provide the second LGTM.  We should have the second LGTM by 1 
Sept 2016 at which time you can re-kick this PR.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1654: Updating pom.xml version numbers for release 4.8.2.0...

[jburwell]

Github user jburwell commented on the issue:

https://github.com/apache/cloudstack/pull/1654
  
@syed per our conversation on Slack, the following is the test procedure to 
verify this PR:

1. Create a fresh install 4.8.1.  Since we are verifying the database 
upgrade, the install only needs to start the Management Server.
2. Build this PR and deploy it against the 4.8.1 database
3. Restart the Management Server will trigger a database upgrade

Following database upgrade operation, the database version in the database 
should be 4.8.2.0.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: CS 4.9 NIO Selector wait time PR-1601

[Rohit Yadav]

Hi Martin,


Looks like something went wrong with the CloudStack generated public/private 
keys. You should not be prompted for any password when trying to ssh to 
systemvms on port 3922. Can you try to destroy the VRs or create a new network 
which would create a new VR.


Regarding the /bin/bash not found issue, that looks fishy. /bin/bash should be 
available on both Ubuntu 14.04 based KVM hosts and also on systemvms (which are 
Debian7 based).


Regards.


From: martin kolly 
Sent: 29 August 2016 20:10:32
To: dev@cloudstack.apache.org
Subject: Re: CS 4.9 NIO Selector wait time PR-1601


We have done more investigations. An error happens when the script
/usr/share/cloudstack-common/scripts/network/domr/router_proxy.sh is
executed:

MGMT LOGS:
2016-08-29 16:00:11,342 DEBUG [c.c.a.t.Request]
(AgentManager-Handler-12:null) (logid:) Seq 8-1837187172990452475:
Processing:  { Ans: , MgmtId: 90520741415395, via: 8, Ver: v1, Flags:
10,
[{"com.cloud.agent.api.GetRouterAlertsAnswer":{"result":false,"details":"/usr/share/cloudstack-common/scripts/network/domr/router_proxy.sh:
1: /usr/share/cloudstack-common/scripts/network/domr/router_proxy.sh:
h!/bin/bash: not found","wait":0}}] }
2016-08-29 16:00:11,345 DEBUG [c.c.a.t.Request]
(RouterStatusMonitor-1:ctx-9cdc1e33) (logid:5f04bf82) Seq
8-1837187172990452475: Received:  { Ans: , MgmtId: 90520741415395, via:
8(kvm702), Ver: v1, Flags: 10, { GetRouterAlertsAnswer } }
*2016-08-29 16:00:11,345 DEBUG [c.c.a.m.AgentManagerImpl]
(RouterStatusMonitor-1:ctx-9cdc1e33) (logid:5f04bf82) Details from
executing class com.cloud.agent.api.routing.GetRouterAlertsCommand:
/usr/share/cloudstack-common/scripts/network/domr/router_proxy.sh: 1:
/usr/share/cloudstack-common/scripts/network/domr/router_proxy.sh:
h!/bin/bash: not found*
2016-08-29 16:00:11,345 WARN
[c.c.n.r.VirtualNetworkApplianceManagerImpl]
(RouterStatusMonitor-1:ctx-9cdc1e33) (logid:5f04bf82) Unable to get
alerts from router r-221-VM
/usr/share/cloudstack-common/scripts/network/domr/router_proxy.sh: 1:
/usr/share/cloudstack-common/scripts/network/domr/router_proxy.sh:
h!/bin/bash: not found
2016-08-29 16:00:11,411 INFO  [o.a.c.e.o.NetworkOrchestrator]
(Network-Scavenger-1:ctx-64537bea) (logid:0c5d8104)
NetworkGarbageCollector uses '600' seconds for GC interval.
2016-08-29 16:00:11,416 DEBUG [o.a.c.e.o.NetworkOrchestrator]
(Network-Scavenger-1:ctx-64537bea) (logid:0c5d8104) We found network 219
to be free for the first time.  Adding it to the list: 1472479211
2016-08-29 16:00:12,392 DEBUG [c.c.s.StatsCollector]
(StatsCollector-6:ctx-c1aeb528) (logid:a364cc05) StorageCollector is
running...
2016-08-29 16:00:12,403 DEBUG [c.c.h.o.r.Ovm3HypervisorGuru]
(StatsCollector-6:ctx-c1aeb528) (logid:a364cc05)
getCommandHostDelegation: class com.cloud.agent.api.GetStorageStatsCommand

For debugging we modified the "router_proxy.sh" slightly on the KVM Host:

kvm# grep -A 2 DEBUG
/usr/share/cloudstack-common/scripts/network/domr/router_proxy.sh -n
46:# DEBUG CS Upgrade
*47-echo "$(date):  ssh -p 3922 -q -o StrictHostKeyChecking=no -i $cert
root@$domRIp /opt/cloud/bin/$script $*" >> /tmp/router.txt*
48-ssh -p 3922 -q -o StrictHostKeyChecking=no -i $cert root@$domRIp
"/opt/cloud/bin/$script $*"

kvm# tail /tmp/router.txt
Mon Aug 29 16:10:11 CEST 2016:  ssh -p 3922 -q -o
StrictHostKeyChecking=no -i /root/.ssh/id_rsa.cloud root@169.254.2.67
/opt/cloud/bin/netusage.sh -g
Mon Aug 29 16:10:11 CEST 2016:  ssh -p 3922 -q -o
StrictHostKeyChecking=no -i /root/.ssh/id_rsa.cloud root@169.254.2.67
/opt/cloud/bin/netusage.sh -g

On the router with IP 169.254.2.67 we see the failed logins as well:

root@r-221-VM:~# tail -n 2 /var/log/auth.log
Aug 29 14:10:11 r-221-VM sshd[7074]: Connection closed by 169.254.0.1
[preauth]
Aug 29 14:10:11 r-221-VM sshd[7075]: Connection closed by 169.254.0.1
[preauth]

These commands did not succeed. If we execute the command manually we
get prompted for the key passphrase.

kvm# ssh -p 3922  -o StrictHostKeyChecking=no -i /root/.ssh/id_rsa.cloud
root@169.254.2.67
Enter passphrase for key '/root/.ssh/id_rsa.cloud':

can someone confirm that the rsa key is protected with a passhrase? this
looks suspicious to us...

regards
martin







On 08/29/2016 10:09 AM, martin kolly wrote:
> thanks Simon and Rohit for the valuable inputs! After applying the
> following procedure the ssl errors are gone and host state is UP.
>
> # service cloudstack-management stop
>
> mysql> delete from configuration  where name = "ssl.keystore" ;
> # mv /etc/cloudstack/management/cloudmanagementserver.keystore
> /etc/cloudstack/management/cloudmanagementserver.keystore.old
>
> # service cloudstack-management start
>
> # file /etc/cloudstack/management/cloudmanagementserver.keystore
> /etc/cloudstack/management/cloudmanagementserver.keystore: Java KeyStore
> # file /etc/cloudstack/management/cloudmanagementserver.keystore.old
> 

[GitHub] cloudstack issue #1667: CLOUDSTACK-9481: Convert MyISAM table to InnoDB for ...

[rhtyd]

Github user rhtyd commented on the issue:

https://github.com/apache/cloudstack/pull/1667
  
@abhinandanprateek thanks, can you close this one?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1578: CLOUDSTACK-9401 : Support for Internal DNS in...

[fmaximus]

Github user fmaximus commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1578#discussion_r76993494
  
--- Diff: 
engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
 ---
@@ -2645,6 +2669,18 @@ public DhcpServiceProvider 
getDhcpServiceProvider(final Network network) {
 }
 }
 
+@Override
+public DnsServiceProvider getDnsServiceProvider(final Network network) 
{
+final String DnsProvider = 
_ntwkSrvcDao.getProviderForServiceInNetwork(network.getId(), Service.Dns);
--- End diff --

Variables should start lowecase


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1578: CLOUDSTACK-9401 : Support for Internal DNS in...

[fmaximus]

Github user fmaximus commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1578#discussion_r76991083
  
--- Diff: 
plugins/network-elements/nuage-vsp/src/com/cloud/network/guru/NuageVspGuestNetworkGuru.java
 ---
@@ -242,6 +249,40 @@ public void reserve(NicProfile nic, Network network, 
VirtualMachineProfile vm, D
 throw new IllegalStateException("The broadcast URI path " 
+ network.getBroadcastUri() + " is empty or in an incorrect format.");
 }
 
+HostVO nuageVspHost = 
getNuageVspHost(network.getPhysicalNetworkId());
+VspNetwork vspNetwork = 
_nuageVspEntityBuilder.buildVspNetwork(network, false);
+
+// Set flags for dhcp options
+boolean networkHasDns = networkHasDns(network);
+Boolean defaultHasDns = null;
+// Determine if dhcp options of the other nics in the network 
need to be updated
+if (VirtualMachine.Type.DomainRouter.equals(vm.getType()) && 
!State.Implementing.equals(network.getState())) {
+// Update dhcp options if a VR is added when we are not 
initiating the network
+s_logger.debug(String.format("DomainRouter is added to an 
existing network: %s in state: %s", network.getName(), network.getState()));
+List dhcpOptions = Lists.newArrayList();
+for (NicVO userNic 
:_nicDao.listByNetworkId(network.getId())){
+if 
(!VirtualMachine.Type.DomainRouter.equals(userNic.getVmType())) {
+// Dhcp options for Domain router are handled later
+VMInstanceVO userVm  = 
_vmInstanceDao.findById(userNic.getInstanceId());
--- End diff --

It is not necessary to fetch the vm here, as only the id is used.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1578: CLOUDSTACK-9401 : Support for Internal DNS in...

[fmaximus]

Github user fmaximus commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1578#discussion_r76991270
  
--- Diff: 
plugins/network-elements/nuage-vsp/src/com/cloud/network/guru/NuageVspGuestNetworkGuru.java
 ---
@@ -403,4 +448,25 @@ private HostVO getNuageVspHost(long physicalNetworkId) 
{
 }
 return nuageVspHost;
 }
+
+private boolean networkHasDns(Network network) {
+
+if (network != null) {
+List dnsProvider = 
_ntwkOfferingSrvcDao.listProvidersForServiceForNetworkOffering(network.getNetworkOfferingId(),
 Network.Service.Dns);
+if (dnsProvider.contains("VirtualRouter") || 
dnsProvider.contains("VpcVirtualRouter")) {
+return true;
+}
+}
+return false;
+}
+
+
+private NetworkVO getDefaultNetwork (long vmId) {
+
+NicVO defaultNic = _nicDao.findDefaultNicForVM(vmId);
+if (defaultNic != null ) return 
_networkDao.findById(_nicDao.findById(defaultNic.getId()).getNetworkId());
--- End diff --

Not necessary to fetch the nic from DB twice.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1666: CLOUDSTACK-9480: Egress Firewall: Incorrect use of A...

[borisstoyanov]

Github user borisstoyanov commented on the issue:

https://github.com/apache/cloudstack/pull/1666
  
LGTM based on code review.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1644: Honors the snapshot.backup.rightafter configuration ...

[bvbharatk]

Github user bvbharatk commented on the issue:

https://github.com/apache/cloudstack/pull/1644
  
### ACS CI BVT Run
 **Sumarry:**
 Build Number 86
 Hypervisor xenserver
 NetworkType Advanced
 Passed=99
 Failed=4
 Skipped=3

_Link to logs Folder (search by build_no):_ 
https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0


**Failed tests:**
* test_scale_vm.py

 * ContextSuite context=TestScaleVm>:setup Failing since 6 runs

* test_reset_vm_on_reboot.py

 * ContextSuite context=TestResetVmOnReboot>:setup Failing since 5 runs

* test_network.py

 * test_delete_account Failed

* test_non_contigiousvlan.py

 * test_extendPhysicalNetworkVlan Failing since 8 runs


**Skipped tests:**
test_vm_nic_adapter_vmxnet3
test_static_role_account_acls
test_deploy_vgpu_enabled_vm

**Passed test suits:**
test_deploy_vm_with_userdata.py
test_affinity_groups_projects.py
test_portable_publicip.py
test_over_provisioning.py
test_global_settings.py
test_service_offerings.py
test_routers_iptables_default_policy.py
test_loadbalance.py
test_routers.py
test_snapshots.py
test_deploy_vms_with_varied_deploymentplanners.py
test_router_dns.py
test_login.py
test_deploy_vm_iso.py
test_list_ids_parameter.py
test_public_ip_range.py
test_multipleips_per_nic.py
test_regions.py
test_affinity_groups.py
test_network_acl.py
test_pvlan.py
test_volumes.py
test_ssvm.py
test_nic.py
test_deploy_vm_root_resize.py
test_resource_detail.py
test_secondary_storage.py
test_vm_life_cycle.py
test_disk_offerings.py


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1651: Marvin Tests: fix expected return string for success...

[borisstoyanov]

Github user borisstoyanov commented on the issue:

https://github.com/apache/cloudstack/pull/1651
  
LGTM.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1669: Make CloudStack JSP-free

[rhtyd]

Github user rhtyd commented on the issue:

https://github.com/apache/cloudstack/pull/1669
  
Thanks @milamberspace I've modified the build process to generate l10n 
translation JS (dictionary) files when client/UI is built. I think if we use 
json file, then also we will need some sort of transformation build step so we 
can consume from messages.properties without changing any of our workflow if we 
don't have a way to directly translate into the `.js` files.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1666: CLOUDSTACK-9480: Egress Firewall: Incorrect use of A...

[murali-reddy]

Github user murali-reddy commented on the issue:

https://github.com/apache/cloudstack/pull/1666
  
@abhinandanprateek @borisstoyanov review?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: Few tables in cloudstack schema use MyISAM engine

[Abhinandan Prateek]

Thanks Erik for correcting.

More precisely, this is in context of migration of mysql db to Galera cluster, 
where there is a limitation with Galera cluster that it supports only InnodDB 
for replication due to its transactional nature.


Regards,
-abhi



On 31/08/16, 12:44 PM, "Erik Weber"  wrote:

>Just nit-picking, but MyISAM allows replication as well.
>
>I'm +1 to the change.
>
>-- 
>Erik
>
>On Wed, Aug 31, 2016 at 5:37 AM, Abhinandan Prateek <
>abhinandan.prat...@shapeblue.com> wrote:
>
>> Thanks, Koushik.
>>
>> Have created a PR to make the engine type consistent for all tables.
>> InnoDB allows for replication, that is all the more reason that we change
>> the engine type of these few tables to InnoDB.
>>
>> Regards,
>> -abhi
>>
>> PR: https://github.com/apache/cloudstack/pull/1667
>>
>>
>> From: Koushik Das  koushik@accelerite.com>>
>> Date: Tuesday, 30 August 2016 at 5:25 PM
>> To: Abhinandan Prateek  abhinandan.prat...@shapeblue.com>>, "dev@cloudstack.apache.org> dev@cloudstack.apache.org>"  dev@cloudstack.apache.org>>
>> Cc: Rajani Karuturi >, Raja
>> Pullela >
>> Subject: Re: Few tables in cloudstack schema use MyISAM engine
>>
>> I cannot think of any. Most probably in all cases the engine was not
>> explicitly specified and the default got used. As per below default was
>> myisam before mysql 5.5.5.
>> https://dev.mysql.com/doc/refman/5.5/en/storage-engine-setting.html
>>
>> Thanks,
>> Koushik
>> From: Abhinandan Prateek  abhinandan.prat...@shapeblue.com>>
>> Date: Tuesday, 30 August 2016 at 4:23 PM
>> To: "dev@cloudstack.apache.org" <
>> dev@cloudstack.apache.org>
>> Cc: Rajani Karuturi >, Raja
>> Pullela >,
>> Koushik Das > >>
>> Subject: Few tables in cloudstack schema use MyISAM engine
>>
>> Hi,
>>
>>Was there any specific reason that some tables in cloudstack schema use
>> MyISAM engine.
>> I know of “ quota_account” that I created and put in the diff table type
>> unknowingly. I assume such is the case with other such tables –
>> load_balancer_cert_map, monitoring_services, nic_ip_alias, sslcerts.
>>
>> Filed a ticket here to get this fixed or reason for not fixing it captured.
>>
>> Regards,
>> -abhi
>>
>> abhinandan.prat...@shapeblue.com
>> www.shapeblue.com
>> @shapeblue
>>
>>
>>
>>
>> DISCLAIMER == This e-mail may contain privileged and confidential
>> information which is the property of Accelerite, a Persistent Systems
>> business. It is intended only for the use of the individual or entity to
>> which it is addressed. If you are not the intended recipient, you are not
>> authorized to read, retain, copy, print, distribute or use this message. If
>> you have received this communication in error, please notify the sender and
>> delete all copies of this message. Accelerite, a Persistent Systems
>> business does not accept any liability for virus infected mails.
>>
>> abhinandan.prat...@shapeblue.com
>> www.shapeblue.com
>> 53 Chandos Place, Covent Garden, London  WC2N 4HSUK
>> @shapeblue
>>
>>
>>
>>

abhinandan.prat...@shapeblue.com 
www.shapeblue.com
53 Chandos Place, Covent Garden, London  WC2N 4HSUK
@shapeblue
  
 

[GitHub] cloudstack issue #1669: Make CloudStack JSP-free

[milamberspace]

Github user milamberspace commented on the issue:

https://github.com/apache/cloudstack/pull/1669
  
@rhtyd Probably the best way will be to change the Transifex L10N resources 
files type from Unicode Properties to JSON Key/value

A sample are here:
https://www.transifex.com/ke4qqq/CloudStack_UI/47xmessagesjson/

We can create the 4.10 json l10n files and change the translation update 
process to get the json files instead of the properties.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: Few tables in cloudstack schema use MyISAM engine

[Erik Weber]

Just nit-picking, but MyISAM allows replication as well.

I'm +1 to the change.

-- 
Erik

On Wed, Aug 31, 2016 at 5:37 AM, Abhinandan Prateek <
abhinandan.prat...@shapeblue.com> wrote:

> Thanks, Koushik.
>
> Have created a PR to make the engine type consistent for all tables.
> InnoDB allows for replication, that is all the more reason that we change
> the engine type of these few tables to InnoDB.
>
> Regards,
> -abhi
>
> PR: https://github.com/apache/cloudstack/pull/1667
>
>
> From: Koushik Das >
> Date: Tuesday, 30 August 2016 at 5:25 PM
> To: Abhinandan Prateek >, "dev@cloudstack.apache.org dev@cloudstack.apache.org>" >
> Cc: Rajani Karuturi >, Raja
> Pullela >
> Subject: Re: Few tables in cloudstack schema use MyISAM engine
>
> I cannot think of any. Most probably in all cases the engine was not
> explicitly specified and the default got used. As per below default was
> myisam before mysql 5.5.5.
> https://dev.mysql.com/doc/refman/5.5/en/storage-engine-setting.html
>
> Thanks,
> Koushik
> From: Abhinandan Prateek >
> Date: Tuesday, 30 August 2016 at 4:23 PM
> To: "dev@cloudstack.apache.org" <
> dev@cloudstack.apache.org>
> Cc: Rajani Karuturi >, Raja
> Pullela >,
> Koushik Das  >>
> Subject: Few tables in cloudstack schema use MyISAM engine
>
> Hi,
>
>Was there any specific reason that some tables in cloudstack schema use
> MyISAM engine.
> I know of “ quota_account” that I created and put in the diff table type
> unknowingly. I assume such is the case with other such tables –
> load_balancer_cert_map, monitoring_services, nic_ip_alias, sslcerts.
>
> Filed a ticket here to get this fixed or reason for not fixing it captured.
>
> Regards,
> -abhi
>
> abhinandan.prat...@shapeblue.com
> www.shapeblue.com
> @shapeblue
>
>
>
>
> DISCLAIMER == This e-mail may contain privileged and confidential
> information which is the property of Accelerite, a Persistent Systems
> business. It is intended only for the use of the individual or entity to
> which it is addressed. If you are not the intended recipient, you are not
> authorized to read, retain, copy, print, distribute or use this message. If
> you have received this communication in error, please notify the sender and
> delete all copies of this message. Accelerite, a Persistent Systems
> business does not accept any liability for virus infected mails.
>
> abhinandan.prat...@shapeblue.com
> www.shapeblue.com
> 53 Chandos Place, Covent Garden, London  WC2N 4HSUK
> @shapeblue
>
>
>
>