Allon Mureinik has posted comments on this change.
Change subject: core: WIP: Support for device custom properties
......................................................................
Patch Set 6: (5 inline comments)
I don't quite get this patch - you added a new column and a way to write/read
it from the database, but I don't see it ever passed anywhere.
What am I missing?
....................................................
File backend/manager/dbscripts/upgrade/pre_upgrade/0000_config.sql
Line 522:
Line 523: -- Device Custom Properties
Line 524: select fn_db_add_config_value('SupportCustomDeviceProperties',
'false', '3.0');
Line 525: select fn_db_add_config_value('SupportCustomDeviceProperties',
'false', '3.1');
Line 526: select fn_db_add_config_value('SupportCustomDeviceProperties',
'false', '3.2');
I think you're missing a line here:
select fn_db_add_config_value('SupportCustomDeviceProperties', 'true', '3.3');
Line 527: select fn_db_add_config_value('CustomDeviceProperties', '', '3.3');
Line 528:
Line 529:
------------------------------------------------------------------------------------
Line 530: -- Update with override section
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AttachDiskToVmCommand.java
Line 137: true,
Line 138: getParameters().isPlugUnPlug(),
Line 139: false,
Line 140: "",
Line 141: null);
I'm assuming this is null for future use for when we'll be able to pass disk
custom properties from the UI?
Line 142: }
Line 143:
Line 144: protected void updateBootOrderInVmDevice() {
Line 145:
VmDeviceUtils.updateBootOrderInVmDeviceAndStoreToDB(getVm().getStaticData());
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java
Line 357: true,
Line 358: is_plugged,
Line 359: isReadOnly,
Line 360: "",
Line 361: null);
why hardcoded null?
Don't we want to exp/imp these custom properties?
Line 362: dao.save(managedDevice);
Line 363: // If we add Disk/Interface/CD/Floppy, we have to recalculate
boot order
Line 364: if (type.equals(VmDeviceType.DISK) ||
type.equals(VmDeviceType.INTERFACE )) {
Line 365: // recalculate boot sequence
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/FeatureSupported.java
Line 63: /**
Line 64: * @param version
Line 65: * Compatibility version to check for.
Line 66: * @return <code>true</code> if get hardware information is
supported for the version, <code>false</code> if it's
Line 67: * not.
I'm assuming this newline is a rebase foobar?
Line 68: */
Line 69: public static boolean hardwareInfo(Version version) {
Line 70: return supportedInConfig(ConfigValues.HardwareInfoEnabled,
version);
Line 71: }
....................................................
Commit Message
Line 8:
Line 9: - Adds custom_properties column to vm_device table.
Line 10: - Adds SupportCustomDeviceProperties to vdc_options to support only
Line 11: 3.3+ versions
Line 12: - Adds CustomDeviceProperties to vdc_options to store properties
I don't get this property - shouldn't the custom properties be stored per
device?
Line 13: - Adds corresponding method to FeatureSupported
Line 14:
Line 15: Change-Id: I67ed453706ac75cdc4356cc2d60913d8958c89ed
--
To view, visit http://gerrit.ovirt.org/14309
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I67ed453706ac75cdc4356cc2d60913d8958c89ed
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Peřina <mper...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Martin Peřina <mper...@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
