Skip to content

Conversation

@caseywilliams
Copy link
Contributor

Adds a new detector for VMware hypervisors.

The first commit adds a new item (BIOS address) to the set of data collected by the DMI detector. Determining the VMware version from inside a guest is tricky - there doesn't seem to be an official approach, but a few blog posts and repositories (example, and another) describe how the BIOS Address can be translated into a version - I included what translations I could find in vmware_detector.cc, but there are doubtless many others that could be added. This version information is also only available to root, since getting the BIOS address requires dmidecode.

The second commit adds the vmware detector; This is really similar to the existing VirtualBox detector.

@caseywilliams caseywilliams requested a review from Magisus July 19, 2017 23:43
@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 95.075% when pulling 4066641 on caseywilliams:vmware-detector into b2c9319 on puppetlabs:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 95.075% when pulling a5bf228 on caseywilliams:vmware-detector into b2c9319 on puppetlabs:master.

/**
* Retrieve the BIOS address
* @return The BIOS address
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this always available in the data from dmidecode, but just not always needed/useful for what we're trying to do? Or only under VMWare?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The BIOS address is always available from dmidecode (as long as the dmidecode output isn't totally empty), but as far as I've seen, VMware is the only place it's actually useful.

The DMI source object is getting pretty crowded as more single-case values like this are collected - I feel like there's probably a better way to organize this.


string vmware_bios_address_to_version(int address)
{
static const std::unordered_map<int, std::string> version_map {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems brittle, but we have plenty of other examples of similar maps that require manual attention. Probably not much else we can do here.

* @param address The value of the BIOS address from DMI
* @return The VMware version
*/
std::string vmware_bios_address_to_version(int address);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this doesn't need to be called outside of the vmware detector, it can just be a static method in that file - there's no need to put it in the header or even in the namespace.

@branan
Copy link
Contributor

branan commented Jul 25, 2017

This doesn't seem to include the change to the entry function to actually check for vmware?

@caseywilliams caseywilliams force-pushed the vmware-detector branch 2 times, most recently from c96d403 to 8672ce5 Compare July 25, 2017 23:21
@caseywilliams
Copy link
Contributor Author

Oops, that is a pretty important thing to forget! Updated now.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 94.468% when pulling 8672ce5 on caseywilliams:vmware-detector into 3ff73ce on puppetlabs:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 94.468% when pulling 8672ce5 on caseywilliams:vmware-detector into 3ff73ce on puppetlabs:master.

@branan
Copy link
Contributor

branan commented Jul 26, 2017

Coverage loss seems to be the one new line in the main query function, which is hard to unit-test properly. I'm cool with that loss 👍

Casey Williams added 2 commits July 26, 2017 11:16
Adds a new field - the BIOS address - to the dataset collected by the
DMI source. This field is only available via `dmidecode` (not
`/sys/class/dmi/`), which requires root. When available, it can be used
to determine hypervisor version for VMware.
Adds a detector function for VMware hypervisors. Uses the CPUID or DMI
data sources for basic detection. When run as root, it can collect the
BIOS address via `dmidecode` and use it to include the VMware version in
the result.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 96.703% when pulling 549ebdd on caseywilliams:vmware-detector into 48845a3 on puppetlabs:master.

@branan branan merged commit 81b1deb into puppetlabs:master Jul 26, 2017
@caseywilliams caseywilliams deleted the vmware-detector branch July 26, 2017 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants