Liron Aravot has posted comments on this change. Change subject: core: Initialize entities in DB from OVF disk on attach ......................................................................
Patch Set 8: (3 comments) http://gerrit.ovirt.org/#/c/29041/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommand.java: Line 219: Map<String, Object> diskDescriptionMap; Line 220: try { Line 221: diskDescriptionMap = JsonHelper.jsonToMap(diskDecription); Line 222: } catch (IOException e) { Line 223: log.errorFormat("Exception while generating json containing ovf store info. Exception: {0}", e); when you rebase i'd change it to Info as it's not error, it's not always expected to have json description so error might be confusing - but that's not an issue. Line 224: continue; Line 225: } Line 226: Line 227: // The purpose of this check is to verify that it's an OVF store with data related to the Storage Domain. Line 238: if (!isFoundOvfDiskUpdated && isUpdated) { Line 239: isBetterOvfDiskFound = true; Line 240: // If also the current disk was not updated, then check which disk has the latest update date. Line 241: } else if (!isUpdated && date.after(foundOvfDiskUpdateDate)) { Line 242: isBetterOvfDiskFound = true; this if is wrong, if the disk isn't updated (!isUpdated) and it's date is later than the found disk it shouldn't be replaced with it as we can't tell what's on it. Line 243: // If also the current disk was updated, then check which disk has the latest update date. Line 244: } else if (isFoundOvfDiskUpdated && isUpdated && date.after(foundOvfDiskUpdateDate)) { Line 245: isBetterOvfDiskFound = true; Line 246: } Line 240: // If also the current disk was not updated, then check which disk has the latest update date. Line 241: } else if (!isUpdated && date.after(foundOvfDiskUpdateDate)) { Line 242: isBetterOvfDiskFound = true; Line 243: // If also the current disk was updated, then check which disk has the latest update date. Line 244: } else if (isFoundOvfDiskUpdated && isUpdated && date.after(foundOvfDiskUpdateDate)) { isFoundOvfDiskUpdated isn't relevant here, if the new disk isUpdated and it's date is newer it should always be selected. I suggested in patchset 6 a simpler solution and it should be changed to that (patch 6) rather than fixing those checks. Line 245: isBetterOvfDiskFound = true; Line 246: } Line 247: Line 248: if (isBetterOvfDiskFound) { -- To view, visit http://gerrit.ovirt.org/29041 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ica68974916821ac1c9fbe6f43400170d19d716c9 Gerrit-PatchSet: 8 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Liron Aravot <[email protected]> Gerrit-Reviewer: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Tal Nisan <[email protected]> Gerrit-Reviewer: [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
