Sergey Gotliv has posted comments on this change.

Change subject: engine: Display correct and user friendly size of iso files
......................................................................


Patch Set 1:

(9 comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/IsoDomainListSyncronizer.java
Line 545
Line 546
Line 547
Line 548
Line 549
I'll try to explain but I can return it back and fix if you insist.

1. It was wrong even before my change - the method contains more parameters 
than in javadoc and has a returned value which is ignored.
2. It doesn't provide any information that we can't find in the code.


Line 539:     }
Line 540: 
Line 541:     private static boolean refreshIsoFileListMetaData(final Guid 
repoStorageDomainId,
Line 542:                                                       final 
RepoFileMetaDataDAO repoFileMetaDataDao,
Line 543:                                                       final 
Map<String, Map<String, String>> isoDomainList,
Done
Line 544:                                                       final 
ImageFileType imageType) {
Line 545:         Lock syncObject = getSyncObject(repoStorageDomainId, 
imageType);
Line 546:         try {
Line 547:             syncObject.lock();


Line 573: 
Line 574:     private static long retrieveIsoFileSize(Map.Entry<String, 
Map<String, String>> isoFile) {
Line 575:         // old VDSM versions don't provide the metadata for iso files 
therefore engine
Line 576:         // cannot determine the correct size of the iso file.
Line 577:         if (isoFile.getValue().isEmpty() || 
!isoFile.getValue().containsKey("size")) {
Good point! Done.
Line 578:             return 0;
Line 579:         } else {
Line 580:             return Long.valueOf(isoFile.getValue().get("size"));
Line 581:         }


Line 575:         // old VDSM versions don't provide the metadata for iso files 
therefore engine
Line 576:         // cannot determine the correct size of the iso file.
Line 577:         if (isoFile.getValue().isEmpty() || 
!isoFile.getValue().containsKey("size")) {
Line 578:             return 0;
Line 579:         } else {
Done
Line 580:             return Long.valueOf(isoFile.getValue().get("size"));
Line 581:         }
Line 582:     }
Line 583: 


Line 576:         // cannot determine the correct size of the iso file.
Line 577:         if (isoFile.getValue().isEmpty() || 
!isoFile.getValue().containsKey("size")) {
Line 578:             return 0;
Line 579:         } else {
Line 580:             return Long.valueOf(isoFile.getValue().get("size"));
Done
Line 581:         }
Line 582:     }
Line 583: 
Line 584:     /**


Line 633:                         .getResourceManager()
Line 634:                         .RunVdsCommand(VDSCommandType.GetIsoList,
Line 635:                                 new 
IrsBaseVDSCommandParameters(repoStoragePoolId));
Line 636:                 @SuppressWarnings("unchecked")
Line 637:                 Map isoDomainList = (Map) 
returnValue.getReturnValue();
Done
Line 638:                 if (returnValue.getSucceeded() && isoDomainList != 
null) {
Line 639:                     log.debugFormat("The refresh process from VDSM, 
for Iso files succeeded.");
Line 640:                     // Set the Iso domain file list fetched from VDSM 
into the DB.
Line 641:                     refreshIsoSucceeded =


....................................................
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/GetFloppyListVDSCommand.java
Line 14
Line 15
Line 16
Line 17
Line 18
It can't be 'null'. I always initialize 'fileToMetadata' in class 
IsoListReturnForXmlRpc.
This class is a part of this patch.


....................................................
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IsoListReturnForXmlRpc.java
Line 5: import java.util.Map;
Line 6: 
Line 7: public final class IsoListReturnForXmlRpc extends StatusReturnForXmlRpc 
{
Line 8:     private static final String ISO_LIST = "isolist";
Line 9:     private static final String ISO_DICT = "isodict";
It was changed to "fileStats" by now, but review is still in the process.
Line 10: 
Line 11:     private Map<String, Map<String, String>> fileToMetadata;
Line 12: 
Line 13:     public IsoListReturnForXmlRpc(Map<String, Object> innerMap) {


....................................................
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HsmGetIsoListVDSCommand.java
Line 13
Line 14
Line 15
Line 16
Line 17
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I77bd99beb8138524b25f0afdcce0815ad8664f0f
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgot...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@redhat.com>
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