Sergey Gotliv has posted comments on this change.
Change subject: core: Plug disk to VM when adding a new disk
......................................................................
Patch Set 1:
(3 comments)
....................................................
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()));
I have no problem with non-related change, but I am curious why parameters are
not properly initialized on the caller side?
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(),
Can you, please, define variable with meaningful name for that row.
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. VM can't be null at this point.
2. HotPlugDiskToVMCommand validates that VM status is UP, DOWN or PAUSED.
Checking VM status here is probably redundant, you keep shared lock on VM so
its status can be changed , but if you doing that anyway do that the same way
the HotPlug does.
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: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
