Martin Mucha has uploaded a new change for review. Change subject: core: refactored util class from ReorderVmNicsCommand ......................................................................
core: refactored util class from ReorderVmNicsCommand • extracted util • refactored so this util class can be used via cdi. Change-Id: Id3353cc030aa42feb07648ee20f25da2891217e2 Signed-off-by: Martin Mucha <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateAndReorderVmNics.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmManagementCommandBase.java A backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/ReorderNics.java D backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/ReorderVmNicsCommand.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcActionType.java 8 files changed, 132 insertions(+), 116 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/32/40032/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java index 949a3b8..4105675 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java @@ -850,7 +850,11 @@ log.error("Failed to add vm . The reasons are: {}", StringUtils.join(errorMessages, ',')); } - new UpdateAndReorderVmNics(getParameters(), this).updateAndReorder(); + //TODO MM: previously was in such situation vm created with nics unsorted. Being part of vm creation seems that that vm will be always 'managed' thus this check can be removed (if I understand preexisting code properly). +// if (!canRunActionOnNonManagedVm()) { +// return false; +// } + updateAndReorderVmNics.updateAndReorder(getParameters(), this); } private void addGraphicsDevice() { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java index 94759c2..1c58d67 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java @@ -70,7 +70,7 @@ } checkTrustedService(); - new UpdateAndReorderVmNics(getParameters(), this).updateNics(); + updateAndReorderVmNics.updateNicsOnly(getParameters(), this); } private void checkTrustedService() { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateAndReorderVmNics.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateAndReorderVmNics.java index 70ae6cd..7eb6e4c 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateAndReorderVmNics.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateAndReorderVmNics.java @@ -9,13 +9,15 @@ import java.util.Map; import java.util.Set; +import javax.inject.Inject; + +import org.ovirt.engine.core.bll.network.vm.ReorderNics; import org.ovirt.engine.core.common.action.AddVmInterfaceParameters; import org.ovirt.engine.core.common.action.RemoveVmInterfaceParameters; import org.ovirt.engine.core.common.action.VdcActionParametersBase; import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.action.VdcReturnValueBase; import org.ovirt.engine.core.common.action.VmManagementParametersBase; -import org.ovirt.engine.core.common.action.VmOperationParameterBase; import org.ovirt.engine.core.common.businessentities.network.VmInterfaceType; import org.ovirt.engine.core.common.businessentities.network.VmNetworkInterface; import org.ovirt.engine.core.common.queries.IdQueryParameters; @@ -29,16 +31,33 @@ public class UpdateAndReorderVmNics { - private Logger log = LoggerFactory.getLogger(getClass()); + private final Logger log = LoggerFactory.getLogger(getClass()); + private final ReorderNics reorderNics; private int osType; private Version compatibilityVersion; private Iterable<VmNetworkInterface> updatedVmNetworkInterfaces; private Guid vmId; + private CommandBase<?> callerCommand; - private final CommandBase<?> callerCommand; + @Inject + public UpdateAndReorderVmNics(ReorderNics reorderNics) { + this.reorderNics = reorderNics; + } - public UpdateAndReorderVmNics(VmManagementParametersBase parameters, CommandBase<?> callerCommand) { + public void updateAndReorder(VmManagementParametersBase parameters, CommandBase<?> callerCommand) { + init(parameters, callerCommand); + + updateNics(); + reorderNics.updateNicsOrder(vmId); + } + + public void updateNicsOnly(VmManagementParametersBase parameters, CommandBase<?> callerCommand) { + init(parameters, callerCommand); + updateNics(); + } + + private void init(VmManagementParametersBase parameters, CommandBase<?> callerCommand) { this.osType = parameters.getVm().getStaticData().getOsId(); this.compatibilityVersion = parameters.getVm().getVdsGroupCompatibilityVersion(); this.updatedVmNetworkInterfaces = parameters.getUpdatedNetworkInterfaces(); @@ -46,17 +65,7 @@ this.callerCommand = callerCommand; } - public void updateAndReorder() { - updateNics(); - reorderNics(); - } - - public void reorderNics() { - VmOperationParameterBase reorderParams = new VmOperationParameterBase(vmId); - callerCommand.runInternalAction(VdcActionType.ReorderVmNics, reorderParams); - } - - public void updateNics() { + private void updateNics() { UpdateNicsVmInterfaceParametersFactory parametersFactory = getParametersFactory(); invokeActionType(VdcActionType.AddVmInterface, parametersFactory.createCreateVnicParameters()); invokeActionType(VdcActionType.UpdateVmInterface, parametersFactory.createUpdateVnicParameters()); @@ -104,7 +113,7 @@ } //TODO MM: duplicate with org.ovirt.engine.ui.uicommonweb.dataprovider.AsyncDataProvider.getDefaultNicType() - public VmInterfaceType getDefaultNicType(Collection<VmInterfaceType> items) { + private VmInterfaceType getDefaultNicType(Collection<VmInterfaceType> items) { if (items == null || items.isEmpty()) { return null; } else if (items.contains(VmInterfaceType.pv)) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java index ebc0f0d..401db97 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java @@ -174,7 +174,7 @@ } VmHandler.updateVmInitToDB(getParameters().getVmStaticData()); - new UpdateAndReorderVmNics(getParameters(), this).updateNics(); + updateAndReorderVmNics.updateNicsOnly(getParameters(), this); checkTrustedService(); setSucceeded(true); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmManagementCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmManagementCommandBase.java index 29904a8..4a1bdc1 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmManagementCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmManagementCommandBase.java @@ -8,15 +8,18 @@ import java.util.Map; import java.util.Set; import java.util.regex.Pattern; + +import javax.inject.Inject; + import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.bll.context.CommandContext; import org.ovirt.engine.core.bll.profiles.CpuProfileHelper; import org.ovirt.engine.core.bll.utils.VmDeviceUtils; import org.ovirt.engine.core.common.FeatureSupported; import org.ovirt.engine.core.common.action.VmManagementParametersBase; -import org.ovirt.engine.core.common.businessentities.InstanceType; import org.ovirt.engine.core.common.businessentities.DisplayType; import org.ovirt.engine.core.common.businessentities.GraphicsType; +import org.ovirt.engine.core.common.businessentities.InstanceType; import org.ovirt.engine.core.common.businessentities.MigrationSupport; import org.ovirt.engine.core.common.businessentities.VDS; import org.ovirt.engine.core.common.businessentities.VM; @@ -38,6 +41,9 @@ private InstanceType instanceType; + @Inject + protected UpdateAndReorderVmNics updateAndReorderVmNics; + public VmManagementCommandBase(T parameters) { super(parameters, null); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/ReorderNics.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/ReorderNics.java new file mode 100644 index 0000000..cd400d0 --- /dev/null +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/ReorderNics.java @@ -0,0 +1,93 @@ +package org.ovirt.engine.core.bll.network.vm; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashMap; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; + +import javax.inject.Inject; +import javax.inject.Singleton; + +import org.apache.commons.lang.StringUtils; +import org.ovirt.engine.core.common.businessentities.VmDevice; +import org.ovirt.engine.core.common.businessentities.VmDeviceGeneralType; +import org.ovirt.engine.core.common.businessentities.network.VmNic; +import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.core.dao.VmDeviceDAO; +import org.ovirt.engine.core.dao.network.VmNicDao; + +@Singleton +public class ReorderNics { + private final VmDeviceDAO vmDeviceDao; + private final VmNicDao vmNicDao; + + @Inject + public ReorderNics(VmNicDao vmNicDao, VmDeviceDAO vmDeviceDao) { + this.vmNicDao = vmNicDao; + this.vmDeviceDao = vmDeviceDao; + } + + public void updateNicsOrder(Guid vmId) { + List<VmNic> nicsToReorder = getVmNicsToReorder(vmId); + List<String> macsOfGivenNics = getMacsOfGivenNics(nicsToReorder); + + sortNicsByName(nicsToReorder); + sortMacsByName(macsOfGivenNics); + + for (int i = 0; i < nicsToReorder.size(); i++) { + VmNic nic = nicsToReorder.get(i); + nic.setMacAddress(macsOfGivenNics.get(i)); + vmNicDao.update(nic); + } + } + + public void sortMacsByName(List<String> macsOfGivenNics) { + Collections.sort(macsOfGivenNics); + } + + public List<String> getMacsOfGivenNics(List<VmNic> nicsToReorder) { + List<String> macsToReorder = new ArrayList<>(); + for (VmNic nic : nicsToReorder) { + macsToReorder.add(nic.getMacAddress()); + } + return macsToReorder; + } + + private List<VmNic> getVmNicsToReorder(Guid vmId) { + List<VmNic> result = new LinkedList<>(); + Map<Guid, VmDevice> vmInterfaceDevicesByDeviceId = getVmInterfaceDevicesByDeviceId(vmId); + for (VmNic nic : vmNicDao.getAllForVm(vmId)) { + VmDevice nicDevice = vmInterfaceDevicesByDeviceId.get(nic.getId()); + boolean notExistingDeviceOrPciAddressEmpty = nicDevice == null || StringUtils.isEmpty(nicDevice.getAddress()); + if (notExistingDeviceOrPciAddressEmpty) { + result.add(nic); + } + } + + return result; + } + + private Map<Guid, VmDevice> getVmInterfaceDevicesByDeviceId(Guid vmId) { + List<VmDevice> vmInterfaceDevicesList = + vmDeviceDao.getVmDeviceByVmIdAndType(vmId, VmDeviceGeneralType.INTERFACE); + + Map<Guid, VmDevice> vmInterfaceDevices = new HashMap<>(); + for (VmDevice device : vmInterfaceDevicesList) { + vmInterfaceDevices.put(device.getDeviceId(), device); + } + return vmInterfaceDevices; + } + + private void sortNicsByName(List<VmNic> nicsToReorder) { + Collections.sort(nicsToReorder, new Comparator<VmNic>() { + @Override + public int compare(VmNic nic1, VmNic nic2) { + return nic1.getName().compareTo(nic2.getName()); + } + }); + } + +} diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/ReorderVmNicsCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/ReorderVmNicsCommand.java deleted file mode 100644 index 123ea1f..0000000 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/ReorderVmNicsCommand.java +++ /dev/null @@ -1,95 +0,0 @@ -package org.ovirt.engine.core.bll.network.vm; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.Comparator; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - -import org.apache.commons.lang.StringUtils; -import org.ovirt.engine.core.bll.VmCommand; -import org.ovirt.engine.core.bll.context.CommandContext; -import org.ovirt.engine.core.common.action.VmOperationParameterBase; -import org.ovirt.engine.core.common.businessentities.VM; -import org.ovirt.engine.core.common.businessentities.VmDevice; -import org.ovirt.engine.core.common.businessentities.VmDeviceGeneralType; -import org.ovirt.engine.core.common.businessentities.network.VmNic; -import org.ovirt.engine.core.common.errors.VdcBllMessages; -import org.ovirt.engine.core.compat.Guid; - -public class ReorderVmNicsCommand<T extends VmOperationParameterBase> extends VmCommand<T> { - - public ReorderVmNicsCommand(T parameters) { - super(parameters); - } - - //required to call this command 'internally' - public ReorderVmNicsCommand(T parameters, CommandContext cmdContext) { - super(parameters, cmdContext); - } - - @Override - protected boolean canDoAction() { - VM vm = getVm(); - if (vm == null || vm.getStaticData() == null) { - addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_VM_NOT_FOUND); - return false; - } - - if (!canRunActionOnNonManagedVm()) { - return false; - } - - return true; - } - - @Override - protected void executeVmCommand() { - reorderNics(); - setSucceeded(true); - } - - private Map<Guid, VmDevice> getVmInterfaceDevices() { - List<VmDevice> vmInterfaceDevicesList = getVmDeviceDao().getVmDeviceByVmIdAndType(getParameters().getVmId(), VmDeviceGeneralType.INTERFACE); - Map<Guid, VmDevice> vmInterfaceDevices = new HashMap<>(); - for (VmDevice device : vmInterfaceDevicesList) { - vmInterfaceDevices.put(device.getDeviceId(), device); - } - return vmInterfaceDevices; - } - - private void reorderNics() { - Map<Guid, VmDevice> vmInterfaceDevices = getVmInterfaceDevices(); - List<VmNic> nics = getVmNicDao().getAllForVm(getParameters().getVmId()); - List<VmNic> nicsToReorder = new ArrayList<VmNic>(); - List<String> macsToReorder = new ArrayList<String>(); - - for (VmNic nic : nics) { - VmDevice nicDevice = vmInterfaceDevices.get(nic.getId()); - // If there is not device, or the PCI address is empty - if (nicDevice == null || StringUtils.isEmpty(nicDevice.getAddress())) { - nicsToReorder.add(nic); - // We know that all the NICs have a MAC address - macsToReorder.add(nic.getMacAddress()); - } - } - - // Sorting the NICs to reorder by name - Collections.sort(nicsToReorder, new Comparator<VmNic>() { - @Override - public int compare(VmNic nic1, VmNic nic2) { - return nic1.getName().compareTo(nic2.getName()); - } - }); - - // Sorting the MAC addresses to reorder - Collections.sort(macsToReorder); - for (int i = 0; i < nicsToReorder.size(); ++i) { - VmNic nic = nicsToReorder.get(i); - nic.setMacAddress(macsToReorder.get(i)); - getVmNicDao().update(nic); - } - } -} - diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcActionType.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcActionType.java index adcbfff..6eeec73 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcActionType.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcActionType.java @@ -23,7 +23,6 @@ MigrateVm(14, ActionGroup.MIGRATE_VM, QuotaDependency.NONE), InternalMigrateVm(15, QuotaDependency.NONE), MigrateVmToServer(16, ActionGroup.MIGRATE_VM, QuotaDependency.NONE), - ReorderVmNics(17, ActionGroup.CREATE_VM, false, QuotaDependency.NONE), VmLogon(18, ActionGroup.CONNECT_TO_VM, QuotaDependency.NONE), SetVmTicket(22, ActionGroup.CONNECT_TO_VM, false, QuotaDependency.NONE), ExportVm(23, ActionGroup.IMPORT_EXPORT_VM, QuotaDependency.NONE), -- To view, visit https://gerrit.ovirt.org/40032 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id3353cc030aa42feb07648ee20f25da2891217e2 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
