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').
Removed ByteSizeValidation since it didn't validate anything. It
validated only String entities and the memory size is an
EntityModel<Integer>. All validation is performed in validateMemorySize
in conjuction with MemorySizeParser.
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
D
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/ByteSizeValidation.java
5 files changed, 48 insertions(+), 197 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/87/26387/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..b2e3a93 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
@@ -5,12 +5,16 @@
import com.google.gwt.regexp.shared.MatchResult;
import com.google.gwt.regexp.shared.RegExp;
import com.google.gwt.text.shared.Parser;
+import org.ovirt.engine.core.compat.StringHelper;
public class MemorySizeParser implements Parser<Integer> {
@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$
+ if (match == null) {
+ return 0;
+ }
String prefix = match.getGroup(1);
String suffix = match.getGroup(2);
Integer size = null;
@@ -21,16 +25,17 @@
return 0;
}
- if (suffix.equalsIgnoreCase("GB") || suffix.equalsIgnoreCase("G")) {
//$NON-NLS-1$ $NON-NLS-2$
- size *= 1024;
+ if (suffix.equalsIgnoreCase("TB") || suffix.equalsIgnoreCase("T")) {
//$NON-NLS-1$ $NON-NLS-2$
+ return size * 1024 * 1024;
+ } else if (suffix.equalsIgnoreCase("GB") ||
suffix.equalsIgnoreCase("G")) { //$NON-NLS-1$ $NON-NLS-2$
+ return size * 1024;
+ } else if (suffix.equalsIgnoreCase("MB") ||
suffix.equalsIgnoreCase("M")) { //$NON-NLS-1$ $NON-NLS-2$
return size;
+ } else if (StringHelper.isNullOrEmpty(suffix)) {
+ return size;
+ } else {
+ return 0; // disallow garbled suffixes
}
-
- if (suffix.equalsIgnoreCase("MB") || suffix.equalsIgnoreCase("M")) {
//$NON-NLS-1$ $NON-NLS-2$
- return Integer.parseInt(prefix);
- }
-
- 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 11b290b..666082b 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
@@ -1330,6 +1330,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 50f1353..6f9ceee 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
@@ -568,20 +568,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
@@ -638,35 +624,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);
}
@@ -1160,24 +1117,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 9b9b90c..0da83c3 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
@@ -43,7 +43,6 @@
import org.ovirt.engine.ui.uicommonweb.models.SystemTreeItemType;
import org.ovirt.engine.ui.uicommonweb.models.storage.DisksAllocationModel;
import org.ovirt.engine.ui.uicommonweb.models.vms.key_value.KeyValueModel;
-import org.ovirt.engine.ui.uicommonweb.validation.ByteSizeValidation;
import org.ovirt.engine.ui.uicommonweb.validation.I18NNameValidation;
import org.ovirt.engine.ui.uicommonweb.validation.IValidation;
import org.ovirt.engine.ui.uicommonweb.validation.IntegerValidation;
@@ -325,6 +324,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$
}
}
@@ -1176,42 +1188,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() {
@@ -1505,8 +1481,6 @@
initFirstBootDevice();
initNumOfMonitors();
initAllowConsoleReconnect();
- initMinimalVmMemSize();
- initMaximalVmMemSize32OS();
initMigrationMode();
initVncKeyboardLayout();
@@ -1697,7 +1671,8 @@
}
}
- }, getHash()));
+ }, getHash()
+ ));
}
@@ -1750,58 +1725,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 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());
- }
}
private void initDisplayProtocol()
@@ -1893,7 +1816,6 @@
.getQuotaEnforcementType());
}
- updateMaximalVmMemSize();
handleQxlClusterLevel();
updateWatchdogModels();
@@ -1950,7 +1872,6 @@
updateWatchdogModels(osType);
vmInitEnabledChanged();
-
}
private void updateWatchdogModels() {
@@ -2316,8 +2237,7 @@
public boolean validate() {
getDataCenterWithClustersList().validateSelectedItem(new IValidation[]
{ new NotEmptyValidation() });
- getMemSize().validateEntity(new IValidation[] { new
ByteSizeValidation() });
- getMinAllocatedMemory().validateEntity(new IValidation[] { new
ByteSizeValidation() });
+ setIsSystemTabValid(true);
getOSType().validateSelectedItem(new NotEmptyValidation[] { new
NotEmptyValidation() });
DataCenterWithCluster dataCenterWithCluster =
getDataCenterWithClustersList().getSelectedItem();
@@ -2352,21 +2272,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() });
@@ -2429,7 +2338,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());
@@ -2440,9 +2349,11 @@
setIsBootSequenceTabValid(getCdImage().getIsValid() &&
getKernel_path().getIsValid());
setIsCustomPropertiesTabValid(customPropertySheetValid);
+ setIsSystemTabValid(getMemSize().getIsValid() &&
getTotalCPUCores().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()
@@ -2451,7 +2362,8 @@
&& getCpuSharesAmount().getIsValid()
&& behaviorValid
&& customPropertySheetValid && getQuota().getIsValid()
- && getMigrationDowntime().getIsValid();
+ && getMigrationDowntime().getIsValid()
+ && getIsSystemTabValid();
}
@@ -2482,11 +2394,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
deleted file mode 100644
index a6a92a4..0000000
---
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/ByteSizeValidation.java
+++ /dev/null
@@ -1,11 +0,0 @@
-package org.ovirt.engine.ui.uicommonweb.validation;
-
-@SuppressWarnings("unused")
-public class ByteSizeValidation extends RegexValidation
-{
- public ByteSizeValidation()
- {
- setExpression("^\\d+\\s*(m|mb|g|gb){0,1}\\s*$"); //$NON-NLS-1$
- setMessage("TODO:"); //$NON-NLS-1$
- }
-}
--
To view, visit http://gerrit.ovirt.org/26387
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I33761ac7c5491c17fff928de13fdf16065d47e1e
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.4
Gerrit-Owner: Martin Betak <mbe...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
