Martin Mucha has posted comments on this change. Change subject: core: refactored 2 overgrown copy-pasted methods ......................................................................
Patch Set 4: (1 comment) https://gerrit.ovirt.org/#/c/41273/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/IsoDomainListSyncronizer.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/IsoDomainListSyncronizer.java: Line 594: repoStorageDomainId, Line 595: imageType, Line 596: e.getMessage()); Line 597: log.debug("Exception", e); Line 598: return false; > you moved the exception handling to here, but I see that updateIsoListFromV First: I may not understand question. This code is in method refreshIsoFileListMetaData. This method is called from: updateIsoListFromVDSM --> refreshVdsmFileList --> refreshIsoFileListMetaData or updateFloppyListFromVDSM --> refreshVdsmFileList --> refreshIsoFileListMetaData that being said, it's not possible for refreshIsoFileListMetaData to call updateIsoListFromVDSM; that would create infinite loop. Maybe you did notice that there's refreshIsoFileListMetaData called also for Floppies. Yes, it's wrong and it's symptom of someone was copypasting. ——— I'll try to describe what I did here: note: I'll try to explain my original intention here, but I can be wrong in following explanation, since I work on so many stuff simultaneously that I usually cannot remember how to get out of building in the evening. I'll try to answer best I can, but please don't trust me that much. I think that I did not 'move' exception handling to here. I added this one as a new one. Trying to understand former code again, I can see two unrelated calls — one to DB one to VDSM, with overly generic exception handling and complaining, that call to VDSM failed, which can be totally wrong. So what I did is (I believe): • I broke overgrown method to few smaller with sole purpose. • update exceptions being thrown, so that exception is thrown in each failure as in former case, but this exception is not related to VDSM (this was the case of wrong exception in past), notice that in exception there's no VDSM mentioned, this exception is related to failure persisting data obtained from VDSM into DB. Line 599: } finally { Line 600: syncObject.unlock(); Line 601: } Line 602: } -- To view, visit https://gerrit.ovirt.org/41273 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ide9f1559b7b36f2052c1beecc05bbc60b1c52a8b Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <[email protected]> Gerrit-Reviewer: Arik Hadas <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Mucha <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
