Change in vdsm[master]: caps: refactor cpuinfo parsing

nsoffer at redhat.com nsoffer at redhat.com
Sat Jan 9 18:07:49 UTC 2016


Nir Soffer has posted comments on this change.

Change subject: caps: refactor cpuinfo parsing
......................................................................


Patch Set 1: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/51523/1/vdsm/caps.py
File vdsm/caps.py:

Line 87
Line 88
Line 89
Line 90
Line 91
This is too complex for no reason. We read and parse cpuinfo once per vdsm run, and there is no need to "optimize" this using complex code.

I would read the enough data for parsing, and use regex to extract the values needed - for example:

    with open("/proc/cpuinfo", "rb", 0) as f:
        data = f.read(4096)

    arch = cpuarch.real()

    if cpuarch.is_x86(arch):
        flags_re = re.compile(r"^flags\s*:\s*(.+?)\s*$", re.M)
        flags = flags_re.search(data).group(1)
        ...
    elif cpuarch.is_ppc(arch):
        flags = "powernv"
        ...

Another question, is why do we need a class - how clients are supposed to access cpuinfo?

We don't need to parse the info more then once, and we don't want clients to create an instance of the CPUInfo class. What we like to provide is simple functions returning the required data, eg. cpuinfo.flags().

It would be nice to parse cpu info on the first access and to avoid running code during import time, where we don't have yet logging setup for reporting errors.

We can something like this:

    CPUInfo = namedtuple("CPUInfo", "flags, mhz, model")

    @utils.memoized()
    def _cpuinfo():
        parse cpu info...
        return CPUInfo(flags, mhz, model)

    def flags():
         return _cpuinfo().flags


-- 
To view, visit https://gerrit.ovirt.org/51523
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib705bba4789bd1938bf26ae387af72b079020ce7
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list