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