Tal Nisan has posted comments on this change. Change subject: core: Block live storage migration to domain of a different type ......................................................................
Patch Set 1: (3 comments) http://gerrit.ovirt.org/#/c/23759/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommand.java: Line 273: } Line 274: Line 275: for (LiveMigrateDiskParameters parameters : getParameters().getParametersList()) { Line 276: StorageDomain sourceDomain = getImageSourceDomain(parameters.getImageId()); Line 277: StorageDomain destDomain = getStorageDomainById(parameters.getStorageDomainId(), getStoragePoolId()); > it'll be better IMO to add a method - "performDomainsRelatedChecks" and per Done Line 278: Line 279: getReturnValue().setCanDoAction(isDiskNotShareable(parameters.getImageId()) Line 280: && isDiskSnapshotNotPluggedToOtherVmsThatAreNotDown(parameters.getImageId()) Line 281: && isTemplateInDestStorageDomain(parameters.getImageId(), parameters.getStorageDomainId()) Line 281: && isTemplateInDestStorageDomain(parameters.getImageId(), parameters.getStorageDomainId()) Line 282: && validateSourceStorageDomain(sourceDomain) Line 283: && validateDestStorage(destDomain) Line 284: && validateDestStorageAndSourceStorageOfSameTypes(destDomain, sourceDomain)); Line 285: > consider to move this check to to be somehow before the domain status check Too much overhead unfortunately, the validators check existence and status together, to achieve that I will have to change the validators, too much effort for too little value Line 286: if (!getReturnValue().getCanDoAction()) { Line 287: return false; Line 288: } Line 289: } http://gerrit.ovirt.org/#/c/23759/1/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties: Line 636: ACTION_TYPE_FAILED_SNAPSHOT_IS_BEING_TAKEN_FOR_VM=Cannot ${action} ${type}. Snapshot is currently being created for VM ${VmName}. Line 637: ACTION_TYPE_FAILED_DISK_IS_USED_FOR_CREATE_VM=Cannot ${action} ${type}. This disk is currently in use to create VM ${VmName}. Line 638: ACTION_TYPE_FAILED_DISK_IS_BEING_REMOVED=Cannot ${action} ${type}. Disk ${DiskName} is being removed. Line 639: ACTION_TYPE_FAILED_DISK_IS_BEING_MIGRATED=Cannot ${action} ${type}. Disk ${DiskName} is being moved or copied. Line 640: ACTION_TYPE_FAILED_DESTINATION_AND_SOURCE_STORAGE_SUB_TYPES_DIFFERENT=Cannot ${action} ${type}. Source and target domains must either both be file domains or block domains. > Just a minor change so it reads better: Done Line 641: ACTION_TYPE_FAILED_TEMPLATE_IS_BEING_EXPORTED=Cannot ${action} ${type}. Template ${TemplateName} is being exported. Line 642: ACTION_TYPE_FAILED_VM_IS_BEING_IMPORTED=Cannot ${action} ${type}. VM ${VmName} is being imported. Line 643: ACTION_TYPE_FAILED_VM_IS_BEING_MIGRATED=Cannot ${action} ${type}. VM ${VmName} is being migrated. Line 644: ACTION_TYPE_FAILED_TEMPLATE_IS_BEING_REMOVED=Cannot ${action} ${type}. Template ${TemplateName} is being removed. -- To view, visit http://gerrit.ovirt.org/23759 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iec4c34132a1a619527627cfa47fa99df3a023b62 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Tal Nisan <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Cheryn Tan <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Itamar Heim <[email protected]> Gerrit-Reviewer: Liron Ar <[email protected]> Gerrit-Reviewer: Tal Nisan <[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
