Ala Hino has posted comments on this change.
Change subject: virt: enable libgfapi
......................................................................
Patch Set 4:
(4 comments)
https://gerrit.ovirt.org/#/c/44061/4/vdsm/clientIF.py
File vdsm/clientIF.py:
Line 319: # The order of imgVolumesInfo is not guaranteed
Line 320: drive['volumeChain'] = res['imgVolumesInfo']
Line 321: drive['volumeInfo'] = res['info']
Line 322: if drive.get('diskType') == DISK_TYPE.NETWORK:
Line 323: volinfo = drive.get('volumeInfo')
Get volinfo from res['info']
Done
Line 324: volPath = volinfo['path']
Line 325: drive['protocol'] = volinfo['protocol']
Line 326: if drive.get('hosts') is None:
Line 327: hosts = [dict(name=volinfo['hosts'][0],
Line 322: if drive.get('diskType') == DISK_TYPE.NETWORK:
Line 323: volinfo = drive.get('volumeInfo')
Line 324: volPath = volinfo['path']
Line 325: drive['protocol'] = volinfo['protocol']
Line 326: if drive.get('hosts') is None:
Why do you allow hosts on the drive? lets simplify and be consistent.
Eithe
In ceph we get hosts from caller (engine) while in glster, caller doesn't
provide hosts
Line 327: hosts = [dict(name=volinfo['hosts'][0],
Line 328: port=volinfo['port'],
Line 329: transport=volinfo['transport'])]
Line 330: drive['hosts'] = hosts
Line 325: drive['protocol'] = volinfo['protocol']
Line 326: if drive.get('hosts') is None:
Line 327: hosts = [dict(name=volinfo['hosts'][0],
Line 328: port=volinfo['port'],
Line 329: transport=volinfo['transport'])]
Correction, we should take here only the first host, since libvirt
does no
Done
Line 330: drive['hosts'] = hosts
Line 331:
Line 332: # GUID drive format
Line 333: elif "GUID" in drive:
https://gerrit.ovirt.org/#/c/44061/4/vdsm/storage/glusterVolume.py
File vdsm/storage/glusterVolume.py:
Line 56: return {'volType': VmVolumeInfo.TYPE_NETWORK, 'path':
glusterPath,
Line 57: 'protocol': 'gluster', 'port': volPort,
Line 58: 'transport': volTrans,
Line 59: 'volfileServer': volfileServer,
Line 60: 'hosts': hosts}
Please drop the old format and return exactly what is required in
prepareVo
Done
--
To view, visit
https://gerrit.ovirt.org/44061
To unsubscribe, visit
https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I54b81e87b959b0b49c0f06810f88410e7c75de1d
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino <ahino(a)redhat.com>
Gerrit-Reviewer: Ala Hino <ahino(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes