Bala.FA has posted comments on this change. Change subject: gluster: volume snapshot create and delete verbs. ......................................................................
Patch Set 13: Code-Review-1 (4 comments) http://gerrit.ovirt.org/#/c/34535/13//COMMIT_MSG Commit Message: Line 16: snapshot. Snap name should be provided as argument, Line 17: that snapshot will be deleted. Returns success/ Line 18: failure. Line 19: Line 20: *glusterVolumeSnapshotDelete: Deletes gluster volume Its better to have glusterVolumeSnapshotDeleteAll Line 21: snapshot. Volume name should be provided as argument, Line 22: all snapshots for that volume will be deleted. Line 23: Returns success/failure. Line 24: http://gerrit.ovirt.org/#/c/34535/13/client/vdsClientGluster.py File client/vdsClientGluster.py: Line 772: serv.do_glusterSnapshotDelete, Line 773: ('snapName=<snap_name>', Line 774: 'delete gluster volume snapshot' Line 775: )), Line 776: 'glusterVolumeSnapshotDelete': ( You could place glusterVolumeSnapshot* and glusterSnapshot* functions in separate group than mixing them in the middle Line 777: serv.do_glusterVolumeSnapshotDelete, Line 778: ('volumeName=<volume name>', Line 779: 'delete all snapshots for given volume' Line 780: )), http://gerrit.ovirt.org/#/c/34535/13/vdsm/gluster/cli.py File vdsm/gluster/cli.py: Line 1092: return True Line 1093: Line 1094: Line 1095: @makePublic Line 1096: def snapshotDelete(snapName): Please consistent with gluster cli in cli.py Line 1097: command = _getGlusterSnapshotCmd() + ["delete", snapName] Line 1098: rc, out, err = _execGluster(command) Line 1099: if rc: Line 1100: raise ge.GlusterSnapshotDeleteFailedException(rc, out, err) Line 1094: Line 1095: @makePublic Line 1096: def snapshotDelete(snapName): Line 1097: command = _getGlusterSnapshotCmd() + ["delete", snapName] Line 1098: rc, out, err = _execGluster(command) doesn't it have xml support? Line 1099: if rc: Line 1100: raise ge.GlusterSnapshotDeleteFailedException(rc, out, err) Line 1101: else: -- To view, visit http://gerrit.ovirt.org/34535 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia2d1f1b2f2a675c5407222cb267a7560927ae84f Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Darshan N <dnara...@redhat.com> Gerrit-Reviewer: Bala.FA <barum...@redhat.com> Gerrit-Reviewer: Darshan N <dnara...@redhat.com> Gerrit-Reviewer: Shubhendu Tripathi <shtri...@redhat.com> Gerrit-Reviewer: Timothy Asir <tjeya...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
