Mike Kolesnik has posted comments on this change.
Change subject: core: AddVmInterface Calls PlugUnplugCommand (#850854)
......................................................................
Patch Set 2: (4 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmInterfaceCommand.java
Line 69:
Line 70: TransactionSupport.executeInNewTransaction(new
TransactionMethod<Void>() {
Line 71: @Override
Line 72: public Void runInTransaction() {
Line 73: addInterfaceToDb(getParameters().getInterface());
Since you're saving the NIC as-is to the DB, I think you should save the active
field as false, since the plug command will take care of making sure it is
active.
Line 74: addInterfaceDeviceToDb();
Line 75: getCompensationContext().stateChanged();
Line 76: return null;
Line 77: }
Line 102: }
Line 103:
Line 104: private boolean plugNic() {
Line 105: PlugUnplugVmNicParameters plugParameters =
createPlugUnPlugParameters();
Line 106: plugParameters.setVmId(getParameters().getVmId());
Why not do this line in the create parameters command?
Line 107: VdcReturnValueBase plugVmNicReturnValue =
Line 108:
Backend.getInstance().runInternalAction(VdcActionType.PlugUnplugVmNic,
Line 109: plugParameters,
Line 110:
ExecutionHandler.createDefaultContexForTasks(getExecutionContext()));
Line 110:
ExecutionHandler.createDefaultContexForTasks(getExecutionContext()));
Line 111: if (!plugVmNicReturnValue.getSucceeded()) {
Line 112: getReturnValue().getExecuteFailedMessages().add("Failed
hot-plugging nic to VM");
Line 113:
getReturnValue().getCanDoActionMessages().addAll(plugVmNicReturnValue.getCanDoActionMessages());
Line 114:
getReturnValue().getCanDoActionMessages().remove(VdcBllMessages.VAR__ACTION__ADD);
FindBugs found you a bug here that the collection is of (generic) type String
but you're trying to remove an enum, so this will always not remove it.
Line 115: }
Line 116: return plugVmNicReturnValue.getSucceeded();
Line 117: }
Line 118:
Line 170
Line 171
Line 172
Line 173
Line 174
Since you don't seem to be calling canPerformHotPlug() after these changes, I
propose it would be moved (in a separate patch) back to PlugUnplug command
since it is the only one using it.
--
To view, visit http://gerrit.ovirt.org/7542
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc4636c8003ea3fc170a13be96a3efb9dc8cca24
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Muli Salem <[email protected]>
Gerrit-Reviewer: Mike Kolesnik <[email protected]>
Gerrit-Reviewer: Muli Salem <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
