Skip to content

Commit 142f07d

Browse files
committed
Merge release branch 4.7 to 4.8
* 4.7: CLOUDSTACK-9353: [XenServer] Fixed VM migration with storage Added ASF license to unit test file Added unit test to verify ordering Fixed ordering of network ACL rules being sent to the VR. The comparator was inverted
2 parents 744f9d5 + 8eedead commit 142f07d

3 files changed

Lines changed: 72 additions & 12 deletions

File tree

core/src/com/cloud/agent/api/routing/SetNetworkACLCommand.java

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,8 @@ public NetworkACLTO[] getRules() {
4545

4646
public String[][] generateFwRules() {
4747
final List<NetworkACLTO> aclList = Arrays.asList(rules);
48-
Collections.sort(aclList, new Comparator<NetworkACLTO>() {
49-
@Override
50-
public int compare(final NetworkACLTO acl1, final NetworkACLTO acl2) {
51-
return acl1.getNumber() < acl2.getNumber() ? 1 : -1;
52-
}
53-
});
48+
49+
orderNetworkAclRulesByRuleNumber(aclList);
5450

5551
final String[][] result = new String[2][aclList.size()];
5652
int i = 0;
@@ -97,6 +93,15 @@ public int compare(final NetworkACLTO acl1, final NetworkACLTO acl2) {
9793
return result;
9894
}
9995

96+
protected void orderNetworkAclRulesByRuleNumber(List<NetworkACLTO> aclList) {
97+
Collections.sort(aclList, new Comparator<NetworkACLTO>() {
98+
@Override
99+
public int compare(final NetworkACLTO acl1, final NetworkACLTO acl2) {
100+
return acl1.getNumber() > acl2.getNumber() ? 1 : -1;
101+
}
102+
});
103+
}
104+
100105
public NicTO getNic() {
101106
return nic;
102107
}
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
//
2+
// Licensed to the Apache Software Foundation (ASF) under one
3+
// or more contributor license agreements. See the NOTICE file
4+
// distributed with this work for additional information
5+
// regarding copyright ownership. The ASF licenses this file
6+
// to you under the Apache License, Version 2.0 (the
7+
// "License"); you may not use this file except in compliance
8+
// with the License. You may obtain a copy of the License at
9+
//
10+
// http://www.apache.org/licenses/LICENSE-2.0
11+
//
12+
// Unless required by applicable law or agreed to in writing,
13+
// software distributed under the License is distributed on an
14+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
// KIND, either express or implied. See the License for the
16+
// specific language governing permissions and limitations
17+
// under the License.
18+
//
19+
20+
package com.cloud.agent.api.routing;
21+
22+
import static org.junit.Assert.assertEquals;
23+
24+
import java.util.List;
25+
26+
import org.junit.Test;
27+
28+
import com.cloud.agent.api.to.NetworkACLTO;
29+
import com.google.common.collect.Lists;
30+
31+
public class SetNetworkACLCommandTest {
32+
33+
@Test
34+
public void testNetworkAclRuleOrdering(){
35+
36+
//given
37+
List<NetworkACLTO> aclList = Lists.newArrayList();
38+
39+
aclList.add(new NetworkACLTO(3, null, null, null, null, false, false, null, null, null, null, false, 3));
40+
aclList.add(new NetworkACLTO(1, null, null, null, null, false, false, null, null, null, null, false, 1));
41+
aclList.add(new NetworkACLTO(2, null, null, null, null, false, false, null, null, null, null, false, 2));
42+
43+
SetNetworkACLCommand cmd = new SetNetworkACLCommand(aclList, null);
44+
45+
//when
46+
cmd.orderNetworkAclRulesByRuleNumber(aclList);
47+
48+
//then
49+
for(int i=0; i< aclList.size();i++){
50+
assertEquals(aclList.get(i).getNumber(), i+1);
51+
}
52+
}
53+
}

plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/xen610/XenServer610MigrateWithStorageCommandWrapper.java

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.util.Map;
2525
import java.util.Set;
2626

27+
import com.cloud.utils.Pair;
2728
import org.apache.cloudstack.storage.to.VolumeObjectTO;
2829
import org.apache.log4j.Logger;
2930

@@ -60,7 +61,7 @@ public final class XenServer610MigrateWithStorageCommandWrapper extends CommandW
6061
public Answer execute(final MigrateWithStorageCommand command, final XenServer610Resource xenServer610Resource) {
6162
final Connection connection = xenServer610Resource.getConnection();
6263
final VirtualMachineTO vmSpec = command.getVirtualMachine();
63-
final Map<VolumeTO, StorageFilerTO> volumeToFiler = command.getVolumeToFiler();
64+
final List<Pair<VolumeTO, StorageFilerTO>> volumeToFiler = command.getVolumeToFilerAsList();
6465
final String vmName = vmSpec.getName();
6566
Task task = null;
6667

@@ -80,13 +81,14 @@ public Answer execute(final MigrateWithStorageCommand command, final XenServer61
8081
final XsLocalNetwork nativeNetworkForTraffic = xenServer610Resource.getNativeNetworkForTraffic(connection, TrafficType.Storage, null);
8182
final Network networkForSm = nativeNetworkForTraffic.getNetwork();
8283

83-
// Create the vif map. The vm stays in the same cluster so we have to pass an empty vif map.
84+
// Create the vif map. The vm stays in the same cluster so we have to pass an empty vif map.
8485
final Map<VIF, Network> vifMap = new HashMap<VIF, Network>();
8586
final Map<VDI, SR> vdiMap = new HashMap<VDI, SR>();
86-
for (final Map.Entry<VolumeTO, StorageFilerTO> entry : volumeToFiler.entrySet()) {
87-
final VolumeTO volume = entry.getKey();
88-
final StorageFilerTO sotrageFiler = entry.getValue();
89-
vdiMap.put(xenServer610Resource.getVDIbyUuid(connection, volume.getPath()), xenServer610Resource.getStorageRepository(connection, sotrageFiler.getUuid()));
87+
88+
for (final Pair<VolumeTO, StorageFilerTO> entry : volumeToFiler) {
89+
final StorageFilerTO storageFiler = entry.second();
90+
final VolumeTO volume = entry.first();
91+
vdiMap.put(xenServer610Resource.getVDIbyUuid(connection, volume.getPath()), xenServer610Resource.getStorageRepository(connection, storageFiler.getUuid()));
9092
}
9193

9294
// Get the vm to migrate.

0 commit comments

Comments
 (0)