Alissa Bonas has posted comments on this change.
Change subject: core: disaster recovery on update connection [wip]
......................................................................
Patch Set 1: (2 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommand.java
Line 205: }
Line 206:
Line 207: for (LUNs lunFromVgInfo : lunsFromVgInfo) {
Line 208: for (LUNs lunFromDb : lunsFromDb) {
Line 209: if (lunFromDb.getphysical_volume_id() == null
||
But the user doesn't know what VG is. From user's point of view, in the current
UI - he selected the luns, and he should be the one to control/manage them.
Otherwise you are taking that capability away, it can lead to lack of
confidence using the system if the system is adding/removing the luns on its
own without user's ability to influence it..
I think that the UI should be changed in the case you want the system to manage
the luns.
Line 210:
!lunFromDb.getphysical_volume_id().equals(lunFromVgInfo.getphysical_volume_id()))
{
Line 211: continue;
Line 212: }
Line 213:
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/StorageServerConnectionParametersBase.java
Line 10:
Line 11: @Valid
Line 12: private StorageServerConnections privateStorageServerConnection;
Line 13:
Line 14: private Guid storageDomainId;
It is incorrect to define this member here for several reasons:
1. The command itself loads the relevant domain/domains from db - why it should
be passed in parameters?
2. There can be several domains using a connection (in block domains when it
represents a target), thus a single storage domain id is incorrect for block
domains.
2. The user is going to be able to update a connection regardless of storage
domains existence, so asking for this parameter in the parameters is not
relevant for some use cases.
Line 15:
Line 16: public StorageServerConnections getStorageServerConnection() {
Line 17: return privateStorageServerConnection;
Line 18: }
--
To view, visit http://gerrit.ovirt.org/16533
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0be9c598ee666df2977ffbc2526e2d3fbd997e6
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
