Liron Ar has posted comments on this change.

Change subject: core: Add storage types to DiskImage entity
......................................................................


Patch Set 5:

(4 comments)

http://gerrit.ovirt.org/#/c/23410/5/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/DiskImage.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/DiskImage.java:

Line 203:         getImage().setLastModified(lastModified);
Line 204:     }
Line 205: 
Line 206:     private ArrayList<Guid> storageIds;
Line 207:     private List<StorageType> storageTypes;
1. add @JsonIgnore or it'll fail on json serialization.

2. it should be a set.
Line 208: 
Line 209:     public ArrayList<Guid> getStorageIds() {
Line 210:         return storageIds;
Line 211:     }


http://gerrit.ovirt.org/#/c/23410/5/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskImageDAODbFacadeImpl.java
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskImageDAODbFacadeImpl.java:

Line 187:             if (splitTypes == null) {
Line 188:                 return null;
Line 189:             }
Line 190: 
Line 191:             List<StorageType> types = new 
ArrayList<>(splitTypes.size());
no need in passing the size here
Line 192:             for (String typeStr : splitTypes) {
Line 193:                 try {
Line 194:                     
types.add(StorageType.forValue(Integer.parseInt(typeStr)));
Line 195:                 }


Line 193:                 try {
Line 194:                     
types.add(StorageType.forValue(Integer.parseInt(typeStr)));
Line 195:                 }
Line 196:                 catch (NumberFormatException e) {
Line 197:                     continue;
we should throw an exception here rather than continue.
Line 198:                 }
Line 199:             }
Line 200:             return types;
Line 201:         }


http://gerrit.ovirt.org/#/c/23410/5/packaging/dbscripts/create_views.sql
File packaging/dbscripts/create_views.sql:

Line 167:     SELECT 0 AS disk_storage_type,
Line 168:            storage_for_image_view.storage_id as storage_id, -- 
Storage fields
Line 169:            storage_for_image_view.storage_path as storage_path,
Line 170:            storage_for_image_view.storage_name as storage_name,
Line 171:            storage_for_image_view.storage_type as storage_type,
you should also add it to all_disks view otherwise you will fail in the engine 
there as the column won't be returned.
Line 172:            storage_pool_id,
Line 173:            image_guid, -- Image fields
Line 174:            creation_date,
Line 175:            actual_size,


-- 
To view, visit http://gerrit.ovirt.org/23410
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8302972bb21fa076cd35fb308f5f660ca9200daa
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Tal Nisan <[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