Skip to content

Commit 39c270b

Browse files
hsato03hsato03
committed
Address reviews
1 parent 9c6d9a0 commit 39c270b

File tree

25 files changed

+90
-110
lines changed

25 files changed

+90
-110
lines changed

‎api/src/main/java/com/cloud/agent/api/to/NicTO.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package com.cloud.agent.api.to;
1818

1919
import com.cloud.offering.NetworkOffering;
20-
import com.cloud.vm.Nic;
2120

2221
import java.util.List;
2322
import java.util.Map;
@@ -34,7 +33,7 @@ public class NicTO extends NetworkTO {
3433
boolean dpdkEnabled;
3534
Integer mtu;
3635
Long networkId;
37-
Nic.LinkState linkState;
36+
boolean enabled;
3837

3938
String networkSegmentName;
4039

@@ -157,11 +156,11 @@ public void setNetworkSegmentName(String networkSegmentName) {
157156
this.networkSegmentName = networkSegmentName;
158157
}
159158

160-
public Nic.LinkState getLinkState() {
161-
return linkState;
159+
public boolean isEnabled() {
160+
return enabled;
162161
}
163162

164-
public void setLinkState(Nic.LinkState linkState) {
165-
this.linkState = linkState;
163+
public void setEnabled(boolean enabled) {
164+
this.enabled = enabled;
166165
}
167166
}

‎api/src/main/java/com/cloud/vm/Nic.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,6 @@ enum Event {
3737
ReservationRequested, ReleaseRequested, CancelRequested, OperationCompleted, OperationFailed,
3838
}
3939

40-
enum LinkState {
41-
Enabled, Disabled
42-
}
43-
4440
public enum State implements FiniteState<State, Event> {
4541
Allocated("Resource is allocated but not reserved"), Reserving("Resource is being reserved right now"), Reserved("Resource has been reserved."), Releasing(
4642
"Resource is being released"), Deallocating("Resource is being deallocated");
@@ -167,5 +163,5 @@ public enum ReservationStrategy {
167163

168164
Integer getMtu();
169165

170-
LinkState getLinkState();
166+
boolean isEnabled();
171167
}

‎api/src/main/java/com/cloud/vm/NicProfile.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public class NicProfile implements InternalIdentity, Serializable {
5252
boolean defaultNic;
5353
Integer networkRate;
5454
boolean isSecurityGroupEnabled;
55-
Nic.LinkState linkState;
55+
boolean enabled;
5656

5757
Integer orderIndex;
5858

@@ -88,7 +88,7 @@ public NicProfile(Nic nic, Network network, URI broadcastUri, URI isolationUri,
8888
broadcastType = network.getBroadcastDomainType();
8989
trafficType = network.getTrafficType();
9090
format = nic.getAddressFormat();
91-
linkState = nic.getLinkState();
91+
enabled = nic.isEnabled();
9292

9393
iPv4Address = nic.getIPv4Address();
9494
iPv4Netmask = nic.getIPv4Netmask();
@@ -416,12 +416,12 @@ public void setIpv4AllocationRaceCheck(boolean ipv4AllocationRaceCheck) {
416416
this.ipv4AllocationRaceCheck = ipv4AllocationRaceCheck;
417417
}
418418

419-
public Nic.LinkState getLinkState() {
420-
return linkState;
419+
public boolean isEnabled() {
420+
return enabled;
421421
}
422422

423-
public void setLinkState(Nic.LinkState linkState) {
424-
this.linkState = linkState;
423+
public void setEnabled(boolean enabled) {
424+
this.enabled = enabled;
425425
}
426426

427427
//

‎api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVmNicCmd.java‎

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,8 @@
1616
// under the License.
1717
package org.apache.cloudstack.api.command.user.vm;
1818

19-
import com.cloud.exception.InvalidParameterValueException;
20-
import com.cloud.vm.Nic;
21-
2219
import org.apache.cloudstack.acl.RoleType;
20+
import org.apache.cloudstack.api.ACL;
2321
import org.apache.cloudstack.api.APICommand;
2422
import org.apache.cloudstack.api.ApiConstants;
2523
import org.apache.cloudstack.api.ApiErrorCode;
@@ -30,7 +28,6 @@
3028
import org.apache.cloudstack.api.response.NicResponse;
3129
import org.apache.cloudstack.api.response.UserVmResponse;
3230
import org.apache.cloudstack.context.CallContext;
33-
import org.apache.commons.lang3.StringUtils;
3431

3532
import com.cloud.event.EventTypes;
3633
import com.cloud.user.Account;
@@ -43,26 +40,19 @@
4340
authorized = { RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User })
4441
public class UpdateVmNicCmd extends BaseAsyncCmd {
4542

43+
@ACL
4644
@Parameter(name = ApiConstants.NIC_ID, type = CommandType.UUID, entityType = NicResponse.class, required = true, description = "NIC ID")
4745
private Long nicId;
4846

49-
@Parameter(name = ApiConstants.STATE, type = CommandType.STRING, description = "Whether the NIC link state is enabled or disabled")
50-
private String linkState;
47+
@Parameter(name = ApiConstants.ENABLED, type = CommandType.BOOLEAN, description = "If true, sets the NIC state to UP; otherwise, sets the NIC state to DOWN")
48+
private Boolean enabled;
5149

5250
public Long getNicId() {
5351
return nicId;
5452
}
5553

56-
public Nic.LinkState getLinkState() {
57-
if (linkState == null) {
58-
return null;
59-
}
60-
61-
try {
62-
return Nic.LinkState.valueOf(StringUtils.capitalize(StringUtils.lowerCase(linkState)));
63-
} catch (IllegalArgumentException ex) {
64-
throw new InvalidParameterValueException(String.format("Unable to resolve link state [%s] to a supported value (Enabled or Disabled).", linkState));
65-
}
54+
public Boolean isEnabled() {
55+
return enabled;
6656
}
6757

6858
@Override
@@ -87,10 +77,13 @@ public long getEntityOwnerId() {
8777
@Override
8878
public void execute() {
8979
CallContext.current().setEventDetails(String.format("NIC ID: %s", getResourceUuid(ApiConstants.NIC_ID)));
80+
9081
UserVm result = _userVmService.updateVirtualMachineNic(this);
91-
ArrayList<ApiConstants.VMDetails> dc = new ArrayList<ApiConstants.VMDetails>();
82+
83+
ArrayList<ApiConstants.VMDetails> dc = new ArrayList<>();
9284
dc.add(ApiConstants.VMDetails.valueOf("nics"));
9385
EnumSet<ApiConstants.VMDetails> details = EnumSet.copyOf(dc);
86+
9487
if (result != null){
9588
UserVmResponse response = _responseGenerator.createUserVmResponse(ResponseObject.ResponseView.Restricted, "virtualmachine", details, result).get(0);
9689
response.setResponseName(getCommandName());

‎api/src/main/java/org/apache/cloudstack/api/response/NicResponse.java‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
import java.util.List;
2020

21-
import com.cloud.vm.Nic.LinkState;
2221
import org.apache.cloudstack.api.ApiConstants;
2322
import org.apache.cloudstack.api.BaseResponse;
2423
import org.apache.cloudstack.api.EntityReference;
@@ -147,9 +146,9 @@ public class NicResponse extends BaseResponse {
147146
@Param(description = "Public IP address associated with this NIC via Static NAT rule")
148147
private String publicIp;
149148

150-
@SerializedName(ApiConstants.STATE)
151-
@Param(description = "NIC's link state")
152-
private LinkState linkState;
149+
@SerializedName(ApiConstants.ENABLED)
150+
@Param(description = "whether the NIC is enabled or not")
151+
private Boolean isEnabled;
153152

154153
public void setVmId(String vmId) {
155154
this.vmId = vmId;
@@ -422,11 +421,11 @@ public void setPublicIp(String publicIp) {
422421
this.publicIp = publicIp;
423422
}
424423

425-
public LinkState getLinkState() {
426-
return linkState;
424+
public Boolean getEnabled() {
425+
return isEnabled;
427426
}
428427

429-
public void setLinkState(LinkState linkState) {
430-
this.linkState = linkState;
428+
public void setEnabled(Boolean enabled) {
429+
isEnabled = enabled;
431430
}
432431
}

‎core/src/main/java/com/cloud/agent/api/UpdateVmNicCommand.java‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,11 @@
1717
//
1818
package com.cloud.agent.api;
1919

20-
import com.cloud.vm.Nic;
21-
2220
public class UpdateVmNicCommand extends Command {
2321

2422
String nicMacAddress;
2523
String instanceName;
26-
Nic.LinkState linkState;
24+
Boolean enabled;
2725

2826
@Override
2927
public boolean executeInSequence() {
@@ -33,10 +31,10 @@ public boolean executeInSequence() {
3331
protected UpdateVmNicCommand() {
3432
}
3533

36-
public UpdateVmNicCommand(String nicMacAdderss, String instanceName, Nic.LinkState linkState) {
34+
public UpdateVmNicCommand(String nicMacAdderss, String instanceName, Boolean enabled) {
3735
this.nicMacAddress = nicMacAdderss;
3836
this.instanceName = instanceName;
39-
this.linkState = linkState;
37+
this.enabled = enabled;
4038
}
4139

4240
public String getNicMacAddress() {
@@ -47,7 +45,7 @@ public String getVmName() {
4745
return instanceName;
4846
}
4947

50-
public Nic.LinkState getLinkState() {
51-
return linkState;
48+
public Boolean isEnabled() {
49+
return enabled;
5250
}
5351
}

‎engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ NicProfile addVmToNetwork(VirtualMachine vm, Network network, NicProfile request
230230

231231
Boolean updateDefaultNicForVM(VirtualMachine vm, Nic nic, Nic defaultNic);
232232

233-
boolean updateVmNic(VirtualMachine vm, Nic nic, Nic.LinkState linkState) throws ResourceUnavailableException;
233+
boolean updateVmNic(VirtualMachine vm, Nic nic, Boolean enabled) throws ResourceUnavailableException;
234234

235235
/**
236236
* @param vm

‎engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6273,8 +6273,8 @@ private Pair<JobInfo.Status, String> orchestrateUpdateDefaultNic(final VmWorkUpd
62736273
}
62746274

62756275
@Override
6276-
public boolean updateVmNic(VirtualMachine vm, Nic nic, Nic.LinkState linkState) {
6277-
Outcome<VirtualMachine> outcome = updateVmNicThroughJobQueue(vm, nic, linkState);
6276+
public boolean updateVmNic(VirtualMachine vm, Nic nic, Boolean enabled) {
6277+
Outcome<VirtualMachine> outcome = updateVmNicThroughJobQueue(vm, nic, enabled);
62786278

62796279
retrieveVmFromJobOutcome(outcome, vm.getUuid(), "updateVmNic");
62806280

@@ -6289,10 +6289,10 @@ public boolean updateVmNic(VirtualMachine vm, Nic nic, Nic.LinkState linkState)
62896289
throw new CloudRuntimeException("Unexpected job execution result.");
62906290
}
62916291

6292-
private boolean orchestrateUpdateVmNic(final VirtualMachine vm, final Nic nic, final Nic.LinkState linkState) throws ResourceUnavailableException {
6292+
private boolean orchestrateUpdateVmNic(final VirtualMachine vm, final Nic nic, final Boolean enabled) throws ResourceUnavailableException {
62936293
if (vm.getState() == State.Running) {
62946294
try {
6295-
UpdateVmNicCommand updateVmNicCmd = new UpdateVmNicCommand(nic.getMacAddress(), vm.getName(), linkState);
6295+
UpdateVmNicCommand updateVmNicCmd = new UpdateVmNicCommand(nic.getMacAddress(), vm.getName(), enabled);
62966296
Commands cmds = new Commands(Command.OnError.Stop);
62976297
cmds.addCommand("updatevmnic", updateVmNicCmd);
62986298

@@ -6309,13 +6309,13 @@ private boolean orchestrateUpdateVmNic(final VirtualMachine vm, final Nic nic, f
63096309
}
63106310

63116311
NicVO nicVo = _nicsDao.findById(nic.getId());
6312-
nicVo.setLinkState(linkState);
6312+
nicVo.setEnabled(enabled);
63136313
_nicsDao.persist(nicVo);
63146314

63156315
return true;
63166316
}
63176317

6318-
public Outcome<VirtualMachine> updateVmNicThroughJobQueue(final VirtualMachine vm, final Nic nic, final Nic.LinkState linkState) {
6318+
public Outcome<VirtualMachine> updateVmNicThroughJobQueue(final VirtualMachine vm, final Nic nic, final Boolean isNicEnabled) {
63196319
Long vmId = vm.getId();
63206320
String commandName = VmWorkUpdateNic.class.getName();
63216321
Pair<VmWorkJobVO, Long> pendingWorkJob = retrievePendingWorkJob(vmId, commandName);
@@ -6326,7 +6326,7 @@ public Outcome<VirtualMachine> updateVmNicThroughJobQueue(final VirtualMachine v
63266326
Pair<VmWorkJobVO, VmWork> newVmWorkJobAndInfo = createWorkJobAndWorkInfo(commandName, vmId);
63276327

63286328
workJob = newVmWorkJobAndInfo.first();
6329-
VmWorkUpdateNic workInfo = new VmWorkUpdateNic(newVmWorkJobAndInfo.second(), nic.getId(), linkState);
6329+
VmWorkUpdateNic workInfo = new VmWorkUpdateNic(newVmWorkJobAndInfo.second(), nic.getId(), isNicEnabled);
63306330

63316331
setCmdInfoAndSubmitAsyncJob(workJob, workInfo, vmId);
63326332
}
@@ -6342,7 +6342,7 @@ private Pair<JobInfo.Status, String> orchestrateUpdateVmNic(final VmWorkUpdateNi
63426342
if (nic == null) {
63436343
throw new CloudRuntimeException(String.format("Unable to find NIC with ID %s.", work.getNicId()));
63446344
}
6345-
final boolean result = orchestrateUpdateVmNic(vm, nic, work.getLinkState());
6345+
final boolean result = orchestrateUpdateVmNic(vm, nic, work.isEnabled());
63466346
return new Pair<>(JobInfo.Status.SUCCEEDED, _jobMgr.marshallResultObject(result));
63476347
}
63486348

‎engine/orchestration/src/main/java/com/cloud/vm/VmWorkUpdateNic.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,19 @@ public class VmWorkUpdateNic extends VmWork {
2020
private static final long serialVersionUID = -8957066627929113278L;
2121

2222
Long nicId;
23-
Nic.LinkState linkState;
23+
Boolean enabled;
2424

25-
public VmWorkUpdateNic(VmWork vmWork, Long nicId, Nic.LinkState linkState) {
25+
public VmWorkUpdateNic(VmWork vmWork, Long nicId, Boolean enabled) {
2626
super(vmWork);
2727
this.nicId = nicId;
28-
this.linkState = linkState;
28+
this.enabled = enabled;
2929
}
3030

3131
public Long getNicId() {
3232
return nicId;
3333
}
3434

35-
public Nic.LinkState getLinkState() {
36-
return linkState;
35+
public Boolean isEnabled() {
36+
return enabled;
3737
}
3838
}

‎engine/schema/src/main/java/com/cloud/vm/NicVO.java‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,8 @@ protected NicVO() {
131131
@Column(name = "mtu")
132132
Integer mtu;
133133

134-
@Column(name = "link_state")
135-
LinkState linkState;
134+
@Column(name = "enabled")
135+
boolean enabled;
136136

137137
@Transient
138138
transient String nsxLogicalSwitchUuid;
@@ -146,7 +146,7 @@ public NicVO(String reserver, Long instanceId, long configurationId, VirtualMach
146146
this.networkId = configurationId;
147147
this.state = State.Allocated;
148148
this.vmType = vmType;
149-
this.linkState = LinkState.Enabled;
149+
this.enabled = true;
150150
}
151151

152152
@Override
@@ -401,13 +401,12 @@ public void setNsxLogicalSwitchPortUuid(String nsxLogicalSwitchPortUuid) {
401401
this.nsxLogicalSwitchPortUuid = nsxLogicalSwitchPortUuid;
402402
}
403403

404-
@Override
405-
public LinkState getLinkState() {
406-
return linkState;
404+
public boolean isEnabled() {
405+
return enabled;
407406
}
408407

409-
public void setLinkState(LinkState linkState) {
410-
this.linkState = linkState;
408+
public void setEnabled(boolean enabled) {
409+
this.enabled = enabled;
411410
}
412411

413412
@Override

0 commit comments

Comments
 (0)