Allon Mureinik has posted comments on this change.
Change subject: engine: Remove getImageDomainsList API
......................................................................
Patch Set 2: I would prefer that you didn't submit this
(4 inline comments)
agree with all of Alissa's comment.
see a couple more inline.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java
Line 93:
Line 94: protected ImportVmCommand(Guid commandId) {
Line 95: super(commandId);
Line 96: }
Line 97:
Moving this here has nothing to do with the patch.
Please separate cleanups and logic to different patches.
Line 98: public ImportVmCommand(ImportVmParameters parameters) {
Line 99: super(parameters);
Line 100: setVmId(parameters.getContainerId());
Line 101: setVm(parameters.getVm());
Line 444: if (!getParameters().isImportAsNewEntity() &&
atLeastOneImageFoundInStorageDomain(imageList,
Line 445: getParameters().getDestDomainId())) {
Line 446: return
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_ALREADY_CONTAINS_DISK);
Line 447: }
Line 448: } catch (RuntimeException e) {
At the very least you should return false here.
Line 449: // There is nothing to do in case of VDSM failure to
retrieve images from the given storage domain
Line 450: // except of writing the log file. The command itself
will probably fail during the execution
Line 451: // and then user will get meaningful error message
Line 452: log.error("Failed to validate if target storage domain
already contains one of the template disks! " +
Line 713: getImageDao().update(disk.getImage());
Line 714: }
Line 715:
Line 716: /**
Line 717: * Generates and saves a {@link DiskImageDynamic} for the given
disk.
This cleanup has nothing to do with the patch - please separate logic and
cleanups
Line 718: * @param disk The imported disk
Line 719: **/
Line 720: protected void saveDiskImageDynamic(DiskImage disk) {
Line 721: DiskImageDynamic diskDynamic = new DiskImageDynamic();
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyTemplateCommand.java
Line 142: return
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_POOL_NOT_MATCH);
Line 143: }
Line 144: }
Line 145: return true;
Line 146: }
great cleanup, but again - how is it relevant to this patch?
Line 147:
Line 148: @Override
Line 149: protected void setActionMessageParameters() {
Line 150: if (getMoveOrCopyImageOperation() == ImageOperation.Move) {
--
To view, visit http://gerrit.ovirt.org/15951
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I33069871db34e8b129c4d80098523654cd2a0629
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Eduardo <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: liron aravot <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
