Kanagaraj M has posted comments on this change.
Change subject: gluster:Support Striped_replicate volume types
......................................................................
Patch Set 4: (6 inline comments)
Apart from inline comments, there a quite amount validation is performed on
no.of bricks getting added. we have to address that as well.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/CreateGlusterVolumeCommand.java
Line 86: addValidationGroup(CreateReplicatedVolume.class);
Line 87: break;
Line 88: default:
Line 89: addValidationGroup(CreateEntity.class);
Line 90: }
can we make this simple as something like following.
if(volume.getVolumeType().isReplicatedType()) {
addValidationGroup(CreateReplicatedVolume.class);
}
if(volume.getVolumeType().isStripedType()) {
addValidationGroup(CreateStripedVolume.class);
}
Line 91: return super.getValidationGroups();
Line 92: }
Line 93:
Line 94: @Override
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/gluster/GlusterVolumeEntity.java
Line 60: @NotNull(message =
"VALIDATION.GLUSTER.VOLUME.STRIPE_COUNT.NOT_NULL", groups = {
CreateStripedVolume.class })
Line 61: private Integer stripeCount;
Line 62:
Line 63: @Valid
Line 64: private final Map<String, GlusterVolumeOptionEntity> options = new
LinkedHashMap<String, GlusterVolumeOptionEntity>();
please remove this final as GWT won't serialize the final fields
Line 65:
Line 66: @NotNull(message = "VALIDATION.GLUSTER.VOLUME.BRICKS.NOT_NULL",
groups = {CreateEntity.class, CreateReplicatedVolume.class,
CreateStripedVolume.class})
Line 67: @Valid
Line 68: private List<GlusterBrickEntity> bricks = new
ArrayList<GlusterBrickEntity>();
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/gluster/GlusterVolumeType.java
Line 55: }
Line 56:
Line 57: public boolean isReplicatedType() {
Line 58: return value().contains("REPLICATE");
Line 59: }
I would prefer having fields for isReplicated and isStriped instead doing
string comparison.
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/VolumeModel.java
Line 135: if (getTypeList().getSelectedItem() ==
GlusterVolumeType.REPLICATE
Line 136: || getTypeList().getSelectedItem() ==
GlusterVolumeType.DISTRIBUTED_REPLICATE
Line 137: || getTypeList().getSelectedItem() ==
GlusterVolumeType.STRIPED_REPLICATE
Line 138: || getTypeList().getSelectedItem() ==
GlusterVolumeType.DISTRIBUTED_STRIPED_REPLICATE) {
Line 139: getReplicaCount().setIsAvailable(true);
this can be more simplified as
getReplicaCount().setIsAvailable(getTypeList().getSelectedItem().isReplicatedType());
also get away with if/else
Line 140: }
Line 141: else {
Line 142: getReplicaCount().setIsAvailable(false);
Line 143: }
Line 144:
Line 145: if (getTypeList().getSelectedItem() ==
GlusterVolumeType.STRIPE
Line 146: || getTypeList().getSelectedItem() ==
GlusterVolumeType.DISTRIBUTED_STRIPE
Line 147: || getTypeList().getSelectedItem() ==
GlusterVolumeType.STRIPED_REPLICATE
Line 148: || getTypeList().getSelectedItem() ==
GlusterVolumeType.DISTRIBUTED_STRIPED_REPLICATE) {
same as above
Line 149: getStripeCount().setIsAvailable(true);
Line 150: }
Line 151: else {
Line 152: getStripeCount().setIsAvailable(false);
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/volumes/VolumeListModel.java
Line 601:
volume.setReplicaCount(volumeModel.getReplicaCountValue());
Line 602: break;
Line 603: default:
Line 604: break;
Line 605: }
same as type.isReplicatedType()
Line 606:
Line 607: volume.setVolumeType(type);
Line 608:
Line 609: if ((Boolean) volumeModel.getTcpTransportType().getEntity()) {
--
To view, visit http://gerrit.ovirt.org/15381
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I517c1e9e41f1bb68626d5a8a45d679208934d98f
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Sahina Bose <[email protected]>
Gerrit-Reviewer: Kanagaraj M <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Sahina Bose <[email protected]>
Gerrit-Reviewer: Shubhendu Tripathi <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
