Tal Nisan has posted comments on this change. Change subject: core: Plug disk to VM when adding a new disk ......................................................................
Patch Set 1: (5 comments) .................................................... Commit Message Line 4: Commit: Tal Nisan <[email protected]> Line 5: CommitDate: 2013-11-28 18:58:58 +0200 Line 6: Line 7: core: Plug disk to VM when adding a new disk Line 8: Will do Line 9: When adding a new disk to a VM the disk is plugged upon creation only if Line 10: the VM is down, this patch is plugging the disk by default after the disk Line 11: creation is completed even if the VM is not down, this behavior can be Line 12: overriden by specifing a parameter to the add disk command, the new disk Line 8: Line 9: When adding a new disk to a VM the disk is plugged upon creation only if Line 10: the VM is down, this patch is plugging the disk by default after the disk Line 11: creation is completed even if the VM is not down, this behavior can be Line 12: overriden by specifing a parameter to the add disk command, the new disk In another patch it will be, for now only through the web, in the bug it was requested that this will be the default behavior from now on Line 13: popup in webadmin was changed to support this behavior Line 14: Line 15: Bug-Url: https://bugzilla.redhat.com/856272 Line 16: Change-Id: Ia9778bcaf21b346a55992590159cabd8d78f0c66 .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java Line 343: Line 344: @Override Line 345: protected void executeVmCommand() { Line 346: getParameters().getDiskInfo().setId(Guid.newGuid()); Line 347: getParameters().setEntityInfo(new EntityInfo(VdcObjectType.Disk, getParameters().getDiskInfo().getId())); Because a) this initialization is only relevant for the execution part and b) once the task ends another instance of this command is created and it regenerates another disk id instead of keeping the one it created initially thus getting to the endSuccessfully part with the wrong disk id Line 348: ImagesHandler.setDiskAlias(getParameters().getDiskInfo(), getVm()); Line 349: if (DiskStorageType.IMAGE == getParameters().getDiskInfo().getDiskStorageType()) { Line 350: createDiskBasedOnImage(); Line 351: } else { Line 402: getVmId()), Line 403: VmDeviceGeneralType.DISK, Line 404: VmDeviceType.DISK, Line 405: null, Line 406: getVm().getStatus() == VMStatus.Down && getParameters().getPlugDiskToVm(), Yes, any suggestion of a name? Line 407: Boolean.TRUE.equals(getParameters().getDiskInfo().getReadOnly()), Line 408: null)); Line 409: getCompensationContext().stateChanged(); Line 410: } Line 546: super.endSuccessfully(); Line 547: } Line 548: Line 549: private void plugDiskToVmIfNeeded() { Line 550: if (getVm() != null && getParameters().getPlugDiskToVm() && getVm().getStatus() != VMStatus.Down) { 1. In case of a a floating disk the getVm() will return null and in that case there is no VM to attach to 2. True but in case the VM is down the execute phase of this command will have already plugged this disk upon it's creation so this will result in try to re-plug it Line 551: HotPlugDiskToVmParameters params = new HotPlugDiskToVmParameters(getVmId(), getParameters().getDiskInfo().getId()); Line 552: Backend.getInstance().runInternalAction(VdcActionType.HotPlugDiskToVm, params); Line 553: } Line 554: } -- To view, visit http://gerrit.ovirt.org/21851 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia9778bcaf21b346a55992590159cabd8d78f0c66 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Tal Nisan <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Ayal Baron <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Liron Ar <[email protected]> Gerrit-Reviewer: Sergey Gotliv <[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
