Alona Kaplan has posted comments on this change.
Change subject: core: add NetworkLinking to UpdateVmInterfaceCommand
......................................................................
Patch Set 14: (4 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmInterfaceCommand.java
Line 45:
Line 46: private enum VdsAction {
Line 47: plug,
Line 48: unplug,
Line 49: unplugPlug,
It is not a valid action. It is blocked in canDoAction.
Line 50: update
Line 51: }
Line 52:
Line 53: public UpdateVmInterfaceCommand(T parameters) {
Line 52:
Line 53: public UpdateVmInterfaceCommand(T parameters) {
Line 54: super(parameters);
Line 55: setVmId(parameters.getVmId());
Line 56: if (getVm().getStatus() == VMStatus.Up) {
You are right. A canDoAction should be added for this case.
But also before my patch the Command would have NPE if the vmId is not valid.
Line 57: setVdsId(getVm().getRunOnVds().getValue());
Line 58: }
Line 59: }
Line 60:
Line 113: getVm(),
Line 114:
getVmNetworkInterfaceDao().get(getInterface().getId()),
Line 115: oldVmDevice));
Line 116: }
Line 117: }
You don't call updateHost if the requiredVdsAction is UnplugPlug.
Instead, you throw a conDoAction explaining to user he has to Unplug and then
Plug the vnic.
Line 118: }
Line 119: return true;
Line 120: }
Line 121:
Line 153: requiredVdsAction = VdsAction.unplug;
Line 154: }
Line 155: // plugUnplug or update
Line 156: else if (oldIface.isActive() &&
getInterface().isActive()) {
Line 157: if (isPropertiesRequireUnplugPlugWereUpdated()) {
I don't agree. You can't pull just this if. You"ll have to duplicate the whole
block.
Because first you have to check it is not plug or unplug.
This whole method is called only once, cause the return value is stored in
requiredVdsAction.
The method gathers all the possible options and I don't think it should be
changed.
Line 158: requiredVdsAction = VdsAction.unplugPlug;
Line 159: } else if
(isProperiesRequireVmUpdateDeviceWereUpdated()) {
Line 160: requiredVdsAction = VdsAction.update;
Line 161: }
--
To view, visit http://gerrit.ovirt.org/9582
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0ea914badc716f2908f45c020b53ced423ed23ec
Gerrit-PatchSet: 14
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alona Kaplan <alkap...@redhat.com>
Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com>
Gerrit-Reviewer: Mike Kolesnik <mkole...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
