Maor Lipchuk has posted comments on this change. Change subject: core: avoid broken status on snapshot removal ......................................................................
Patch Set 6: (4 comments) http://gerrit.ovirt.org/#/c/27611/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java: Line 192: getSnapshotDao().remove(getParameters().getSnapshotId()); Line 193: } else { Line 194: List<String> failedToRemoveDisks = new ArrayList<>(); Line 195: Line 196: // Remove successfully deleted images from the snapshot The comment for successfully deleted images from the snapshot should be at line 200 Line 197: for (VdcActionParametersBase parametersBase : getParameters().getImagesParameters()) { Line 198: ImagesContainterParametersBase imagesParams = (ImagesContainterParametersBase) parametersBase; Line 199: Line 200: if (imagesParams.getTaskGroupSuccess()) { Line 197: for (VdcActionParametersBase parametersBase : getParameters().getImagesParameters()) { Line 198: ImagesContainterParametersBase imagesParams = (ImagesContainterParametersBase) parametersBase; Line 199: Line 200: if (imagesParams.getTaskGroupSuccess()) { Line 201: Snapshot snapshotWithoutImage = ImagesHandler.prepareSnapshotConfigWithoutImageSingleImage( On each disk you are fetching the snapshot from the DB. Since all those disks are only related to one snapshot. you can fetch the snapshot before the for loop and use it when calling prepareSnapshotConfigWithoutImageS ingleImage That way you can only update the snapshot once, after the for loop finishes, and you avoid multiple I/O to the DB (On getting the snapshot and update it every time) Line 202: getParameters().getSnapshotId(), imagesParams.getImageId()); Line 203: getSnapshotDao().update(snapshotWithoutImage); Line 204: } Line 205: else { Line 201: Snapshot snapshotWithoutImage = ImagesHandler.prepareSnapshotConfigWithoutImageSingleImage( Line 202: getParameters().getSnapshotId(), imagesParams.getImageId()); Line 203: getSnapshotDao().update(snapshotWithoutImage); Line 204: } Line 205: else { please use one liner for else, same in line 191 Line 206: log.errorFormat("Could not delete image {0} from snapshot {1}", Line 207: getParameters().getSnapshotId(), imagesParams.getImageId()); Line 208: Line 209: DiskImage diskImage = getDiskImageDao().getSnapshotById(imagesParams.getImageId()); Line 202: getParameters().getSnapshotId(), imagesParams.getImageId()); Line 203: getSnapshotDao().update(snapshotWithoutImage); Line 204: } Line 205: else { Line 206: log.errorFormat("Could not delete image {0} from snapshot {1}", 1) you mismatched the parameters here. {0} should be the image id. {1} should be the snapshot id 2) Please also indicate the snapshot name as well (Maybe also the disk_alias if you can move the log after line 210) Line 207: getParameters().getSnapshotId(), imagesParams.getImageId()); Line 208: Line 209: DiskImage diskImage = getDiskImageDao().getSnapshotById(imagesParams.getImageId()); Line 210: failedToRemoveDisks.add(diskImage.getDiskAlias()); -- To view, visit http://gerrit.ovirt.org/27611 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2a15034185ef30f53fbeaa46440cd92954d39eee Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Daniel Erez <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Liron Ar <[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
