Yair Zaslavsky has posted comments on this change. Change subject: core: Adds custom properties to VmDevice ......................................................................
Patch Set 8: (2 inline comments) .................................................... File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/GetDeviceCustomPropertiesParameters.java Line 16: Line 17: /** Line 18: * Device type to get custom properties for Line 19: */ Line 20: private VmDeviceGeneralType deviceType; Shouldn't this be VmDeviceType ? i.e - arent we interested to get custom properties for bridge, and disk, and not of the general types? What do you think? Line 21: Line 22: public Version getVersion() { Line 23: return version; Line 24: } .................................................... File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/DevicePropertiesUtils.java Line 85: Line 86: /** Line 87: * Map of device custom properties for each version and device type Line 88: */ Line 89: private Map<Version, EnumMap<VmDeviceGeneralType, Map<String, Pattern>>> deviceProperties; Hi, I looked at the wiki - looks like the format - [{type=disk; props={value1=regex1, ..., valueN=regexN}}, {type=interface; props={value1=regex1, ..., valueN=regexN}}] Is based on general type as you did. But I wonder - isn't this a mistake? When I edit a device, it can be cdrom, or a "real" disk. I don't think we should keep for the general types. Same goes I guess for "interface". I am sending an email to dan and assaf for verification of my claim, however - what do you think? Line 90: Line 91: /** Line 92: * List of device types for which custom properties can be set Line 93: */ -- 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: 8 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Martin Peřina <mper...@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