Maor Lipchuk has posted comments on this change. Change subject: core: support add template with Cinder disks ......................................................................
Patch Set 2: Code-Review+1 (2 comments) minor log comment https://gerrit.ovirt.org/#/c/39645/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java: Line 286: @Override Line 287: public Void runInTransaction() { Line 288: addPermission(); Line 289: addVmTemplateImages(srcDeviceIdToTargetDeviceIdMapping); Line 290: if (!addVmTemplateCinderDisks(srcDeviceIdToTargetDeviceIdMapping)) { Suggestion: Maybe To make the process more readable I would just extract this code here: List<CinderDisk> cinderDisks ImagesHandler.filterDisksBasedOnCinder(getVm().getDiskMap().values()); if (!cinderDisks.isEmpty() && !addVmTemplateCinderDisks(srcDeviceIdToTargetDeviceIdMapping)) { .... Line 291: setSucceeded(false); Line 292: return null; Line 293: } Line 294: addVmInterfaces(srcDeviceIdToTargetDeviceIdMapping); Line 722: log.error("Error cloning Cinder disks for template"); Line 723: return false; Line 724: } Line 725: } catch (InterruptedException | ExecutionException e) { Line 726: log.error("Error cloning Cinder disks for template"); Please also print the error Line 727: return false; Line 728: } Line 729: } Line 730: return true; -- To view, visit https://gerrit.ovirt.org/39645 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0390560c3e790c2435216382e586bcbd3a57d4bb Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Daniel Erez <[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
