Ori Liel has posted comments on this change.
Change subject: restapi: Fix lun mapping for add lun-disk
......................................................................
Patch Set 1: (1 inline comment)
....................................................
File
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/StorageLogicalUnitMapper.java
Line 76: if (model.isSetLogicalUnits()) {
Line 77: LogicalUnit logicalUnit = model.getLogicalUnits().get(0);
Line 78: entity.setLUN_id(logicalUnit.getId());
Line 79: ArrayList<storage_server_connections> connections = new
ArrayList<storage_server_connections>();
Line 80: connections.add(map(logicalUnit, null));
we don't have anywhere that maps LogicalUnit to storage_server_connections. We
have code that maps Storage to storage_server_connections (and it's not
currently in a Mapper object, by the way).
So this is not duplication.
Creating iscis storage domains is possible in two ways: 1) assuming host
already has a session with the storage server (no need for address, target,
port) 2) not assuming host already has a session with the storage server
(requires address, target, port).
Adding lun-disk is like the second case, so we need to supply, and map, storage
details.
The only thing worth considering is wheter the connection info should be passed
on the storage object instead of the lun, i.e, instead of this XML, which was
reported in the bug:
<disk>
<name>direct0</name>
<provisioned_size>1073741824</provisioned_size>
<interface>virtio</interface>
<format>cow</format>
<lunStorage>
<logical_unit id="36006048c14e9eb8f668dfc53ea5995ca">
<address>10.34.63.200</address>
<port>3260</port>
<target>iqn.1992-05.com.emc:ckm001201002300000-5-vnxe</target>
</logical_unit>
</lunStorage>
</disk>
Perhaps we should expect the data this way:
<disk>
<name>direct0</name>
<provisioned_size>1073741824</provisioned_size>
<interface>virtio</interface>
<format>cow</format>
<lunStorage>
<address>10.34.63.200</address>
<port>3260</port>
<target>iqn.1992-05.com.emc:ckm001201002300000-5-vnxe</target>
<logical_unit id="36006048c14e9eb8f668dfc53ea5995ca"/>
</lunStorage>
</disk>
Then, with moving some code around, we can re-use mapping of
Storage-->storage_server_connections. But the code re-use shouldn't be a
deciding factor, it's a modelling decision.
Line 81: entity.setLunConnections(connections);
Line 82: }
Line 83: if (model.isSetType()) {
Line 84: StorageType storageType =
StorageType.fromValue(model.getType());
--
To view, visit http://gerrit.ovirt.org/7206
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e1299265f4c24b4212b57924eb2c4389411c5a4
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ori Liel <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Ori Liel <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
