Allon Mureinik has posted comments on this change.
Change subject: core: Add engine read-only disks capabilities
......................................................................
Patch Set 5: Code-Review-1
(8 comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java
Line 92: return false;
Line 93: }
Line 94:
Line 95: updateDisksFromDb();
Line 96: // if user sent drive check that is not in use
This is wrong - if anything, it should be "it's"
(also, seems unrelated to the patch, no?)
Line 97: if (!isDiskCanBeAddedToVm(getParameters().getDiskInfo(),
vm) ||
Line 98:
!isDiskPassPciAndIdeLimit(getParameters().getDiskInfo())) {
Line 99: return false;
Line 100: }
Line 352: getBaseDiskDao().save(getParameters().getDiskInfo());
Line 353: getDiskLunMapDao().save(new
DiskLunMap(getParameters().getDiskInfo().getId(), lun.getLUN_id()));
Line 354: if (getVm() != null) {
Line 355: boolean isReadOnly =
Line 356:
BooleanUtils.toBooleanDefaultIfNull(getParameters().getDiskInfo().getReadOnly(),
false);
This line repeats twice here - can't you extract it to a utility method?
Line 357: VmDeviceUtils.addManagedDevice(new
VmDeviceId(getParameters().getDiskInfo().getId(), getVmId()),
Line 358: VmDeviceGeneralType.DISK,
Line 359: VmDeviceType.DISK,
Line 360: null,
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java
Line 471: getOldDisk().getDiskInterface() !=
getNewDisk().getDiskInterface() ||
Line 472: getOldDisk().getPropagateErrors() !=
getNewDisk().getPropagateErrors() ||
Line 473: getOldDisk().isWipeAfterDelete() !=
getNewDisk().isWipeAfterDelete() ||
Line 474: getOldDisk().isShareable() !=
getNewDisk().isShareable() ||
Line 475: getOldDisk().getReadOnly() !=
getNewDisk().getReadOnly() ||
there's not guarantee that two Booleans containing true would be the same
instance.
Line 476: getOldDisk().getSgio() != getNewDisk().getSgio() ||
Line 477:
!StringUtils.equals(getOldDisk().getDiskDescription(),
getNewDisk().getDiskDescription()) ||
Line 478: !StringUtils.equals(getOldDisk().getDiskAlias(),
getNewDisk().getDiskAlias());
Line 479: }
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/UpdateVmDiskParameters.java
Line 9
Line 10
Line 11
Line 12
Line 13
This ctor is required for GWT serialization. Please return it.
....................................................
File
backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml
Line 560: =======
Line 561: optionalArguments:
Line 562: disk.active: xs:boolean
Line 563: disk.read_only: xs:boolean
Line 564: >>>>>>> 9a98cee... core: Add engine read-only disks capabilities
Seriously?
Line 565: urlparams: {}
Line 566: headers:
Line 567: Content-Type: {value: application/xml|json, required: true}
Line 568: Expect: {value: 201-created, required: false}
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmDisksResource.java
Line 107:
Line 108: @Override
Line 109: protected String[] getRequiredUpdateFields() {
Line 110: return new String[0];
Line 111: //return new String[] { "readOnly" };
huh?
Line 112: }
Line 113:
Line 114: @Override
Line 115: protected VdcActionParametersBase
getAddParameters(org.ovirt.engine.core.common.businessentities.Disk entity,
Disk disk) {
....................................................
File
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
Line 1200: return newDeviceId;
Line 1201: }
Line 1202:
Line 1203: private boolean getDiskReadOnly(Map device)
Line 1204: {
"{" should not be on its own line
Line 1205: String readOnly = StringUtils.defaultIfEmpty(
Line 1206: (String) device.get(VdsProperties.Type),
Boolean.FALSE.toString());
Line 1207: return Boolean.getBoolean(readOnly);
Line 1208: }
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/disks/DiskListModel.java
Line 280
Line 281
Line 282
Line 283
Line 284
how is this related to the patch?
--
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: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Vered Volansky <vvola...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
