Sergey Gotliv has posted comments on this change.
Change subject: backend, restapi: Add read-only disk functionality
......................................................................
Patch Set 17:
(8 comments)
....................................................
Commit Message
Line 9: All underlying components are already supporting RO disks
Line 10: functionlity. After that change Engine will support it either through
Line 11: the REST API.
Line 12:
Line 13: When adding/attaching a disk to a VM, this feature adds a read-only
Done
Line 14: property to the VM-Disk relationship which is persisted through
Line 15: VMDevice (vm_device in the DB). Floating disks continue to be RW.
Line 16:
Line 17: Note that shareable disk could be attached to one VM as RO, and to
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java
Line 240: }
Line 241:
Line 242: private boolean validateCanUpdateReadOnly() {
Line 243: if (shouldUpdateReadOnly() && getVm().getStatus() !=
VMStatus.Down) {
Line 244: return
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_NOT_DOWN);
I will do that in another patch.
Line 245: }
Line 246: return true;
Line 247: }
Line 248:
....................................................
File
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmDiskCommandTest.java
Line 274: final UpdateVmDiskParameters parameters = createParameters();
Line 275: parameters.getDiskInfo().setReadOnly(true);
Line 276:
Line 277: // creating old disk with interface different than interface
of disk from parameters
Line 278: // have to return original disk on each request to dao,
Done
Line 279: // since the command updates retrieved instance of disk
Line 280: when(diskDao.get(diskImageGuid)).thenAnswer(new Answer() {
Line 281: @Override
Line 282: public Object answer(InvocationOnMock invocationOnMock)
throws Throwable {
Line 281: @Override
Line 282: public Object answer(InvocationOnMock invocationOnMock)
throws Throwable {
Line 283: final DiskImage oldDisk = createDiskImage();
Line 284: oldDisk.setReadOnly(false);
Line 285:
assert(!oldDisk.getReadOnly().equals(parameters.getDiskInfo().getReadOnly()));
Done
Line 286: return oldDisk;
Line 287: }
Line 288: });
Line 289:
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/AttachDettachVmDiskParameters.java
Line 5: import org.ovirt.engine.core.compat.Guid;
Line 6:
Line 7: public class AttachDettachVmDiskParameters extends
VmDiskOperationParameterBase {
Line 8:
Line 9: private static final long serialVersionUID = 5640716432695539552L;
According to Java documentation if the change is backward compatible with the
class previous version then there is no need to change serialVersionUID.
I think that my change is backward compatible - readOnly will just get the
default value (I assume this is false).
Line 10: private boolean isPlugUnPlug;
Line 11: private boolean isReadOnly;
Line 12:
Line 13: public AttachDettachVmDiskParameters() {
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmDevice.java
Line 266: && bootOrder == other.bootOrder
Line 267: && ObjectUtils.objectsEqual(specParams,
other.specParams)
Line 268: && isManaged == other.isManaged
Line 269: && getIsPlugged().equals(other.getIsPlugged())
Line 270: && getIsReadOnly().equals(other.getIsReadOnly())
I am not against, I am aligning readOnly with plugged.
>From other side I don't see any use case where I want to compare 2 devices
>with the same id but one has readOnly = false and another has readOnly = null
>and want to get false as a result of that compare.
Line 271: && alias.equals(other.alias)
Line 272: && ObjectUtils.objectsEqual(customProperties,
other.customProperties)
Line 273: && ObjectUtils.objectsEqual(snapshotId,
other.snapshotId));
Line 274: }
....................................................
File
backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml
Line 559: description: add a new direct lun disk to the virtual
machine, this operation does not require size but needs lun connection details
Line 560: - mandatoryArguments: {disk.id: 'xs:string'}
Line 561: optionalArguments:
Line 562: disk.active: xs:boolean
Line 563: disk.read_only: xs:boolean
Look, I don't know what to say. I added "read only" in the line 523 as a part
of optional arguments and there are no any brackets there.
Please, look either at lines 512, 536, 550, 561, 988, 1002 ....
Line 564: description: attach a disk to the virtual machine
Line 565: - mandatoryArguments: {disk.id: 'xs:string', disk.snapshot.id:
'xs:string'}
Line 566: optionalArguments: {disk.active: 'xs:boolean'}
Line 567: description: attach a disk snapshot to the virtual machine
....................................................
File
backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmDiskResourceTest.java
Line 251:
Line 252: protected Disk getUpdate() {
Line 253: Disk update = new Disk();
Line 254: update.setWipeAfterDelete(false);
Line 255: update.setReadOnly(false);
Done
Line 256: return update;
Line 257: }
Line 258:
Line 259: @Override
--
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: 17
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: Vered Volansky <[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
