Martin Betak has uploaded a new change for review.

Change subject: frontend: Proper VM SystemTab Validation
......................................................................

frontend: Proper VM SystemTab Validation

UnitVmModel#validate() used async query to get max valid mem size and
thus caused race with submiting the Vm dialog so the memory was not
validated at all.

Added missing validation for whole SystemTab.

Removed unused AsyncDataProvider methods to get various memory-limit
related ConfigValues and corresponding (also unused) UnitVmModel fields.

Added support for TB memory size in MemorySizeParser and disabled
parsing of garbled input '12a30 MB' as longest valid prefix ('12 MB').

Change-Id: I33761ac7c5491c17fff928de13fdf16065d47e1e
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1069628
Signed-off-by: Martin Betak <mbe...@redhat.com>
---
M 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/parser/MemorySizeParser.java
M 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/AbstractVmPopupWidget.java
M 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java
M 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java
M 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/ByteSizeValidation.java
5 files changed, 45 insertions(+), 178 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/68/26268/1

diff --git 
a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/parser/MemorySizeParser.java
 
b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/parser/MemorySizeParser.java
index 112e5ee..0fd8587 100644
--- 
a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/parser/MemorySizeParser.java
+++ 
b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/parser/MemorySizeParser.java
@@ -10,7 +10,7 @@
 
     @Override
     public Integer parse(CharSequence text) throws ParseException {
-        MatchResult match = 
RegExp.compile("(\\d*)\\s*(\\w*)").exec(text.toString()); //$NON-NLS-1$
+        MatchResult match = 
RegExp.compile("^(\\d*)\\s*(\\w*)$").exec(text.toString()); //$NON-NLS-1$
         String prefix = match.getGroup(1);
         String suffix = match.getGroup(2);
         Integer size = null;
@@ -21,13 +21,18 @@
             return 0;
         }
 
+        if (suffix.equalsIgnoreCase("TB") || suffix.equalsIgnoreCase("T")) { 
//$NON-NLS-1$ $NON-NLS-2$
+            size *= 1024 * 1024;
+            return size;
+        }
+
         if (suffix.equalsIgnoreCase("GB") || suffix.equalsIgnoreCase("G")) {  
//$NON-NLS-1$ $NON-NLS-2$
             size *= 1024;
             return size;
         }
 
         if (suffix.equalsIgnoreCase("MB") || suffix.equalsIgnoreCase("M")) { 
//$NON-NLS-1$ $NON-NLS-2$
-            return Integer.parseInt(prefix);
+            return size;
         }
 
         return size;
diff --git 
a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/AbstractVmPopupWidget.java
 
b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/AbstractVmPopupWidget.java
index 9ce8f6c..42d970d 100644
--- 
a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/AbstractVmPopupWidget.java
+++ 
b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/AbstractVmPopupWidget.java
@@ -1358,6 +1358,12 @@
                     } else {
                         generalTab.markAsInvalid(null);
                     }
+                } else if ("IsSystemTabValid".equals(propName)) {  
//$NON-NLS-1$
+                    if (vm.getIsSystemTabValid()) {
+                        systemTab.markAsValid();
+                    } else {
+                        systemTab.markAsInvalid(null);
+                    }
                 } else if ("IsDisplayTabValid".equals(propName)) { 
//$NON-NLS-1$
                     if (vm.getIsDisplayTabValid()) {
                         consoleTab.markAsValid();
diff --git 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java
 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java
index c6e513a..609ec00 100644
--- 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java
+++ 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java
@@ -652,20 +652,6 @@
                 aQuery);
     }
 
-    public static void getMinimalVmMemSize(AsyncQuery aQuery) {
-        aQuery.converterCallback = new IAsyncConverter() {
-            @Override
-            public Object Convert(Object source, AsyncQuery _asyncQuery)
-            {
-                return source != null ? ((Integer) source).intValue() : 1;
-            }
-        };
-        getConfigFromCache(
-                new 
GetConfigurationValueParameters(ConfigurationValues.VMMinMemorySizeInMB,
-                        getDefaultConfigurationVersion()),
-                aQuery);
-    }
-
     public static void getSpiceUsbAutoShare(AsyncQuery aQuery) {
         aQuery.converterCallback = new IAsyncConverter() {
             @Override
@@ -722,35 +708,6 @@
         };
         getConfigFromCache(
                 new 
GetConfigurationValueParameters(ConfigurationValues.WANDisableEffects,
-                        getDefaultConfigurationVersion()),
-                aQuery);
-    }
-
-    public static void getMaximalVmMemSize64OS(AsyncQuery aQuery, String 
version) {
-        aQuery.converterCallback = new IAsyncConverter() {
-            @Override
-            public Object Convert(Object source, AsyncQuery _asyncQuery)
-            {
-                // we should detect missing config values instead of putting 
in obsolete hardcoded values
-                return source != null ? ((Integer) source).intValue() : -1;
-            }
-        };
-        GetConfigurationValueParameters tempVar =
-                new 
GetConfigurationValueParameters(ConfigurationValues.VM64BitMaxMemorySizeInMB);
-        tempVar.setVersion(version);
-        getConfigFromCache(tempVar, aQuery);
-    }
-
-    public static void getMaximalVmMemSize32OS(AsyncQuery aQuery) {
-        aQuery.converterCallback = new IAsyncConverter() {
-            @Override
-            public Object Convert(Object source, AsyncQuery _asyncQuery)
-            {
-                return source != null ? ((Integer) source).intValue() : 20480;
-            }
-        };
-        getConfigFromCache(
-                new 
GetConfigurationValueParameters(ConfigurationValues.VM32BitMaxMemorySizeInMB,
                         getDefaultConfigurationVersion()),
                 aQuery);
     }
@@ -1240,24 +1197,6 @@
             setSnapshots(snapshots);
             setDisk(disk);
         }
-    }
-
-    public static void getMaxVmMemSize(AsyncQuery aQuery, boolean is64) {
-        aQuery.converterCallback = new IAsyncConverter() {
-            @Override
-            public Object Convert(Object source, AsyncQuery _asyncQuery)
-            {
-                if (source != null)
-                {
-                    return source;
-                }
-                return 262144;
-            }
-        };
-        getConfigFromCache(
-                new GetConfigurationValueParameters(is64 ? 
ConfigurationValues.VM64BitMaxMemorySizeInMB
-                        : ConfigurationValues.VM32BitMaxMemorySizeInMB, 
getDefaultConfigurationVersion()),
-                aQuery);
     }
 
     public static void getDomainList(AsyncQuery aQuery, boolean 
filterInternalDomain) {
diff --git 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java
 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java
index ca6175f..7f3f139 100644
--- 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java
+++ 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java
@@ -73,6 +73,7 @@
     private boolean privateIsNew;
 
     private EntityModel<String> spiceProxy;
+
     public EntityModel<String> getSpiceProxy() {
         return spiceProxy;
     }
@@ -327,6 +328,19 @@
         {
             isGeneralTabValid = value;
             onPropertyChanged(new 
PropertyChangedEventArgs("IsGeneralTabValid")); //$NON-NLS-1$
+        }
+    }
+
+    private boolean isSystemTabValid;
+
+    public boolean getIsSystemTabValid() {
+        return isSystemTabValid;
+    }
+
+    public void setIsSystemTabValid(boolean value) {
+        if (isSystemTabValid != value) {
+            isSystemTabValid = value;
+            onPropertyChanged(new 
PropertyChangedEventArgs("IsSystemTabValid")); //$NON-NLS-1$
         }
     }
 
@@ -1178,42 +1192,6 @@
     private void setBehavior(VmModelBehaviorBase value) {
     }
 
-    private int _minMemSize = 1;
-
-    public int get_MinMemSize()
-    {
-        return _minMemSize;
-    }
-
-    public void set_MinMemSize(int value)
-    {
-        _minMemSize = value;
-    }
-
-    private int _maxMemSize32 = 20480;
-
-    public int get_MaxMemSize32()
-    {
-        return _maxMemSize32;
-    }
-
-    public void set_MaxMemSize32(int value)
-    {
-        _maxMemSize32 = value;
-    }
-
-    private int _maxMemSize64 = 2097152;
-
-    public int get_MaxMemSize64()
-    {
-        return _maxMemSize64;
-    }
-
-    public void set_MaxMemSize64(int value)
-    {
-        _maxMemSize64 = value;
-    }
-
     private NotChangableForVmInPoolEntityModel<String> cpuPinning;
 
     public EntityModel<String> getCpuPinning() {
@@ -1534,8 +1512,6 @@
         initFirstBootDevice();
         initNumOfMonitors();
         initAllowConsoleReconnect();
-        initMinimalVmMemSize();
-        initMaximalVmMemSize32OS();
         initMigrationMode();
         initVncKeyboardLayout();
 
@@ -1725,7 +1701,8 @@
                                                                      }
 
                                                                  }
-                                                             }, getHash()));
+                                                             }, getHash()
+        ));
 
     }
 
@@ -1780,34 +1757,6 @@
         getUsbPolicy().setSelectedItem(UsbPolicy.DISABLED);
     }
 
-    private void initMinimalVmMemSize()
-    {
-        AsyncDataProvider.getMinimalVmMemSize(new AsyncQuery(this,
-                                                             new 
INewAsyncCallback() {
-                                                                 @Override
-                                                                 public void 
onSuccess(Object target, Object returnValue) {
-
-                                                                     
UnitVmModel vmModel = (UnitVmModel) target;
-                                                                     
vmModel.set_MinMemSize((Integer) returnValue);
-
-                                                                 }
-                                                             }, getHash()));
-    }
-
-    private void initMaximalVmMemSize32OS()
-    {
-        AsyncDataProvider.getMaximalVmMemSize32OS(new AsyncQuery(this,
-                                                                 new 
INewAsyncCallback() {
-                                                                     @Override
-                                                                     public 
void onSuccess(Object target, Object returnValue) {
-
-                                                                         
UnitVmModel vmModel = (UnitVmModel) target;
-                                                                         
vmModel.set_MaxMemSize32((Integer) returnValue);
-
-                                                                     }
-                                                                 }, 
getHash()));
-    }
-
     private void updateMigrationOptions()
     {
         DataCenterWithCluster dataCenterWithCluster =
@@ -1820,36 +1769,12 @@
 
         Boolean isMigrationSupported =
                 
AsyncDataProvider.isMigrationSupported(cluster.getArchitecture(),
-                        cluster.getcompatibility_version());
+                                                       
cluster.getcompatibility_version());
 
         if (isMigrationSupported) {
             
getMigrationMode().setItems(Arrays.asList(MigrationSupport.values()));
         } else {
             
getMigrationMode().setItems(Arrays.asList(MigrationSupport.PINNED_TO_HOST));
-        }
-    }
-
-    private void updateMaximalVmMemSize()
-    {
-        DataCenterWithCluster dataCenterWithCluster = 
getDataCenterWithClustersList().getSelectedItem();
-        if (dataCenterWithCluster == null) {
-            return;
-        }
-
-        VDSGroup cluster = dataCenterWithCluster.getCluster();
-
-        if (cluster != null)
-        {
-            AsyncDataProvider.getMaximalVmMemSize64OS(new AsyncQuery(this,
-                    new INewAsyncCallback() {
-                        @Override
-                        public void onSuccess(Object target, Object 
returnValue) {
-
-                            UnitVmModel vmModel = (UnitVmModel) target;
-                            vmModel.set_MaxMemSize64((Integer) returnValue);
-
-                        }
-                    }, getHash()), 
cluster.getcompatibility_version().toString());
         }
     }
 
@@ -1943,7 +1868,6 @@
         }
 
         updateMigrationOptions();
-        updateMaximalVmMemSize();
         handleQxlClusterLevel();
 
         updateWatchdogModels();
@@ -2000,7 +1924,6 @@
         updateWatchdogModels(osType);
 
         vmInitEnabledChanged();
-
     }
 
     private void updateWatchdogModels() {
@@ -2366,6 +2289,7 @@
 
     public boolean validate() {
         getDataCenterWithClustersList().validateSelectedItem(new IValidation[] 
{ new NotEmptyValidation() });
+        setIsSystemTabValid(true);
         getMemSize().validateEntity(new IValidation[] { new 
ByteSizeValidation() });
         getMinAllocatedMemory().validateEntity(new IValidation[] { new 
ByteSizeValidation() });
         getOSType().validateSelectedItem(new NotEmptyValidation[] { new 
NotEmptyValidation() });
@@ -2402,21 +2326,10 @@
                             new SpecialAsciiI18NOrNoneValidation()
                     });
 
-            AsyncQuery asyncQuery = new AsyncQuery();
-            asyncQuery.setModel(this);
-            asyncQuery.asyncCallback = new INewAsyncCallback() {
-                @Override
-                public void onSuccess(Object model, Object returnValue) {
-                    validateMemorySize(getMemSize(), 
(Integer)((VdcQueryReturnValue)returnValue).getReturnValue(), _minMemSize);
-                    if (!(((UnitVmModel)model).getBehavior() instanceof 
TemplateVmModelBehavior)) {
-                        // Minimum 'Physical Memory Guaranteed' is 1MB
-                        validateMemorySize(getMinAllocatedMemory(), 
getMemSize().getEntity(), 1);
-                    }
-                }
-            };
-
-            if (getSelectedCluster() != null) {
-                AsyncDataProvider.getOsMaxRam(osType, 
getSelectedCluster().getcompatibility_version(), asyncQuery);
+            // Minimum 'Physical Memory Guaranteed' is 1MB
+            validateMemorySize(getMemSize(), Integer.MAX_VALUE, 1);
+            if (!(getBehavior() instanceof TemplateVmModelBehavior) && 
getMemSize().getIsValid()) {
+                validateMemorySize(getMinAllocatedMemory(), 
getMemSize().getEntity(), 1);
             }
 
             getComment().validateEntity(new IValidation[] { new 
SpecialAsciiI18NOrNoneValidation() });
@@ -2485,7 +2398,7 @@
         boolean behaviorValid = behavior.validate();
         setIsGeneralTabValid(getName().getIsValid() && 
getDescription().getIsValid() && getComment().getIsValid()
                 && getDataCenterWithClustersList().getIsValid()
-                && getTemplate().getIsValid() && getMemSize().getIsValid()
+                && getTemplate().getIsValid()
                 && getMinAllocatedMemory().getIsValid());
 
         setIsFirstRunTabValid(getDomain().getIsValid() && 
getTimeZone().getIsValid());
@@ -2496,9 +2409,13 @@
         setIsBootSequenceTabValid(getCdImage().getIsValid() && 
getKernel_path().getIsValid());
         setIsCustomPropertiesTabValid(customPropertySheetValid);
 
+        setIsSystemTabValid(getMemSize().getIsValid() &&
+                            getTotalCPUCores().getIsValid() &&
+                            
getSerialNumberPolicy().getCustomSerialNumber().getIsValid());
+
         return getName().getIsValid() && getDescription().getIsValid() && 
getDataCenterWithClustersList().getIsValid()
                 && getDisksAllocationModel().getIsValid() && 
getTemplate().getIsValid() && getComment().getIsValid()
-                && getDefaultHost().getIsValid() && getMemSize().getIsValid() 
&& getMinAllocatedMemory().getIsValid()
+                && getDefaultHost().getIsValid() && 
getMinAllocatedMemory().getIsValid()
                 && getNumOfMonitors().getIsValid() && getDomain().getIsValid() 
&& getUsbPolicy().getIsValid()
                 && getTimeZone().getIsValid() && getOSType().getIsValid() && 
getCdImage().getIsValid()
                 && getKernel_path().getIsValid()
@@ -2508,7 +2425,7 @@
                 && behaviorValid
                 && customPropertySheetValid && getQuota().getIsValid()
                 && getMigrationDowntime().getIsValid()
-                && 
getSerialNumberPolicy().getCustomSerialNumber().getIsValid();
+                && getIsSystemTabValid();
 
     }
 
@@ -2539,11 +2456,11 @@
 
     }
 
-    private void validateMemorySize(EntityModel model, int maxMemSize, int 
minMemSize)
+    private void validateMemorySize(EntityModel<Integer> model, int 
maxMemSize, int minMemSize)
     {
         boolean isValid = false;
 
-        int memSize = (Integer) model.getEntity();
+        int memSize = model.getEntity();
 
         if (memSize == 0)
         {
diff --git 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/ByteSizeValidation.java
 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/ByteSizeValidation.java
index a6a92a4..87c8d2b 100644
--- 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/ByteSizeValidation.java
+++ 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/ByteSizeValidation.java
@@ -5,7 +5,7 @@
 {
     public ByteSizeValidation()
     {
-        setExpression("^\\d+\\s*(m|mb|g|gb){0,1}\\s*$"); //$NON-NLS-1$
+        setExpression("^\\d+\\s*(m|mb|g|gb|t|tb){0,1}\\s*$"); //$NON-NLS-1$
         setMessage("TODO:"); //$NON-NLS-1$
     }
 }


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I33761ac7c5491c17fff928de13fdf16065d47e1e
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Betak <mbe...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to