Check for Virtual Machine #704
Conversation
Get the latest from upstream, merge, and test, before I send a pull request
Manufacturer and model check for Microsoft. Renamed the Map so its clearly states we are using the OUI portion.
Codecov Report
@@ Coverage Diff @@
## master #704 +/- ##
============================================
- Coverage 82.56% 82.52% -0.05%
Complexity 198 198
============================================
Files 28 28
Lines 1503 1528 +25
Branches 220 225 +5
============================================
+ Hits 1241 1261 +20
- Misses 126 128 +2
- Partials 136 139 +3
Continue to review full report at Codecov.
|
| String mac = nif.getMacaddr().substring(0, 8).toUpperCase(); | ||
| if (vmMacAddressOUI.containsKey(mac)) { | ||
| isvm = true; | ||
| return "On a VM: " + vmMacAddressOUI.get(mac); |
There was a problem hiding this comment.
Rather than adding the words "On a VM" let's just return the VM string here.
| if (manufacturer.equals("Microsoft Corporation") | ||
| && model.equals("Virtual Machine")) { | ||
| isvm = true; | ||
| return "On a VM: Microsoft Hyper-V"; |
There was a problem hiding this comment.
Similarly just return "Microsoft Hyper-V"
| for (String vm : vmModelArray) { | ||
| if (model.contains(vm)) { | ||
| isvm = true; | ||
| return "On a VM: " + vm; |
| return "On a VM: Microsoft Hyper-V"; | ||
| } | ||
| } | ||
| return "Couldn't detect VM"; |
There was a problem hiding this comment.
Return empty string if no vm. Update javadoc comment to explain this return value.
| System.out.println("virtualization: " + Util.identifyVM()); | ||
| } | ||
|
|
||
| private static void printComputerSystem(final ComputerSystem computerSystem) { |
There was a problem hiding this comment.
Minor point, you've got tabs in your editor rather than spaces. This causes unneeded whitespace changes in the diffs. Can you set your editor to use 4 spaces rather than a tab? If not, I can easily reformat and commit to your branch, but if it's an easy switch it'll make it easier.
| + String.format("%08x", processorSerialNumber.hashCode()) + delimiter | ||
| + String.format("%08x", processorIdentifier.hashCode()) + delimiter + processors; | ||
| } | ||
| private static final Logger LOG = LoggerFactory.getLogger(Util.class); |
There was a problem hiding this comment.
Nearly the entire class shows as a change in the diff because of the tabs/spaces... makes it harder to review! :)
|
Thanks @dbwiddis for the feedback. Sorry about the tab thing, it's a good reminder. I've made all changes you mentioned. Do I need to do a brand new PR? Or will this one here will continue automatically? |
|
Never mind. I know the answers to my 2 questions above. Let me know if you have more feedback. Thanks! |
|
You're good, pushing new changes to this branch automatically update the PR. |
|
Looks good, I'll make some minor tweaks in the morning and merge it! |
|
Wonderful, thanks @dbwiddis It has been pretty interesting and good learning experience. I appreciate your effort very much! I'd be happy to help out and do it again. |
|
Thanks for your contribution to open source! Hope to see you around more! |
Followed instructions here #648 (comment) . I also made a small change to SystemInfoTest.java for a good test. I hope that's OK. If not, I can roll that change back.
In addition to the code provided, I added a check using manufacturer and model for Microsoft Hyper-V VMs. I noticed that the Mac address and model check isn't enough for a VM I use for work, but manufacture and model check took care of that.
Thanks and let me know your comments/suggestions!