Arik Hadas has uploaded a new change for review.

Change subject: core: fixed video device setting on import vm/template
......................................................................

core: fixed video device setting on import vm/template

This patch solves a bug in which the wrong video device was set for
vm/template that was exported in a release prior to 3.1 and imported
in release 3.1 or later.

The logic that selected the video device according to the number of
monitors of the vm/template was replaced. the logic now is to set it
according to the default display type of the vm/template, and only if
the default display type doesn't exist (should never happen though),
the video device would be decided according to the number of monitors.

Change-Id: Id1bfaa3414507a982b9f22aa21fb3d05fe87e84d
Bug-Url: https://bugzilla.redhat.com/889499
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, 47 insertions(+), 26 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/25/10525/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 c7d0f31..35f097a 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
@@ -10,6 +10,7 @@
 import org.ovirt.engine.core.common.businessentities.BootSequence;
 import org.ovirt.engine.core.common.businessentities.DiskImage;
 import org.ovirt.engine.core.common.businessentities.DiskInterface;
+import org.ovirt.engine.core.common.businessentities.DisplayType;
 import org.ovirt.engine.core.common.businessentities.ImageStatus;
 import org.ovirt.engine.core.common.businessentities.OriginType;
 import org.ovirt.engine.core.common.businessentities.VmBase;
@@ -22,6 +23,7 @@
 import org.ovirt.engine.core.common.businessentities.VolumeType;
 import org.ovirt.engine.core.common.utils.VmDeviceType;
 import org.ovirt.engine.core.compat.Guid;
+import org.ovirt.engine.core.compat.StringHelper;
 import org.ovirt.engine.core.compat.backendcompat.XmlDocument;
 import org.ovirt.engine.core.compat.backendcompat.XmlNamespaceManager;
 import org.ovirt.engine.core.compat.backendcompat.XmlNode;
@@ -41,6 +43,7 @@
     protected String name = EmptyName;
     private String version;
     private final VmBase vmBase;
+    private DisplayType defaultDisplayType;
 
     public OvfReader(XmlDocument document, ArrayList<DiskImage> images, 
ArrayList<VmNetworkInterface> interfaces, VmBase vmBase) {
         _images = images;
@@ -271,6 +274,15 @@
         return iface;
     }
 
+    /**
+     * This method should return the string representation of 'default display 
type'
+     * property in the ovf file. this representation is different for ovf file
+     * of VM and ovf file of template.
+     *
+     * @return the String representation of 'default display type' property in 
the ovf file
+     */
+    protected abstract String getDefaultDisplayTypeStringRepresentation();
+
     protected abstract void ReadOsSection(XmlNode section);
 
     protected abstract void ReadHardwareSection(XmlNode section);
@@ -348,6 +360,16 @@
             vmBase.setDbGeneration(Long.parseLong(node.InnerText));
         } else {
             vmBase.setDbGeneration(1L);
+        }
+
+        // Note: the fetching of 'default display type' should happen before 
reading
+        // the hardware section
+        node = 
content.SelectSingleNode(getDefaultDisplayTypeStringRepresentation());
+        if (node != null) {
+            if (!StringHelper.isNullOrEmpty(node.InnerText)) {
+                defaultDisplayType = 
DisplayType.forValue(Integer.parseInt(node.InnerText));
+                vmBase.setdefault_display_type(defaultDisplayType);
+            }
         }
 
         XmlNodeList list = content.SelectNodes("Section");
@@ -477,19 +499,25 @@
         if (resourceSubType == -1) {
             // we need special handling for Monitor to define it as vnc or 
spice
             if (Integer.valueOf(OvfHardware.Monitor) == resourceType) {
-                // get number of monitors from VirtualQuantity in OVF
-                if (node.SelectSingleNode(OvfProperties.VMD_VIRTUAL_QUANTITY, 
_xmlNS) != null
-                        && 
!StringUtils.isEmpty(node.SelectSingleNode(OvfProperties.VMD_VIRTUAL_QUANTITY,
-                                _xmlNS).InnerText)) {
-                    int virtualQuantity =
-                            
Integer.valueOf(node.SelectSingleNode(OvfProperties.VMD_VIRTUAL_QUANTITY, 
_xmlNS).InnerText);
-                    if (virtualQuantity > 1) {
+                // if default display type is defined in the ovf, set the 
video device that is suitable for it
+                if (defaultDisplayType != null) {
+                    
vmDevice.setDevice(defaultDisplayType.getVmDeviceType().getName());
+                }
+                else {
+                    // get number of monitors from VirtualQuantity in OVF
+                    if 
(node.SelectSingleNode(OvfProperties.VMD_VIRTUAL_QUANTITY, _xmlNS) != null
+                            && 
!StringUtils.isEmpty(node.SelectSingleNode(OvfProperties.VMD_VIRTUAL_QUANTITY,
+                                    _xmlNS).InnerText)) {
+                        int virtualQuantity =
+                                
Integer.valueOf(node.SelectSingleNode(OvfProperties.VMD_VIRTUAL_QUANTITY, 
_xmlNS).InnerText);
+                        if (virtualQuantity > 1) {
+                            vmDevice.setDevice(VmDeviceType.QXL.getName());
+                        } else {
+                            vmDevice.setDevice(VmDeviceType.CIRRUS.getName());
+                        }
+                    } else { // default to spice if quantity not found
                         vmDevice.setDevice(VmDeviceType.QXL.getName());
-                    } else {
-                        vmDevice.setDevice(VmDeviceType.CIRRUS.getName());
                     }
-                } else { // default to spice if quantity not found
-                    vmDevice.setDevice(VmDeviceType.QXL.getName());
                 }
             } else {
                 
vmDevice.setDevice(VmDeviceType.getoVirtDevice(resourceType).getName());
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 bc9def2..9189fe0 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
@@ -6,7 +6,6 @@
 
 import org.apache.commons.lang.StringUtils;
 import org.ovirt.engine.core.common.businessentities.DiskImage;
-import org.ovirt.engine.core.common.businessentities.DisplayType;
 import org.ovirt.engine.core.common.businessentities.UsbPolicy;
 import org.ovirt.engine.core.common.businessentities.VmInterfaceType;
 import org.ovirt.engine.core.common.businessentities.VmNetworkInterface;
@@ -159,11 +158,9 @@
                 _vmTemplate.setId(new Guid(node.InnerText));
             }
         }
-        node = content.SelectSingleNode("default_display_type");
-        if (node != null) {
-            if (!StringHelper.isNullOrEmpty(node.InnerText)) {
-                
_vmTemplate.setdefault_display_type(DisplayType.forValue(Integer.parseInt(node.InnerText)));
-            }
-        }
+    }
+
+    protected String getDefaultDisplayTypeStringRepresentation() {
+        return "default_display_type";
     }
 }
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 ffc11fd..a3e9566 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
@@ -7,7 +7,6 @@
 
 import org.apache.commons.codec.binary.Base64;
 import org.ovirt.engine.core.common.businessentities.DiskImage;
-import org.ovirt.engine.core.common.businessentities.DisplayType;
 import org.ovirt.engine.core.common.businessentities.Snapshot;
 import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotStatus;
 import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotType;
@@ -209,20 +208,17 @@
             }
         }
 
-        node = content.SelectSingleNode("DefaultDisplayType");
-        if (node != null) {
-            if (!StringHelper.isNullOrEmpty(node.InnerText)) {
-                
_vm.setDefaultDisplayType(DisplayType.forValue(Integer.parseInt(node.InnerText)));
-            }
-        }
-
         node = content.SelectSingleNode("MinAllocatedMem");
         if (node != null) {
             if (!StringHelper.isNullOrEmpty(node.InnerText)) {
                 _vm.setMinAllocatedMem(Integer.parseInt(node.InnerText));
             }
         }
+    }
 
+    @Override
+    protected String getDefaultDisplayTypeStringRepresentation() {
+        return "DefaultDisplayType";
     }
 
     // function returns the index of the image that has no parent


--
To view, visit http://gerrit.ovirt.org/10525
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Id1bfaa3414507a982b9f22aa21fb3d05fe87e84d
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

Reply via email to