Liron Aravot has posted comments on this change. Change subject: core: Improve attach process when register disks ......................................................................
Patch Set 8: (1 comment) http://gerrit.ovirt.org/#/c/33154/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 273: result); Line 274: addOvfStoreDiskToDomain(ovfStoreDiskImage); Line 275: } Line 276: } Line 277: I disagree with the approach taken in this patch, There's no need to copy the existing logic that already implemented in GetUnregisteredDiskQuery and to maintain that code in two different places. What you want to achieve to avoid the log mentioned in the bug can be accomplished by passing a parameter to the query or by filtering the output. AttachStorageDomainToPool is "rare" operation, there's no need to save one/two operations at that cost, even if you do want to save stuff let's do that with passed parameters. Line 278: private List<Disk> getUnregisteredDisks() { Line 279: Guid storagePoolId = getVds().getStoragePoolId(); Line 280: Guid storageDomainId = getParameters().getStorageDomainId(); Line 281: List<Disk> unregisteredDisks = new ArrayList<Disk>(); -- To view, visit http://gerrit.ovirt.org/33154 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If302d623ab13217776e0cb6ec49dda9cc59b7b39 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: Idan Shaby <[email protected]> Gerrit-Reviewer: Liron Aravot <[email protected]> Gerrit-Reviewer: Maor Lipchuk <[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
