Omer Frenkel has posted comments on this change.

Change subject: core, db: add logical_name to vm_device
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.ovirt.org/#/c/31754/2//COMMIT_MSG
Commit Message:

Line 4: Commit:     Liron Aravot <[email protected]>
Line 5: CommitDate: 2014-08-21 04:45:21 -0400
Line 6: 
Line 7: core, db: add logical_name to vm_device
Line 8: 
this is where you suppose to explain what the patch does, and why it is needed..
Line 9: Change-Id: If9b5a8e771a05c1fe44834adfe998cf921f6ae52
Line 10: Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1063597


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 cases 
the ctor is called, so it might be better to have an overload for it.. it would 
also make this change a lot smaller and focused on the change.
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?


-- 
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: 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

Reply via email to