Change in vdsm[master]: gluster: volume snapshot list verb.
dnarayan at redhat.com
dnarayan at redhat.com
Tue Feb 3 13:28:45 UTC 2015
Darshan N has posted comments on this change.
Change subject: gluster: volume snapshot list verb.
......................................................................
Patch Set 12:
(7 comments)
http://gerrit.ovirt.org/#/c/36087/12/vdsm/gluster/cli.py
File vdsm/gluster/cli.py:
Line 1232: volume[volumeName] = {}
Line 1233: volume[volumeName]['snapshots'] = []
Line 1234: volume[volumeName]['snapRemaining'] = tree.find(
Line 1235: 'snapInfo/originVolume/snapRemaining'
Line 1236: ).text
> how about
Done
Line 1237: volume[volumeName]['snapCount'] = tree.find(
Line 1238: 'snapInfo/originVolume/snapCount'
Line 1239: ).text
Line 1240: if volume[volumeName]['snapCount'] == 0:
Line 1235: 'snapInfo/originVolume/snapRemaining'
Line 1236: ).text
Line 1237: volume[volumeName]['snapCount'] = tree.find(
Line 1238: 'snapInfo/originVolume/snapCount'
Line 1239: ).text
> Please remove snapCount which is a duplicate and it can be get by len(volum
Done
Line 1240: if volume[volumeName]['snapCount'] == 0:
Line 1241: return {}
Line 1242: for el in tree.findall('snapInfo/snapshots/snapshot'):
Line 1243: snapshot = {}
Line 1236: ).text
Line 1237: volume[volumeName]['snapCount'] = tree.find(
Line 1238: 'snapInfo/originVolume/snapCount'
Line 1239: ).text
Line 1240: if volume[volumeName]['snapCount'] == 0:
> Does the output have missing 'snapInfo/snapshots' or 'snapInfo/snapshots' i
Done
Line 1241: return {}
Line 1242: for el in tree.findall('snapInfo/snapshots/snapshot'):
Line 1243: snapshot = {}
Line 1244: snapshot['snapshotUuid'] = el.find('uuid').text
Line 1242: for el in tree.findall('snapInfo/snapshots/snapshot'):
Line 1243: snapshot = {}
Line 1244: snapshot['snapshotUuid'] = el.find('uuid').text
Line 1245: snapshot['snapDescription'] = "" if el.find('description') is None\
Line 1246: else el.find('description').text
> cannot the below work?
if el.find('description') is None then the above statement will raise an exception
Line 1247: snapshot['createTime'] = el.find('createTime').text
Line 1248: snapshot['snapVolume'] = el.find('snapVolume/name').text
Line 1249: snapshot['snapVolumeStatus'] = el.find('snapVolume/status').text
Line 1250: snapshot['snapshotName'] = el.find('name').text
Line 1245: snapshot['snapDescription'] = "" if el.find('description') is None\
Line 1246: else el.find('description').text
Line 1247: snapshot['createTime'] = el.find('createTime').text
Line 1248: snapshot['snapVolume'] = el.find('snapVolume/name').text
Line 1249: snapshot['snapVolumeStatus'] = el.find('snapVolume/status').text
> you should send status as 'ACTIVATED' or 'DEACTIVATED' than 'Started' or so
Done
Line 1250: snapshot['snapshotName'] = el.find('name').text
Line 1251: volume[volumeName]['snapshots'].append(snapshot)
Line 1252: return volume
Line 1253:
Line 1293: volumes[volumeName]['snapRemaining'] = el.find(
Line 1294: 'snapVolume/originVolume/snapRemaining').text
Line 1295: volumes[volumeName]['snapCount'] = el.find(
Line 1296: 'snapVolume/originVolume/snapCount').text
Line 1297: volumes[volumeName]['snapshots'].append(snapshot)
> all above comments apply here
Done
Line 1298: return volumes
Line 1299:
Line 1300:
Line 1301: @makePublic
Line 1311: try:
Line 1312: if volumeName:
Line 1313: return _parseVolumeSnapshotList(xmltree)
Line 1314: else:
Line 1315: return _parseSnapshotList(xmltree)
> How about having this function name as _parseAllVolumeSnapshotList()? If y
Done
Line 1316: except _etreeExceptions:
--
To view, visit http://gerrit.ovirt.org/36087
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I06945d6781432b7fb40e417dd21c0ecf107de132
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Darshan N <dnarayan at redhat.com>
Gerrit-Reviewer: Bala.FA <barumuga at redhat.com>
Gerrit-Reviewer: Darshan N <dnarayan at redhat.com>
Gerrit-Reviewer: Sahina Bose <sabose at redhat.com>
Gerrit-Reviewer: Shubhendu Tripathi <shtripat at redhat.com>
Gerrit-Reviewer: Timothy Asir <tjeyasin at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list