Maor Lipchuk has posted comments on this change. Change subject: core: Support detach Storage Domain with disks. ......................................................................
Patch Set 13: (2 comments) http://gerrit.ovirt.org/#/c/24286/13/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DetachStorageDomainFromPoolCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DetachStorageDomainFromPoolCommand.java: Line 42: protected DetachStorageDomainFromPoolCommand(Guid commandId) { Line 43: super(commandId); Line 44: } Line 45: Line 46: private boolean isChildReturnValueSucceeded(List<VdcReturnValueBase> vdcReturnValues) { > Consider extracting to CommandBase - so it could be reused in MoveDisksComm Should be removed Line 47: for (VdcReturnValueBase vdcReturnValue : vdcReturnValues) { Line 48: getReturnValue().getCanDoActionMessages().addAll(vdcReturnValue.getCanDoActionMessages()); Line 49: getReturnValue().setCanDoAction(getReturnValue().getCanDoAction() && vdcReturnValue.getCanDoAction()); Line 50: } http://gerrit.ovirt.org/#/c/24286/13/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java File frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java: Line 675: Line 676: @DefaultStringValue("Cannot ${action} ${type}. The following VMs are not down: ${vms}.") Line 677: String ACTION_TYPE_FAILED_STORAGE_VMS_NOT_DOWN(); Line 678: Line 679: @DefaultStringValue("Cannot ${action} ${type}. The following VMs are attached to pool: ${vms}.") > Can the pool name be added? it can, but I saw that other pool messages also does not indicate pool name (like VM_CANNOT_SUSPEND_VM_FROM_POOL), for now I wanted to keep it consistent, but I can fix it in this patch if you think it is necesery. Perhaps it can be added in another patch, and also fix the other messages as well. Line 680: String ACTION_TYPE_FAILED_STORAGE_VMS_IN_POOL(); Line 681: Line 682: @DefaultStringValue("Cannot ${action} ${type}. The Storage Domain name is already in use.") Line 683: String ACTION_TYPE_FAILED_STORAGE_DOMAIN_NAME_ALREADY_EXIST(); -- To view, visit http://gerrit.ovirt.org/24286 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I971fe6acd4a2667a09487c5e1108cf7c759587f1 Gerrit-PatchSet: 13 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Liran Zelkha <lzel...@redhat.com> Gerrit-Reviewer: Liron Ar <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
