Liron Aravot has posted comments on this change. Change subject: core, db: add logical_name to vm_device ......................................................................
Patch Set 2: (2 comments) http://gerrit.ovirt.org/#/c/31754/2/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmDevice.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmDevice.java: Line 90: isPlugged = Boolean.FALSE; Line 91: alias = ""; Line 92: } Line 93: Line 94: public VmDevice(VmDeviceId id, VmDeviceGeneralType type, String device, String address, > because this field only relevant for disks, it is null by default in most c I see your point, I added it to this ctor to align with the 'convention' here and adding a separate ctor might be irritating when more fields would be added. if you don't mind i prefer to keep it as is. Line 95: int bootOrder, Line 96: Map<String, Object> specParams, Line 97: boolean isManaged, Line 98: Boolean isPlugged, http://gerrit.ovirt.org/#/c/31754/2/packaging/dbscripts/upgrade/03_06_0240_add_logical_name_to_vm_device.sql File packaging/dbscripts/upgrade/03_06_0240_add_logical_name_to_vm_device.sql: Line 1: select fn_db_add_column('vm_device', 'logical_name', 'VARCHAR(30) '); > you have extra space after (30), and why just 30? is it enough for any os? 1. I've removed the space. 2. 30 seems enough to me, if it'll be needed we can increase it. any size that you see as more appropriate? -- To view, visit http://gerrit.ovirt.org/31754 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If9b5a8e771a05c1fe44834adfe998cf921f6ae52 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liron Aravot <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Liron Aravot <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
