Francesco Romani has posted comments on this change. Change subject: core: not migrate if dst host resolved to src host ......................................................................
Patch Set 3: (4 comments) http://gerrit.ovirt.org/#/c/30239/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java: Line 360: addCanDoActionMessage(VdcBllMessages.VAR__HOST_STATUS__UP); Line 361: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VDS_STATUS_ILLEGAL); Line 362: } Line 363: Line 364: if (getDestinationVds() != null && !canMigrateToDestinationVDS(getDestinationVds())) { > no need to pass the dest host as parameter, you can use getDestVds() inside Sure, will move. Line 365: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VDS_WITH_SAME_HOST_EXIST); Line 366: } Line 367: Line 368: return validate(new SnapshotsValidator().vmNotDuringSnapshot(vm.getId())) Line 368: return validate(new SnapshotsValidator().vmNotDuringSnapshot(vm.getId())) Line 369: // This check was added to prevent migration of VM while its disks are being migrated Line 370: // TODO: replace it with a better solution Line 371: && validate(new DiskImagesValidator(ImagesHandler.getPluggedActiveImagesForVm(vm.getId())).diskImagesNotLocked()) Line 372: && SchedulingManager.getInstance().canSchedule(getVdsGroup(), > im not so sure about this, but..: i think destVds is null on normal migrate ok, will check. Line 373: getVm(), Line 374: getVdsBlackList(), Line 375: getParameters().getInitialHosts(), Line 376: getDestinationVdsId(), Line 376: getDestinationVdsId(), Line 377: getReturnValue().getCanDoActionMessages()); Line 378: } Line 379: Line 380: // BZ#1107650 > although i dont mind the bz # here, i dont think its needed, in my opinion OK, will unclutter the code Line 381: private boolean canMigrateToDestinationVDS(VDS dstVds) { Line 382: VDS srcVds = getVds(); Line 383: if (srcVds.getHostName().equals(dstVds.getHostName())) { Line 384: log.warnFormat("cannot migrate to VDS {0}: same hostname as source", dstVds.getHostName()); Line 377: getReturnValue().getCanDoActionMessages()); Line 378: } Line 379: Line 380: // BZ#1107650 Line 381: private boolean canMigrateToDestinationVDS(VDS dstVds) { > * please name the method according to what it does, something like validate your name is better, then I'll make use of it. Line 382: VDS srcVds = getVds(); Line 383: if (srcVds.getHostName().equals(dstVds.getHostName())) { Line 384: log.warnFormat("cannot migrate to VDS {0}: same hostname as source", dstVds.getHostName()); Line 385: return false; -- To view, visit http://gerrit.ovirt.org/30239 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7d05fb0c1f8108580edc391f87b9e95ffc2ae434 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Francesco Romani <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: Roy Golan <[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
