Liron Aravot has posted comments on this change. Change subject: core: Use the latest OVF_STORE files ......................................................................
Patch Set 5: (4 comments) http://gerrit.ovirt.org/#/c/32747/5/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 192: } Line 193: } Line 194: } Line 195: Line 196: protected List<OvfEntityData> getEntitiesFromStorageOvfDisk() { this method name is no longer correct, you also save data here. Line 197: List<OvfEntityData> ovfEntitiesFromTar = Collections.emptyList(); Line 198: // Get all unregistered disks. Line 199: List<Disk> unregisteredDisks = getBackend().runInternalQuery(VdcQueryType.GetUnregisteredDisks, Line 200: new GetUnregisteredDisksQueryParameters(getParameters().getStorageDomainId(), Line 203: List<DiskImage> ovfStoreDiskImages = getAllOVFDisks(unregisteredDisks); Line 204: if (!ovfStoreDiskImages.isEmpty()) { Line 205: registerAllOvfDisks(ovfStoreDiskImages); Line 206: boolean entitiesRetrieved = false; Line 207: while (!ovfStoreDiskImages.isEmpty()) { you need here a for-each loop instead, there's no need to remove from that list which reduces the performance here and has no benefit. Line 208: Pair<DiskImage, Long> ovfDiskAndSize = getLatestOVFDisk(ovfStoreDiskImages); Line 209: DiskImage ovfDisk = ovfDiskAndSize.getFirst(); Line 210: if (ovfDisk != null) { Line 211: if (!entitiesRetrieved) { Line 258: null, Line 259: ovfDisk.getId(), Line 260: StorageDomainOvfInfoStatus.OUTDATED, Line 261: null); Line 262: getDbFacade().getStorageDomainOvfInfoDao().save(storageDomainOvfInfo); have you looked into transactivity of that new addition with the rest of the flow? it seems like there might be different issues here, if you want elaboration let me know. Line 263: } Line 264: Line 265: private void registerAllOvfDisks(List<DiskImage> ovfStoreDiskImages) { Line 266: for (DiskImage ovfStoreDiskImage : ovfStoreDiskImages) { Line 276: } catch (Exception e) { Line 277: result = "failed"; Line 278: log.error(e); Line 279: } Line 280: log.infoFormat("Register new floating OVF_STORE disk with disk id {0} for storage domain {1} has {2}", seems to belong to a different patch Line 281: ovfStoreDiskImage.getId(), Line 282: getParameters().getStorageDomainId(), Line 283: result); Line 284: } -- To view, visit http://gerrit.ovirt.org/32747 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8252281f295df49a28c3c131d9a98fef9af8f35e Gerrit-PatchSet: 5 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: 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
