Improve vbox-adapter-check #679
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The changes include the following enhancement to the `get_vm_uuids`
function:
- Rename the function to `get_vms` to reflect that it returns both the
name and UUID.
- Include the `{` and `}` in the matched UUID for consistency with the
function `get_vm_uuid` and the rest of printed information.
- Remove unneeded exception re-raising.
- Improve the function documentation.
- Simplifying the conditional logic for filtering dynamic VMs. The
condition is now inverted for better readability.
Move the logic to disable the nic adapter to the new function `disable_adapter` implementing the following enhacements: - Use `DISABLED_ADAPTER_TYPE` instead of its value (`"hostonly"`). - Add the string `nic` directly in the commands instead of in every of the elements of the `nics_with_internet` list.
Refactor the `change_network_adapters_to_hostonly` function including the following enhancements: - Rename the function `verify_network_adapters` to better reflect its main purpose. - Replace exception re-raising with a print that includes the error. - Improve printed information, using a consistent format and adding emojis for better readability. - Improve the function documentation.
Remove the unnecessary try-except block in the main function. Errors during VM verification are already handled within `verify_network_adapters`. Improve warning message for no VMs found.
Only send notification if an adapter is actually modified. Improve documentation to clarify the script's behavior and options, including when notifications are shown.
Add `list_to_str` function to improve readability of invalid NICs.
Verify the nic has been modified in `disable_adapter` as the command may return 0 even if it fails to set the adapter. Move logic to get configured network interfaces to a new function `get_nics` to avoid duplicating the code.
VMs in state `aborted` are also not running and running `VBoxManage controlvm` fails.
The `VBoxManage modifyvm --nic` command does not set the hostonly adapter. This change explicitly sets the hostonly adapter after disabling the NIC to prevent errors when starting the VM.
Improve documentation to reflect latest changes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR includes the following improvements in
vbox-adapter-check:disable_adapterfunction as the VBoxManage command may return 0 even if it fails to set the adapter.VBoxManage modifyvm --nicto ensure it has a name, fixing the bug that prevented the VM to start after disabling the NIC.abortedstate.get_vm_uuids, including giving it a name that better reflects its functionality and including the curly braces in the UUID for consistency with the rest of the code.disable_adapter.list_to_strfunction to improve readability of invalid NICs.I plan to send a PR after this one has been merged to provide automation via GH actions to generate a linux binary for vbox-adapter-check and vbox-remove-snapshots.