Liron Aravot has posted comments on this change. Change subject: core: Initialize entities in DB from OVF disk on attach ......................................................................
Patch Set 6: (11 comments) http://gerrit.ovirt.org/#/c/29041/6/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 172: } Line 173: } Line 174: Line 175: protected List<OvfEntityData> getEntitiesFromStorageOvfDisk() { Line 176: List<OvfEntityData> ovfEntitiesFromTar = new ArrayList<>(); initialize it to Collections.emptyList(). Line 177: Line 178: // Get all unregistered disks. Line 179: List<Disk> unregisteredDisks = getBackend().runInternalQuery(VdcQueryType.GetUnregisteredDisks, Line 180: new GetUnregisteredDisksQueryParameters(getParameters().getStorageDomainId(), Line 208: * - A list of disks Line 209: * @return A Pair which contains the best OVF disk to retrieve data from and its size. Line 210: */ Line 211: private Pair<DiskImage, Long> getLatestOVFDisk(List<Disk> disks) { Line 212: Date lastUpdate = new Date(); rename to foundOvfDiskUpdateDate Line 213: boolean lastIsUpdated = false; Line 214: Long size = 0L; Line 215: Disk diskToGetOVF = null; Line 216: for (Disk disk : disks) { Line 209: * @return A Pair which contains the best OVF disk to retrieve data from and its size. Line 210: */ Line 211: private Pair<DiskImage, Long> getLatestOVFDisk(List<Disk> disks) { Line 212: Date lastUpdate = new Date(); Line 213: boolean lastIsUpdated = false; rename to isFoundOvfDiskUpdated Line 214: Long size = 0L; Line 215: Disk diskToGetOVF = null; Line 216: for (Disk disk : disks) { Line 217: boolean isDiskLastUpdated = false; Line 211: private Pair<DiskImage, Long> getLatestOVFDisk(List<Disk> disks) { Line 212: Date lastUpdate = new Date(); Line 213: boolean lastIsUpdated = false; Line 214: Long size = 0L; Line 215: Disk diskToGetOVF = null; rename to ovfDisk Line 216: for (Disk disk : disks) { Line 217: boolean isDiskLastUpdated = false; Line 218: // Check which disks are of OVF_STORE Line 219: String diskDecription = ((DiskImage) disk).getDescription(); Line 213: boolean lastIsUpdated = false; Line 214: Long size = 0L; Line 215: Disk diskToGetOVF = null; Line 216: for (Disk disk : disks) { Line 217: boolean isDiskLastUpdated = false; please rename to isBetterOvfDiskFound. Line 218: // Check which disks are of OVF_STORE Line 219: String diskDecription = ((DiskImage) disk).getDescription(); Line 220: if (diskDecription.contains(OvfInfoFileConstants.OvfStoreDescriptionLabel)) { Line 221: Map<String, Object> diskDescriptionMap = buildJson(diskDecription); Line 217: boolean isDiskLastUpdated = false; Line 218: // Check which disks are of OVF_STORE Line 219: String diskDecription = ((DiskImage) disk).getDescription(); Line 220: if (diskDecription.contains(OvfInfoFileConstants.OvfStoreDescriptionLabel)) { Line 221: Map<String, Object> diskDescriptionMap = buildJson(diskDecription); if it fails you should continue, catch the exception and add a log in warn Line 222: if (!isDomainExistsInDiskDescription(diskDescriptionMap, getParameters().getStorageDomainId())) { Line 223: break; Line 224: } Line 225: Line 218: // Check which disks are of OVF_STORE Line 219: String diskDecription = ((DiskImage) disk).getDescription(); Line 220: if (diskDecription.contains(OvfInfoFileConstants.OvfStoreDescriptionLabel)) { Line 221: Map<String, Object> diskDescriptionMap = buildJson(diskDecription); Line 222: if (!isDomainExistsInDiskDescription(diskDescriptionMap, getParameters().getStorageDomainId())) { please add a comment to clarify that the purpose of this check is to verify that it's an ovf store with data related to this domain Line 223: break; Line 224: } Line 225: Line 226: boolean isUpdated = Boolean.valueOf(diskDescriptionMap.get(OvfInfoFileConstants.IsUpdated).toString()); Line 219: String diskDecription = ((DiskImage) disk).getDescription(); Line 220: if (diskDecription.contains(OvfInfoFileConstants.OvfStoreDescriptionLabel)) { Line 221: Map<String, Object> diskDescriptionMap = buildJson(diskDecription); Line 222: if (!isDomainExistsInDiskDescription(diskDescriptionMap, getParameters().getStorageDomainId())) { Line 223: break; why break? continue. Line 224: } Line 225: Line 226: boolean isUpdated = Boolean.valueOf(diskDescriptionMap.get(OvfInfoFileConstants.IsUpdated).toString()); Line 227: Date date = getDateFromDiskDescription(diskDescriptionMap); Line 225: Line 226: boolean isUpdated = Boolean.valueOf(diskDescriptionMap.get(OvfInfoFileConstants.IsUpdated).toString()); Line 227: Date date = getDateFromDiskDescription(diskDescriptionMap); Line 228: // If last is updated is false, and the current disk was updated, then the current disk is what we are Line 229: // about to update from. all the checks between 229-238 can be replaced with - if (lastIsUpdated && !isUpdated) { continue; } if (date.after(lastupdate)) { !!!! new disk was found) !!!! } Line 230: if (!lastIsUpdated && isUpdated) { Line 231: isDiskLastUpdated = true; Line 232: // If also the current disk was not updated, then check which disk has the latest update date. Line 233: } else if (!isUpdated && date.after(lastUpdate)) { Line 253: return new SimpleDateFormat(OvfParser.formatStrFromDiskDescription).parse(map.get(OvfInfoFileConstants.LastUpdated) Line 254: .toString()); Line 255: } catch (java.text.ParseException e) { Line 256: log.errorFormat("LastUpdate Date could not be parsed from disk desscription"); Line 257: e.printStackTrace(); log the exception in debug. Line 258: return null; Line 259: } Line 260: } Line 261: Line 262: private boolean isDomainExistsInDiskDescription(Map<String, Object> map, Guid storageDomainId) { Line 263: return map.get(OvfInfoFileConstants.Domains).toString().contains(storageDomainId.toString()); Line 264: } Line 265: Line 266: private Map<String, Object> buildJson(String json) { use the helper for that, you have that exact method there Line 267: try { Line 268: return JsonHelper.jsonToMap(json); Line 269: } catch (IOException e) { Line 270: throw new RuntimeException("Exception while generating json containing ovf store info", e); -- 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: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@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
