Arik Hadas has uploaded a new change for review. Change subject: core: cleanup in ovf readers where hardware is read ......................................................................
core: cleanup in ovf readers where hardware is read This patch reduces the duplicated code in template's OVF reader and VM's OVF reader which is related to the reads of the hardware section. The common code is now in OvfReader, and OvfTemplateReader & OvfVmReader overrides it when needed. Change-Id: Ia44e11f4c6eca2dc47ed2ee24bd855d7888fe015 Signed-off-by: Arik Hadas <[email protected]> --- M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfReader.java M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfTemplateReader.java M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfVmReader.java 3 files changed, 143 insertions(+), 209 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/41/29841/1 diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfReader.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfReader.java index d4257f9..373031b 100644 --- a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfReader.java +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfReader.java @@ -16,6 +16,7 @@ import org.ovirt.engine.core.common.businessentities.OriginType; import org.ovirt.engine.core.common.businessentities.SerialNumberPolicy; import org.ovirt.engine.core.common.businessentities.SsoMethod; +import org.ovirt.engine.core.common.businessentities.UsbPolicy; import org.ovirt.engine.core.common.businessentities.VmBase; import org.ovirt.engine.core.common.businessentities.VmDevice; import org.ovirt.engine.core.common.businessentities.VmDeviceGeneralType; @@ -30,6 +31,7 @@ import org.ovirt.engine.core.common.config.ConfigValues; import org.ovirt.engine.core.common.osinfo.OsRepository; import org.ovirt.engine.core.common.utils.SimpleDependecyInjector; +import org.ovirt.engine.core.common.utils.VmDeviceCommonUtils; import org.ovirt.engine.core.common.utils.VmDeviceType; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.compat.Version; @@ -312,7 +314,99 @@ protected abstract void readOsSection(XmlNode section); - protected abstract void readHardwareSection(XmlNode section); + protected void readHardwareSection(XmlNode section) { + for (XmlNode node : section.SelectNodes("Item")) { + + switch (node.SelectSingleNode("rasd:ResourceType", _xmlNS).innerText) { + case OvfHardware.CPU: + readCpuItem(node); + break; + + case OvfHardware.Memory: + readMemoryItem(node); + break; + + case OvfHardware.DiskImage: + readDiskImageItem(node); + break; + + case OvfHardware.Network: + readNetworkItem(node); + break; + + case OvfHardware.USB: + readUsbItem(node); + break; + + case OvfHardware.Monitor: + readMonitorItem(node); + break; + + case OvfHardware.CD: + readCdItem(node); + break; + + case OvfHardware.OTHER: + readOtherHardwareItem(node); + break; + } + } + } + + protected abstract void readDiskImageItem(XmlNode node); + + protected void readMonitorItem(XmlNode node) { + vmBase.setNumOfMonitors( + Integer.parseInt(node.SelectSingleNode("rasd:VirtualQuantity", _xmlNS).innerText)); + if (node.SelectSingleNode("rasd:SinglePciQxl", _xmlNS) != null) { + vmBase.setSingleQxlPci(Boolean.parseBoolean(node.SelectSingleNode("rasd:SinglePciQxl", _xmlNS).innerText)); + } + } + + private void readCpuItem(XmlNode node) { + vmBase.setNumOfSockets( + Integer.parseInt(node.SelectSingleNode("rasd:num_of_sockets", _xmlNS).innerText)); + vmBase.setCpuPerSocket( + Integer.parseInt(node.SelectSingleNode("rasd:cpu_per_socket", _xmlNS).innerText)); + } + + private void readMemoryItem(XmlNode node) { + vmBase.setMemSizeMb( + Integer.parseInt(node.SelectSingleNode("rasd:VirtualQuantity", _xmlNS).innerText)); + } + + private void readCdItem(XmlNode node) { + readVmDevice(node, vmBase, Guid.newGuid(), Boolean.TRUE); + } + + private void readNetworkItem(XmlNode node) { + VmNetworkInterface iface = getNetwotkInterface(node); + updateSingleNic(node, iface); + vmBase.getInterfaces().add(iface); + readVmDevice(node, vmBase, iface.getId(), Boolean.TRUE); + } + + private void readUsbItem(XmlNode node) { + vmBase.setUsbPolicy( + UsbPolicy.forStringValue(node.SelectSingleNode("rasd:UsbPolicy", _xmlNS).innerText)); + } + + private void readOtherHardwareItem(XmlNode node) { + boolean managed = false; + if (node.SelectSingleNode(OvfProperties.VMD_TYPE, _xmlNS) != null + && StringUtils.isNotEmpty(node.SelectSingleNode(OvfProperties.VMD_TYPE, _xmlNS).innerText)) { + VmDeviceGeneralType type = VmDeviceGeneralType.forValue(String.valueOf(node.SelectSingleNode(OvfProperties.VMD_TYPE, _xmlNS).innerText)); + String device = node.SelectSingleNode(OvfProperties.VMD_DEVICE, _xmlNS).innerText; + // special devices are treated as managed devices but still have the OTHER OVF ResourceType + managed = VmDeviceCommonUtils.isSpecialDevice(device, type); + } + + if (managed) { + readVmDevice(node, vmBase, Guid.newGuid(), Boolean.TRUE); + } else { + readVmDevice(node, vmBase, Guid.newGuid(), Boolean.FALSE); + } + } protected VmDeviceType getDisplayDevice(DisplayType displayType) { return osRepository.getDisplayDevice(vmBase.getOsId(), new Version(getVersion()), displayType); @@ -648,11 +742,6 @@ } protected void updateSingleNic(XmlNode node, VmNetworkInterface iface) { - - iface.setName(node.SelectSingleNode(OvfProperties.VMD_NAME, _xmlNS).innerText); - iface.setMacAddress((node.SelectSingleNode("rasd:MACAddress", _xmlNS) != null) ? node.SelectSingleNode( - "rasd:MACAddress", _xmlNS).innerText : ""); - String networkName = node.SelectSingleNode(OvfProperties.VMD_CONNECTION, _xmlNS).innerText; iface.setNetworkName(StringUtils.defaultIfEmpty(networkName, null)); diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfTemplateReader.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfTemplateReader.java index a214d55..32eb234 100644 --- a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfTemplateReader.java +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfTemplateReader.java @@ -7,19 +7,14 @@ import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.common.businessentities.ArchitectureType; import org.ovirt.engine.core.common.businessentities.DiskImage; -import org.ovirt.engine.core.common.businessentities.UsbPolicy; -import org.ovirt.engine.core.common.businessentities.VmDeviceGeneralType; import org.ovirt.engine.core.common.businessentities.VmEntityType; import org.ovirt.engine.core.common.businessentities.VmTemplate; -import org.ovirt.engine.core.common.businessentities.network.VmInterfaceType; import org.ovirt.engine.core.common.businessentities.network.VmNetworkInterface; import org.ovirt.engine.core.common.osinfo.OsRepository; import org.ovirt.engine.core.common.utils.SimpleDependecyInjector; -import org.ovirt.engine.core.common.utils.VmDeviceCommonUtils; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.compat.backendcompat.XmlDocument; import org.ovirt.engine.core.compat.backendcompat.XmlNode; -import org.ovirt.engine.core.compat.backendcompat.XmlNodeList; import org.ovirt.engine.core.utils.linq.LinqUtils; import org.ovirt.engine.core.utils.linq.Predicate; @@ -50,121 +45,48 @@ } @Override - protected void readHardwareSection(XmlNode section) { - XmlNodeList list = section.SelectNodes("Item"); - for (XmlNode node : list) { - int resourceType = Integer.parseInt(node.SelectSingleNode("rasd:ResourceType", _xmlNS).innerText); + protected void readMonitorItem(XmlNode node) { + super.readMonitorItem(node); + readVmDevice(node, _vmTemplate, Guid.newGuid(), Boolean.TRUE); + } - switch (resourceType) { - // CPU - case 3: - _vmTemplate - .setNumOfSockets(Integer.parseInt(node.SelectSingleNode("rasd:num_of_sockets", _xmlNS).innerText)); - _vmTemplate - .setCpuPerSocket(Integer.parseInt(node.SelectSingleNode("rasd:cpu_per_socket", _xmlNS).innerText)); - break; + @Override + protected void readDiskImageItem(XmlNode node) { + final Guid guid = new Guid(node.SelectSingleNode("rasd:InstanceId", _xmlNS).innerText); - // Memory - case 4: - _vmTemplate - .setMemSizeMb(Integer.parseInt(node.SelectSingleNode("rasd:VirtualQuantity", _xmlNS).innerText)); - break; - - // Image - case 17: - final Guid guid = new Guid(node.SelectSingleNode("rasd:InstanceId", _xmlNS).innerText); - - DiskImage image = LinqUtils.firstOrNull(_images, new Predicate<DiskImage>() { - @Override - public boolean eval(DiskImage diskImage) { - return diskImage.getImageId().equals(guid); - } - }); - image.setId(OvfParser.GetImageGrupIdFromImageFile(node.SelectSingleNode( - "rasd:HostResource", _xmlNS).innerText)); - if (StringUtils.isNotEmpty(node.SelectSingleNode("rasd:Parent", _xmlNS).innerText)) { - image.setParentId(new Guid(node.SelectSingleNode("rasd:Parent", _xmlNS).innerText)); - } - if (StringUtils.isNotEmpty(node.SelectSingleNode("rasd:Template", _xmlNS).innerText)) { - image.setImageTemplateId(new Guid(node.SelectSingleNode("rasd:Template", _xmlNS).innerText)); - } - image.setAppList(node.SelectSingleNode("rasd:ApplicationList", _xmlNS).innerText); - if (StringUtils.isNotEmpty(node.SelectSingleNode("rasd:StorageId", _xmlNS).innerText)) { - image.setStorageIds(new ArrayList<Guid>(Arrays.asList(new Guid(node.SelectSingleNode("rasd:StorageId", - _xmlNS).innerText)))); - } - if (StringUtils.isNotEmpty(node.SelectSingleNode("rasd:StoragePoolId", _xmlNS).innerText)) { - image.setStoragePoolId(new Guid(node.SelectSingleNode("rasd:StoragePoolId", _xmlNS).innerText)); - } - final Date creationDate = OvfParser.UtcDateStringToLocaDate( - node.SelectSingleNode("rasd:CreationDate", _xmlNS).innerText); - if (creationDate != null) { - image.setCreationDate(creationDate); - } - final Date lastModified = OvfParser.UtcDateStringToLocaDate( - node.SelectSingleNode("rasd:LastModified", _xmlNS).innerText); - if (lastModified != null) { - image.setLastModified(lastModified); - } - readVmDevice(node, _vmTemplate, image.getId(), Boolean.TRUE); - break; - - // Network - case 10: - VmNetworkInterface iface = getNetwotkInterface(node); - if (StringUtils.isNotEmpty(node.SelectSingleNode("rasd:ResourceSubType", _xmlNS).innerText)) { - iface.setType(Integer.parseInt(node.SelectSingleNode("rasd:ResourceSubType", _xmlNS).innerText)); - } - - String resourceSubNetworkName = node.SelectSingleNode(OvfProperties.VMD_CONNECTION, _xmlNS).innerText; - iface.setNetworkName(StringUtils.defaultIfEmpty(resourceSubNetworkName, null)); - - XmlNode vnicProfileNameNode = node.SelectSingleNode(OvfProperties.VMD_VNIC_PROFILE_NAME, _xmlNS); - iface.setVnicProfileName(vnicProfileNameNode == null ? null - : StringUtils.defaultIfEmpty(vnicProfileNameNode.innerText, null)); - - XmlNode linkedNode = node.SelectSingleNode(OvfProperties.VMD_LINKED, _xmlNS); - iface.setLinked(linkedNode == null ? true : Boolean.valueOf(linkedNode.innerText)); - iface.setName(node.SelectSingleNode("rasd:Name", _xmlNS).innerText); - iface.setSpeed((node.SelectSingleNode("rasd:speed", _xmlNS) != null) ? Integer - .parseInt(node.SelectSingleNode("rasd:speed", _xmlNS).innerText) - : VmInterfaceType.forValue(iface.getType()).getSpeed()); - _vmTemplate.getInterfaces().add(iface); - readVmDevice(node, _vmTemplate, iface.getId(), Boolean.TRUE); - break; - // CDROM - case 15: - readVmDevice(node, _vmTemplate, Guid.newGuid(), Boolean.TRUE); - break; - // USB - case 23: - _vmTemplate.setUsbPolicy(UsbPolicy.forStringValue(node.SelectSingleNode("rasd:UsbPolicy", _xmlNS).innerText)); - break; - - // Monitor - case 20: - _vmTemplate - .setNumOfMonitors(Integer.parseInt(node.SelectSingleNode("rasd:VirtualQuantity", _xmlNS).innerText)); - if (node.SelectSingleNode("rasd:SinglePciQxl", _xmlNS) != null) { - _vmTemplate.setSingleQxlPci(Boolean.parseBoolean(node.SelectSingleNode("rasd:SinglePciQxl", _xmlNS).innerText)); - } - readVmDevice(node, _vmTemplate, Guid.newGuid(), Boolean.TRUE); - break; - // OTHER - case 0: - boolean addAsManaged = false; - if (node.SelectSingleNode(OvfProperties.VMD_TYPE, _xmlNS) != null - && StringUtils.isNotEmpty(node.SelectSingleNode(OvfProperties.VMD_TYPE, _xmlNS).innerText)) { - VmDeviceGeneralType type = VmDeviceGeneralType.forValue(node.SelectSingleNode(OvfProperties.VMD_TYPE, _xmlNS).innerText); - String device = node.SelectSingleNode(OvfProperties.VMD_DEVICE, _xmlNS).innerText; - // special devices are treated as managed devices but still have the OTHER OVF ResourceType - addAsManaged = VmDeviceCommonUtils.isSpecialDevice(device, type); - } - readVmDevice(node, _vmTemplate, Guid.newGuid(), addAsManaged); - break; - + DiskImage image = LinqUtils.firstOrNull(_images, new Predicate<DiskImage>() { + @Override + public boolean eval(DiskImage diskImage) { + return diskImage.getImageId().equals(guid); } + }); + image.setId(OvfParser.GetImageGrupIdFromImageFile(node.SelectSingleNode( + "rasd:HostResource", _xmlNS).innerText)); + if (StringUtils.isNotEmpty(node.SelectSingleNode("rasd:Parent", _xmlNS).innerText)) { + image.setParentId(new Guid(node.SelectSingleNode("rasd:Parent", _xmlNS).innerText)); } + if (StringUtils.isNotEmpty(node.SelectSingleNode("rasd:Template", _xmlNS).innerText)) { + image.setImageTemplateId(new Guid(node.SelectSingleNode("rasd:Template", _xmlNS).innerText)); + } + image.setAppList(node.SelectSingleNode("rasd:ApplicationList", _xmlNS).innerText); + if (StringUtils.isNotEmpty(node.SelectSingleNode("rasd:StorageId", _xmlNS).innerText)) { + image.setStorageIds(new ArrayList<Guid>(Arrays.asList(new Guid(node.SelectSingleNode("rasd:StorageId", + _xmlNS).innerText)))); + } + if (StringUtils.isNotEmpty(node.SelectSingleNode("rasd:StoragePoolId", _xmlNS).innerText)) { + image.setStoragePoolId(new Guid(node.SelectSingleNode("rasd:StoragePoolId", _xmlNS).innerText)); + } + final Date creationDate = OvfParser.UtcDateStringToLocaDate( + node.SelectSingleNode("rasd:CreationDate", _xmlNS).innerText); + if (creationDate != null) { + image.setCreationDate(creationDate); + } + final Date lastModified = OvfParser.UtcDateStringToLocaDate( + node.SelectSingleNode("rasd:LastModified", _xmlNS).innerText); + if (lastModified != null) { + image.setLastModified(lastModified); + } + readVmDevice(node, _vmTemplate, image.getId(), Boolean.TRUE); } @Override diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfVmReader.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfVmReader.java index adc8db0..4fa4447 100644 --- a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfVmReader.java +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfVmReader.java @@ -55,58 +55,7 @@ } @Override - protected void readHardwareSection(XmlNode section) { - for (XmlNode node : section.SelectNodes("Item")) { - - switch (node.SelectSingleNode("rasd:ResourceType", _xmlNS).innerText) { - case OvfHardware.CPU: - readCpuItem(node); - break; - - case OvfHardware.Memory: - readMemoryItem(node); - break; - - case OvfHardware.DiskImage: - readDiskImageItem(node); - break; - - case OvfHardware.Network: - readNetworkItem(node); - break; - - case OvfHardware.USB: - readUsbItem(node); - break; - - case OvfHardware.Monitor: - readMonitorItem(node); - break; - - case OvfHardware.CD: - readCdItem(node); - break; - - case OvfHardware.OTHER: - readOtherHardwareItem(node); - break; - } - } - } - - private void readCpuItem(XmlNode node) { - _vm.getStaticData().setNumOfSockets( - Integer.parseInt(node.SelectSingleNode("rasd:num_of_sockets", _xmlNS).innerText)); - _vm.getStaticData().setCpuPerSocket( - Integer.parseInt(node.SelectSingleNode("rasd:cpu_per_socket", _xmlNS).innerText)); - } - - private void readMemoryItem(XmlNode node) { - _vm.getStaticData().setMemSizeMb( - Integer.parseInt(node.SelectSingleNode("rasd:VirtualQuantity", _xmlNS).innerText)); - } - - private void readDiskImageItem(XmlNode node) { + protected void readDiskImageItem(XmlNode node) { final Guid guid = new Guid(node.SelectSingleNode("rasd:InstanceId", _xmlNS).innerText); DiskImage image = LinqUtils.firstOrNull(_images, new Predicate<DiskImage>() { @@ -153,24 +102,10 @@ image.setReadOnly(readDevice.getIsReadOnly()); } - private void readNetworkItem(XmlNode node) { - VmNetworkInterface iface = getNetwotkInterface(node); - updateSingleNic(node, iface); - _vm.getInterfaces().add(iface); - readVmDevice(node, _vm.getStaticData(), iface.getId(), Boolean.TRUE); - } + @Override + protected void readMonitorItem(XmlNode node) { + super.readMonitorItem(node); - private void readUsbItem(XmlNode node) { - _vm.getStaticData().setUsbPolicy( - UsbPolicy.forStringValue(node.SelectSingleNode("rasd:UsbPolicy", _xmlNS).innerText)); - } - - private void readMonitorItem(XmlNode node) { - _vm.getStaticData().setNumOfMonitors( - Integer.parseInt(node.SelectSingleNode("rasd:VirtualQuantity", _xmlNS).innerText)); - if (node.SelectSingleNode("rasd:SinglePciQxl", _xmlNS) != null) { - _vm.setSingleQxlPci(Boolean.parseBoolean(node.SelectSingleNode("rasd:SinglePciQxl", _xmlNS).innerText)); - } if (new Version(getVersion()).compareTo(Version.v3_1) >= 0) { readVmDevice(node, _vm.getStaticData(), Guid.newGuid(), Boolean.TRUE); } else { @@ -182,24 +117,12 @@ } } - private void readCdItem(XmlNode node) { - readVmDevice(node, _vm.getStaticData(), Guid.newGuid(), Boolean.TRUE); - } - - private void readOtherHardwareItem(XmlNode node) { - if (node.SelectSingleNode(OvfProperties.VMD_TYPE, _xmlNS) != null - && StringUtils.isNotEmpty(node.SelectSingleNode(OvfProperties.VMD_TYPE, _xmlNS).innerText)) { - VmDeviceGeneralType type = VmDeviceGeneralType.forValue(String.valueOf(node.SelectSingleNode(OvfProperties.VMD_TYPE, _xmlNS).innerText)); - String device = String.valueOf(node.SelectSingleNode(OvfProperties.VMD_DEVICE, _xmlNS).innerText); - // special devices are treated as managed devices but still have the OTHER OVF ResourceType - if (VmDeviceCommonUtils.isSpecialDevice(device, type)) { - readVmDevice(node, _vm.getStaticData(), Guid.newGuid(), Boolean.TRUE); - } else { - readVmDevice(node, _vm.getStaticData(), Guid.newGuid(), Boolean.FALSE); - } - } else { - readVmDevice(node, _vm.getStaticData(), Guid.newGuid(), Boolean.FALSE); - } + @Override + protected void updateSingleNic(XmlNode node, VmNetworkInterface iface) { + super.updateSingleNic(node, iface); + iface.setName(node.SelectSingleNode(OvfProperties.VMD_NAME, _xmlNS).innerText); + iface.setMacAddress((node.SelectSingleNode("rasd:MACAddress", _xmlNS) != null) ? node.SelectSingleNode( + "rasd:MACAddress", _xmlNS).innerText : ""); } @Override -- To view, visit http://gerrit.ovirt.org/29841 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia44e11f4c6eca2dc47ed2ee24bd855d7888fe015 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
