Vered Volansky has posted comments on this change.

Change subject: core: Report storage size closer to reality
......................................................................


Patch Set 8:

(5 comments)

https://gerrit.ovirt.org/#/c/37428/8/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/StorageDomain.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/StorageDomain.java:

Line 202: 
Line 203:         if (available != null && used != null) {
Line 204:             setTotalDiskSize(available + used);
Line 205:         } else {
Line 206:             setTotalDiskSize(null); // GREGM prevents NPEs
> Remove the comment please as it's not relevant anymore
Done
Line 207:         }
Line 208:     }
Line 209: 
Line 210:     private Integer totalDiskSize;


https://gerrit.ovirt.org/#/c/37428/8/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/column/AbstractStorageSizeColumn.java
File 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/column/AbstractStorageSizeColumn.java:

Line 5: 
Line 6: public abstract class AbstractStorageSizeColumn<T> extends 
AbstractSizeColumn<T> {
Line 7: 
Line 8:     public AbstractStorageSizeColumn() {
Line 9:         super(new DiskSizeRenderer<Long>(SizeConverter.SizeUnit.GB));/*{
> Why is the inner block commented out? I guess the original intention was to
Done
Line 10: 
Line 11:             @Override
Line 12:             protected boolean isUnavailable(Long size) {
Line 13:                 return size == null || size.longValue() == 0;


https://gerrit.ovirt.org/#/c/37428/8/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/MainTabStorageView.java
File 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/MainTabStorageView.java:

Line 112:         AbstractStorageSizeColumn<StorageDomain> totalSpaceColumn = 
new AbstractStorageSizeColumn<StorageDomain>() {
Line 113:             @Override
Line 114:             public Long getRawValue(StorageDomain object) {
Line 115:                 Integer totalSpace = object.getTotalDiskSize();
Line 116:                 return totalSpace == null ? null : (long) totalSpace;
> Let's save another useless conversion - please use Long.valueOf(totalSpace)
Done
Line 117:             }
Line 118:         };
Line 119:         getTable().addColumn(totalSpaceColumn, 
constants.totalSpaceStorage(), "130px"); //$NON-NLS-1$
Line 120: 


Line 121:         AbstractStorageSizeColumn<StorageDomain> freeSpaceColumn = 
new AbstractStorageSizeColumn<StorageDomain>() {
Line 122:             @Override
Line 123:             public Long getRawValue(StorageDomain object) {
Line 124:                 Integer availableDiskSize = 
object.getAvailableDiskSize();
Line 125:                 return availableDiskSize == null ? null :(long) 
availableDiskSize;
> same here
Done
Line 126:             }
Line 127:         };
Line 128:         
freeSpaceColumn.makeSortable(StorageDomainFieldAutoCompleter.SIZE);
Line 129:         getTable().addColumn(freeSpaceColumn, 
constants.freeSpaceStorage(), "130px"); //$NON-NLS-1$


https://gerrit.ovirt.org/#/c/37428/8/packaging/dbscripts/upgrade/03_06_1070_set_glance_sotrage_size_to_NA.sql
File packaging/dbscripts/upgrade/03_06_1070_set_glance_sotrage_size_to_NA.sql:

Line 2: SET    available_disk_size = NULL, used_disk_size = NULL
Line 3: FROM   storage_domain_static, providers
Line 4: WHERE  storage_domain_dynamic.id = storage_domain_static.id AND
Line 5:        storage_domain_static.storage = CAST(providers.id AS VARCHAR) AND
Line 6:        provider_type = 'OPENSTACK_IMAGE';_
> * You have a redundant "_" at the end
A. Done.
B. What's the issue with Cinder? I noticed the issue only in Glance.


-- 
To view, visit https://gerrit.ovirt.org/37428
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I758febd3b6b1a1f13ae4e48635bf185ea81d918b
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Vered Volansky <vvola...@redhat.com>
Gerrit-Reviewer: Ala Hino <ah...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Amit Aviram <aavi...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Freddy Rolland <froll...@redhat.com>
Gerrit-Reviewer: Idan Shaby <ish...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@redhat.com>
Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to