Allon Mureinik has posted comments on this change.

Change subject: core: Remove storage type parameter in create storage pool VDS 
command
......................................................................


Patch Set 2: Code-Review+2

(4 comments)

I wonder if we don't also want to remove this parameter complete from VDSM and 
stop sending it.
Perhaps in another patch.

http://gerrit.ovirt.org/#/c/23293/2//COMMIT_MSG
Commit Message:

Line 6: 
Line 7: core: Remove storage type parameter in create storage pool VDS command
Line 8: 
Line 9: As a part of the effort to remove the type of storage pool, the storage
Line 10: type parameter in create storage pool VDS command was removed as it's
s/create storage pool VDS command/CreateStoragePoolVDSCommand/
Line 11: ignored anyway in VDSM so no need to set it, the method signature in
Line 12: the broker and VDSM will remain the same for legacy purposes
Line 13: 
Line 14: Change-Id: Ia7efe3008fcdcbde4336ad026249b290b6b3eed5


Line 7: core: Remove storage type parameter in create storage pool VDS command
Line 8: 
Line 9: As a part of the effort to remove the type of storage pool, the storage
Line 10: type parameter in create storage pool VDS command was removed as it's
Line 11: ignored anyway in VDSM so no need to set it, the method signature in
s/, the/. The/
Line 12: the broker and VDSM will remain the same for legacy purposes
Line 13: 
Line 14: Change-Id: Ia7efe3008fcdcbde4336ad026249b290b6b3eed5
Line 15: Relates-To: https://bugzilla.redhat.com/1038053


Line 8: 
Line 9: As a part of the effort to remove the type of storage pool, the storage
Line 10: type parameter in create storage pool VDS command was removed as it's
Line 11: ignored anyway in VDSM so no need to set it, the method signature in
Line 12: the broker and VDSM will remain the same for legacy purposes
s/legacy/backwards compatibility/
Line 13: 
Line 14: Change-Id: Ia7efe3008fcdcbde4336ad026249b290b6b3eed5
Line 15: Relates-To: https://bugzilla.redhat.com/1038053


http://gerrit.ovirt.org/#/c/23293/2/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/CreateStoragePoolVDSCommand.java
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/CreateStoragePoolVDSCommand.java:

Line 20:             ids[i] = guids[i].toString();
Line 21:         }
Line 22: 
Line 23:         //The first parameter, poolType parameter is ignored by VDSM 
and thus can be set to any arbitrary value
Line 24:         status = getBroker().createStoragePool(0,
I'd use StorageType.UNKNOWN.getValue() - seems clearer.
Line 25:                 getParameters().getStoragePoolId().toString(), 
getParameters().getStoragePoolName(),
Line 26:                 getParameters().getMasterDomainId().toString(), ids, 
getParameters().getMasterVersion(),
Line 27:                 Config.<String> getValue(ConfigValues.LockPolicy),
Line 28:                 Config.<Integer> 
getValue(ConfigValues.LockRenewalIntervalSec),


-- 
To view, visit http://gerrit.ovirt.org/23293
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7efe3008fcdcbde4336ad026249b290b6b3eed5
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[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

Reply via email to