Sergey Gotliv has posted comments on this change.
Change subject: engine: Ties more consistently between vm disk device and disk
entities.
......................................................................
Patch Set 3:
(2 comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
Line 856: }
Line 857: }
Line 858:
Line 859: protected void addDiskPermissions() {
Line 860: List<Guid> newDiskImageIds = new
ArrayList<>(srcDiskIdToTargetDiskIdMapping.values());
I understand your review, but I think in the opposite direction -
1. I don't like to send private member as parameter to method of the same class.
2. This method must be private. Nobody calls it from outside, so there is no
reason to keep it as protected.
If someone in the future will need to change it for any reason, he can do that,
right.
Line 861: permissions[] permsArray = new
permissions[newDiskImageIds.size()];
Line 862: for (int i = 0; i < newDiskImageIds.size(); i++) {
Line 863: permsArray[i] =
Line 864: new permissions(getCurrentUser().getUserId(),
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java
Line 383: Guid dstId,
Line 384: Map<Guid, Guid>
srcDiskToTargetDiskMapping,
Line 385: List<VmNetworkInterface> ifaces,
Line 386: boolean soundDeviceEnabled,
Line 387: boolean isConsoleEnabled) {
Great catch! I'll fix NICs in another patch.
Line 388: VM vm = DbFacade.getInstance().getVmDao().get(dstId);
Line 389: VmBase vmBase = (vm != null) ? vm.getStaticData() : null;
Line 390: boolean isVm = (vmBase != null);
Line 391:
--
To view, visit http://gerrit.ovirt.org/17854
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8bf284ea4e05f03fc0a7e94a1ec901aacbdc1ab1
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgot...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Haim Ateya <hat...@redhat.com>
Gerrit-Reviewer: Liron Ar <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgot...@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
