Liron Ar has posted comments on this change. Change subject: core: Block live storage migration to domain of a different type ......................................................................
Patch Set 1: (2 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 perform the queries there so they'll be performed only when needed (right now we always load the domains even if we don't actually get to perform the check) 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 done in the two previous lines (though we still need to check for existense before of course) If the live migration isn't possible between the domains, it's better to return it before checking the domain status..otherwise the user might "handle" the domain status while it's not even possible (e.g - activate it) Line 286: if (!getReturnValue().getCanDoAction()) { Line 287: return false; Line 288: } Line 289: } -- 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 <tni...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Cheryn Tan <cheryn...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Itamar Heim <ih...@redhat.com> Gerrit-Reviewer: Liron Ar <lara...@redhat.com> 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
