Shubhendu Tripathi has posted comments on this change.

Change subject: gluster: Sync job for gluster volume snapshots
......................................................................


Patch Set 5:

(2 comments)

http://gerrit.ovirt.org/#/c/35904/5/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterSnapshotSyncJob.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterSnapshotSyncJob.java:

Line 183:         try (EngineLock lock = acquireEntityLock(clusterId)) {
Line 184:             addOrUpdateClusterConfig(clusterId, 
"snap-max-hard-limit", configInfo.getSystemSnapMaxHardLimit()
Line 185:                     .toString());
Line 186:             addOrUpdateClusterConfig(clusterId, 
"snap-max-soft-limit", configInfo.getSystemSoftLimitPcnt());
Line 187:             addOrUpdateClusterConfig(clusterId, "auto-delete", 
configInfo.isAutoDeleteEnabled() ? "enable" : "disable");
> Can vdsm verb not return it as key value pairs?
Yes that should be fine and favourable as well because later if there are 
additional entries, only VDSM layer gets affected.
Will do that.
Line 188:         }
Line 189: 
Line 190:         List<GlusterSnapshotConfigInfo.VolumeSnapshotConfigInfo> 
volumeConfigs = configInfo.getVolumeConfigList();
Line 191:         for (GlusterSnapshotConfigInfo.VolumeSnapshotConfigInfo 
volumeConfig : volumeConfigs) {


http://gerrit.ovirt.org/#/c/35904/5/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/gluster/GlusterSnapshotConfigInfo.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/gluster/GlusterSnapshotConfigInfo.java:

Line 13:     private Guid clusterId;
Line 14:     private Integer systemSnapMaxHardLimit;
Line 15:     private String systemSoftLimitPcnt;
Line 16:     private Boolean autoDeleteEnabled;
Line 17:     private List<VolumeSnapshotConfigInfo> volumeConfigList;
> Volume specific configuration options would be a property of the volume, ri
Yes. currently only volume specific option is snap-max-hard-limit. Later there 
might be more.
So planning to have a dictionary with two keys say clusterConfigs and 
volumeConfigs returned from VDSM and this class as well would have those two 
HashMap fields only. This would make code simple and generic.
Line 18: 
Line 19:     public Guid getClusterId() {
Line 20:         return clusterId;
Line 21:     }


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7b7bf79b72fc5680dab301b290e7aa860d5c714d
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Shubhendu Tripathi <[email protected]>
Gerrit-Reviewer: Kanagaraj M <[email protected]>
Gerrit-Reviewer: Ramesh N <[email protected]>
Gerrit-Reviewer: Sahina Bose <[email protected]>
Gerrit-Reviewer: Shubhendu Tripathi <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: [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