Code review comment for lp:~fcorrea/landscape-client/add-vminfo-to-computer-info-message

Looks good, just a few more minor things. +1 with those fixed!

[5]

+ path_join = os.path.join

and

+ def join_root_path(path):
+ return path_join(self._root_path, path)

I'd avoid setting the the path_join variable, just call os.path.join directly, like:

         def join_root_path(path):
             return os.path.join(self._root_path, path)

it's one less line of code and little tiny bit easier to parse (you don't have to know that path_join is os.path.join).

The same holds for

+ exists = os.path.exists

and

+ elif filter(exists, xen_paths):
+ virt_info = "xen"

[6]

There should be a test checking that if "proc/cpuinfo" exists and does *not* contain the "QEMU Virtual CPU" string, then the vm-info will be empty. Without such a test one could simply remove the "if" and "open" check and all tests would pass, like:

        elif exists(join_root_path("proc/cpuinfo")):
            virt_info = "kvm"

[7]

- "total-swap": 1584}
+ "total-swap": 1584, "vm-info": ""}

I think in this test (test_resynchronize) we should also pass a root_path=self.makeDir() to the plugin constructor, otherwise the test would fail when run on KVM machine (which is not unlikely if we want to run the suite in an Eucalyptus cloud instance)

[8]

There's a test missing for the openvz case.