Moti Asayag has posted comments on this change.
Change subject: core: Adds custom properties to VmDevice
......................................................................
Patch Set 1: (6 inline comments)
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
Line 1335:
Line 1336: @TypeConverterAttribute(Boolean.class)
Line 1337: @DefaultValueAttribute("true")
Line 1338: SupportCustomDeviceProperties(505),
Line 1339:
Is there any reason to prevent from this property to be changed dynamically ?
Line 1340: @TypeConverterAttribute(String.class)
Line 1341: @DefaultValueAttribute("")
Line 1342: CustomDeviceProperties(506),
Line 1343:
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/ConfigurationValues.java
Line 84: MigrationSupportForNativeUsb(ConfigAuthType.User),
Line 85: MigrationNetworkEnabled,
Line 86: VncKeyboardLayout(ConfigAuthType.User),
Line 87: VncKeyboardLayoutValidValues(ConfigAuthType.User),
Line 88: SupportCustomDeviceProperties(ConfigAuthType.User),
you probably meant a question to the community ;-)
Is there a way to prevent a user via the api to provide custom-device
properties ?
Since we don't have permissions on property level, i don't see how we can
prevent this from being invoked via the api.
Line 89: CustomDeviceProperties(ConfigAuthType.User),
Line 90: ;
Line 91:
Line 92: public static enum ConfigAuthType {
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/CustomPropertiesUtils.java
Line 70:
Line 71: private Map<Version, Map<String, Pattern>> predefinedProperties;
Line 72: private Map<Version, Map<String, Pattern>> userdefinedProperties;
Line 73: private Map<Version, String> allVmProperties;
Line 74:
Martin, would you consider extracting the custom device properties parsing and
validation into a separate class ?
It will ease the maintenance of this class and will allow the new
parsing/validation class to be testable which due to the nature of
CustomPropertiesUtils (singleton/static) it is less convenient to do.
Also providing tests for the expected and unsupported device custom properties
will aid the developer on understanding the correct usage and its format.
Line 75: /**
Line 76: * Prefix for device type definition
Line 77: */
Line 78: private static final String TYPE_PREFIX_STR = "\\{type=(";
Line 666: * version of the cluster that the VM containing the
device is in
Line 667: * @param device
Line 668: * specified device
Line 669: * @return map containing all device custom properties
Line 670: */
i haven't found where this method is being used, and also not for the
storeDeviceProperties() method which is called only here.
Line 671: public Map<String, String> getDeviceProperties(Version version,
VmDevice device) {
Line 672: Map<String, String> map = new HashMap<String, String>();
Line 673: storeDeviceProperties(version, device, map);
Line 674: return map;
Line 794: return devicePropValidationPattern.matcher(propDef).matches();
Line 795: }
Line 796:
Line 797: /**
Line 798: * Returns string decribing format custom device properties
specification
s/decribing/describing
Line 799: */
Line 800: public String getCustomDevicePropSpec() {
Line 801: return devicePropSplitStr;
Line 802: }
....................................................
File
backend/manager/tools/src/main/java/org/ovirt/engine/core/config/entity/helper/DeviceCustomPropertiesValueHelper.java
Line 16: boolean result = true;
Line 17: String errMsg = null;
Line 18:
Line 19: // HACK, because ConfigValues are not loaded inside
engine-config
Line 20: Version version = new Version(key.getVersion());
perhaps the condition could be extracted into a method:
if (deviceCustomPropertiesSupported(version)){
...
}
boolean deviceCustomPropertiesSupprted(Version version) {
return Version.v3_3.compareTo(version) > 0;
}
Line 21: if (Version.v3_3.compareTo(version) > 0) {
Line 22: result = false;
Line 23: errMsg = String.format("Device custom properties are not
supported in version %s", version.toString());
Line 24: }
--
To view, visit http://gerrit.ovirt.org/14814
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I07daa5cccbf8e3cbd6b4191bbc8ebf0729d9d9a0
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Peřina <mper...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
