Sergey Gotliv has posted comments on this change.
Change subject: backend, restapi: Add read-only disk functionality
......................................................................
Patch Set 14:
(6 comments)
Maor,
Thanks for review. Please, see my answers inside.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmFromConfigurationCommand.java
Line 70: private AuditLogType
attemptToAttachDisksToImportedVm(Collection<Disk> disks){
Line 71: List<String> failedDisks = new LinkedList<>();
Line 72: for (Disk disk : disks) {
Line 73: AttachDettachVmDiskParameters params = new
AttachDettachVmDiskParameters(getVm().getId(),
Line 74: disk.getId(), disk.getPlugged(),
disk.getReadOnly());
Yes.
Line 75: VdcReturnValueBase returnVal =
getBackend().runInternalAction(VdcActionType.AttachDiskToVm, params);
Line 76: if (!returnVal.getSucceeded()) {
Line 77: failedDisks.add(disk.getDiskAlias());
Line 78: }
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java
Line 313: setSucceeded(true);
Line 314: return null;
Line 315: }
Line 316:
Line 317: private void updateDeviceProperties() {
Fair enough. I have to check if someone else is using it.
If not, I will remove this stored procedure in another path.
Line 318: VmDevice device = getVmDeviceDao().get(new
VmDeviceId(getOldDisk().getId(), getVmId()));
Line 319: if
(!ObjectUtils.objectsEqual(getOldDisk().getReadOnly(),
getNewDisk().getReadOnly())) {
Line 320: device.setIsReadOnly(getNewDisk().getReadOnly());
Line 321: }
Line 315: }
Line 316:
Line 317: private void updateDeviceProperties() {
Line 318: VmDevice device = getVmDeviceDao().get(new
VmDeviceId(getOldDisk().getId(), getVmId()));
Line 319: if
(!ObjectUtils.objectsEqual(getOldDisk().getReadOnly(),
getNewDisk().getReadOnly())) {
Done
Line 320: device.setIsReadOnly(getNewDisk().getReadOnly());
Line 321: }
Line 322: if (getOldDisk().getDiskInterface() !=
getNewDisk().getDiskInterface()) {
Line 323: device.setAddress(StringUtils.EMPTY);
Line 503: return !ObjectUtils.objectsEqual(oldQuotaId, getQuotaId());
Line 504: }
Line 505:
Line 506: private boolean shouldUpdateDeviceProperties() {
Line 507: return !ObjectUtils.objectsEqual(getOldDisk().getReadOnly(),
getNewDisk().getReadOnly()) ||
== is good for boolean but not for Boolean.
I will use equals. NullPointerException is on you :-)
Line 508: getOldDisk().getDiskInterface() !=
getNewDisk().getDiskInterface();
Line 509: }
Line 510:
Line 511: private boolean isAtLeastOneVmIsNotDown(List<VM>
vmsDiskPluggedTo) {
Line 573: vmsDiskOrSnapshotPluggedTo.add(vm);
Line 574: }
Line 575:
Line 576: if (vm.getId().equals(getParameters().getVmId())) {
Line 577: vmDeviceForVm = pair.getSecond();
Done
Line 578: // disk's 'readOnly' property is transient, more
than that disk can be read only in the scope of
Line 579: // one VM and read write in the scope of another,
therefore 'readOnly' should be updated with
Line 580: // the value from device.
Line 581:
getOldDisk().setReadOnly(vmDeviceForVm.getIsReadOnly());
Line 577: vmDeviceForVm = pair.getSecond();
Line 578: // disk's 'readOnly' property is transient, more
than that disk can be read only in the scope of
Line 579: // one VM and read write in the scope of another,
therefore 'readOnly' should be updated with
Line 580: // the value from device.
Line 581:
getOldDisk().setReadOnly(vmDeviceForVm.getIsReadOnly());
Check it again, please.
It takes device in scope of vm and only if vm id in parameters is the same as
vm id on device it updates the disk.
Anyway oldDisk and vmDeviceForVM both members of that class and should match.
Line 582: }
Line 583: }
Line 584: }
Line 585: }
--
To view, visit http://gerrit.ovirt.org/15409
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I124176e8feb91b601a71e76dd63051648ec4353a
Gerrit-PatchSet: 14
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Vered Volansky <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[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
