Nir Soffer has posted comments on this change. Change subject: engine: Display correct and user friendly size of iso files ......................................................................
Patch Set 4: Code-Review-1 (4 comments) Same issue as in 19550 - when file size is not available, return value that means that, not valid file size. See the inline comments. .................................................... Commit Message Line 3: AuthorDate: 2013-09-24 14:09:54 +0300 Line 4: Commit: Sergey Gotliv <[email protected]> Line 5: CommitDate: 2013-09-30 09:26:24 +0300 Line 6: Line 7: engine: Display correct and user friendly size of iso files Because you separated the patch that display human readable size, the "user friendly" part can be removed. Line 8: Line 9: Previously all iso files appear under Storage -> Images with bogus size Line 10: of '< 1 GB' although some of them are larger than that. It happened Line 11: because the 'getIsoList' API between Engine and VDSM returned only file Line 8: Line 9: Previously all iso files appear under Storage -> Images with bogus size Line 10: of '< 1 GB' although some of them are larger than that. It happened Line 11: because the 'getIsoList' API between Engine and VDSM returned only file Line 12: names, so Engine had no choice but to set their size to 0. Engine had many choices, but it chose a bad one. Line 13: Line 14: Now this API returns all information VDSM keeps about these files Line 15: including their size so Engine can display the correct value. Line 16: .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/IsoDomainListSyncronizer.java Line 59: public class IsoDomainListSyncronizer { Line 60: private static final Log log = LogFactory.getLog(IsoDomainListSyncronizer.class); Line 61: private List<RepoImage> problematicRepoFileList = new ArrayList<RepoImage>(); Line 62: private static final int MIN_TO_MILLISECONDS = 60 * 1000; Line 63: private static final long DEFAULT_ISO_FILE_SIZE = 0; This value is valid file size and cannot be used as default. The constants should be renamed to SIZE_NOT_AVAILABLE and the value should be impossible value such as -1. Line 64: private static IsoDomainListSyncronizer isoDomainListSyncronizer; Line 65: private static final ConcurrentMap<Object, Lock> syncDomainForFileTypeMap = new ConcurrentHashMap<Object, Lock>(); Line 66: private int isoDomainRefreshRate; Line 67: RepoFileMetaDataDAO repoStorageDom; Line 585: } catch (NumberFormatException e) { Line 586: // Default file size(for backward compatibility) is used if for some reason VDSM will return Line 587: // non valid number as a size of the iso file Line 588: log.errorFormat("File's '{0}' size is illegal number", isoFileWithMetadata.getKey(), e); Line 589: return DEFAULT_ISO_FILE_SIZE; Should return value that means "size is not available", and not a default. Line 590: } Line 591: } Line 592: Line 593: /** -- 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: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Sergey Gotliv <[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
