Allon Mureinik has posted comments on this change. Change subject: core: Improve attach process when register disks ......................................................................
Patch Set 1: (6 comments) http://gerrit.ovirt.org/#/c/33154/1//COMMIT_MSG Commit Message: Line 5: CommitDate: 2014-09-22 00:59:29 +0300 Line 6: Line 7: core: Improve attach process when register disks Line 8: Line 9: Remove redundant failure log when disk is failed to be registered s/disk is failed to be registered/failed to register disk/ Line 10: as part of the attach process. Line 11: Disks will not be registered if they have snapshots, for example, Line 12: although the attach process will still succeed and should not be related Line 13: to this. Line 12: although the attach process will still succeed and should not be related Line 13: to this. Line 14: Line 15: As part of the fix, there is also an improvement of the images registration Line 16: flow, this are the improvements: As part of the fix, there are also several improvement to the images registration flow: Line 17: 1. Avoid redundant validation for Storage Domain availability, since it is Line 18: already being validated in the CDA Line 19: 2. Avoid feching for existing images in the Storage Domain, since the Storage Domain was only Line 20: attached and does not contain any images in the setup yet. Line 16: flow, this are the improvements: Line 17: 1. Avoid redundant validation for Storage Domain availability, since it is Line 18: already being validated in the CDA Line 19: 2. Avoid feching for existing images in the Storage Domain, since the Storage Domain was only Line 20: attached and does not contain any images in the setup yet. But what if it's a template image with several copies? Line 21: 3. Avoid calling two internal query command. Line 22: Line 23: Change-Id: If302d623ab13217776e0cb6ec49dda9cc59b7b39 Line 24: Bug-Url: https://bugzilla.redhat.com/1136808 Line 17: 1. Avoid redundant validation for Storage Domain availability, since it is Line 18: already being validated in the CDA Line 19: 2. Avoid feching for existing images in the Storage Domain, since the Storage Domain was only Line 20: attached and does not contain any images in the setup yet. Line 21: 3. Avoid calling two internal query command. s/command/commands/ Line 22: Line 23: Change-Id: If302d623ab13217776e0cb6ec49dda9cc59b7b39 Line 24: Bug-Url: https://bugzilla.redhat.com/1136808 http://gerrit.ovirt.org/#/c/33154/1/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 290: return unregisteredDisks; Line 291: } Line 292: imagesFromVds = (List<Guid>) vdsReturnValue.getReturnValue(); Line 293: } catch (VdcBLLException e) { Line 294: log.warnFormat("Failed to get a list of images for Storage Domain to register OVF disks. Error: {0}", e); shouldn't we just return here? unregisteredDisks will be empty, and the for would be redundant. Line 295: } Line 296: for (Guid unregisteredDiskId : imagesFromVds) { Line 297: populateUnregisteredDisksListWithUnregisteredDisk(storagePoolId, Line 298: storageDomainId, Line 324: Line 325: // We can't deal with snapshots, so there should only be a single volume associated with the Line 326: // image. If there are multiple volumes, skip the image and move on to the next. Line 327: if (volumesList.size() != 1) { Line 328: log.info("Could not get populated disk since the disk contains snapshots."); add the disk's guid. Line 329: return; Line 330: } Line 331: Line 332: // Get the information about the volume from VDSM. -- 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: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Allon Mureinik <[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
