Martin Peřina has posted comments on this change. Change subject: vdsm: adding handling for NGN in osinfo.py ......................................................................
Patch Set 3: (3 comments) https://gerrit.ovirt.org/#/c/57620/3/lib/vdsm/osinfo.py File lib/vdsm/osinfo.py: Line 51: Line 52: class OSName: Line 53: UNKNOWN = 'unknown' Line 54: OVIRT = 'oVirt Node' Line 55: OVIRT_NGN = 'oVirt New Generation Node' Hmm, as this is used only internally, I would shorten the name to 'oVirt NGN' Line 56: RHEL = 'RHEL' Line 57: FEDORA = 'Fedora' Line 58: RHEVH = 'RHEV Hypervisor' Line 59: DEBIAN = 'Debian' Line 92: @utils.memoized Line 93: def _release_name(): Line 94: if os.path.exists('/etc/rhev-hypervisor-release'): Line 95: return OSName.RHEVH Line 96: elif os.path.exists('/etc/os-release'): I'm just wondering if /etc/os-release file is not present in any other OS. Maybe it would be safe to use original file '/usr/lib/os.release.d/ovirt-release-host-node' instead of the '/etc/os-release' link ... Line 97: return OSName.OVIRT_NGN Line 98: elif glob.glob('/etc/ovirt-node-*-release'): Line 99: return OSName.OVIRT Line 100: elif os.path.exists('/etc/fedora-release'): Line 130: key, value = [kv.strip() for kv in line.split('=', 1)] Line 131: except ValueError: Line 132: continue Line 133: Line 134: data[key] = value Hmm, wouldn't it be better to replace surrounding quotes here? Line 135: Line 136: return ( Line 137: data.get('NAME', '').replace('"',''), Line 138: data.get('VERSION', '').replace('"',''), -- To view, visit https://gerrit.ovirt.org/57620 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I17337643cd4e986c09b07137d80da3555161ee70 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eli Mesika <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Peřina <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
