Skip to content

Conversation

@mchromin
Copy link
Contributor

No description provided.

@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

This comment was marked as outdated.

@adrianlasota adrianlasota requested a review from Copilot July 11, 2025 12:49
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Initial commit of the mfd-hyperv library, adding core Hyper-V management APIs, instance classes, unit tests, project metadata, and Sphinx documentation.

  • Implemented VSwitchManager, VMNetworkInterfaceManager, Hypervisor, VM, and related instance classes
  • Added comprehensive unit tests under tests/unit/test_mfd_hyperv
  • Configured packaging and documentation (pyproject.toml, requirements, Sphinx)

Reviewed Changes

Copilot reviewed 51 out of 55 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
mfd_hyperv/vswitch_manager.py Core VSwitchManager implementation
mfd_hyperv/vm_network_interface_manager.py VMNetworkInterfaceManager implementation
mfd_hyperv/hypervisor.py HypervHypervisor class with VM lifecycle APIs
mfd_hyperv/instances/*.py VSwitch, VM, VMNetworkInterface instance classes
tests/unit/test_mfd_hyperv/*.py Unit tests for all modules
sphinx-doc/* Initial Sphinx docs scaffolding
pyproject.toml & requirements-*.txt Project metadata and dependencies
Comments suppressed due to low confidence (1)

mfd_hyperv/vm_network_interface_manager.py:168

  • Remove stray double-quote at end of docstring line for :param interface_name.
        :param interface_name: name of VM network  seen from hypervisor"

:param vswitch_name: Name of virtual switch
:param interface_names: list of names of network interface that vswitch will be created on
:param enable_iov: is vswitch Sriov enabled
:param enable_teaming: is teaming oenabled (in case of multiple ports)
Copy link

Copilot AI Jul 11, 2025

Choose a reason for hiding this comment

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

Fix typo in docstring parameter description: change 'oenabled' to 'enabled' in ':param enable_teaming'.

Suggested change
:param enable_teaming: is teaming oenabled (in case of multiple ports)
:param enable_teaming: is teaming enabled (in case of multiple ports)

Copilot uses AI. Check for mistakes.
Comment on lines +13 to +18
apidoc.main(["-e", "-o", "mfd_hyperv", os.path.join("..", "mfd_hyperv")])

build.main(["-b", "html", ".", "build/html"])

logging.info("Cleaning folders from build process...")
shutil.rmtree("mfd_hyperv")
Copy link

Copilot AI Jul 11, 2025

Choose a reason for hiding this comment

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

[nitpick] Wrap script execution logic under if name == "main" guard to prevent unintended execution when this module is imported.

Suggested change
apidoc.main(["-e", "-o", "mfd_hyperv", os.path.join("..", "mfd_hyperv")])
build.main(["-b", "html", ".", "build/html"])
logging.info("Cleaning folders from build process...")
shutil.rmtree("mfd_hyperv")
if __name__ == "__main__":
apidoc.main(["-e", "-o", "mfd_hyperv", os.path.join("..", "mfd_hyperv")])
build.main(["-b", "html", ".", "build/html"])
logging.info("Cleaning folders from build process...")
shutil.rmtree("mfd_hyperv")

Copilot uses AI. Check for mistakes.
) -> None:
"""Set specified VMProcessorAttributes to specified value."""
if self.get_attributes()["State"] == "Off":
self.hyperv.vm_network_interface_manager.get_vm_interfaces(self.name, attribute, value)
Copy link

Copilot AI Jul 11, 2025

Choose a reason for hiding this comment

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

Incorrect method call in set_processor_attributes: should call hypervisor.set_vm_processor_attribute (or similar) instead of vm_network_interface_manager.get_vm_interfaces.

Suggested change
self.hyperv.vm_network_interface_manager.get_vm_interfaces(self.name, attribute, value)
self.hyperv.hypervisor.set_vm_processor_attribute(self.name, attribute, value)

Copilot uses AI. Check for mistakes.
:param vlan_id: VLAN Id
:param management_os whether to apply settings on virtual switch in the management OS
"""
assert state.lower() in ["access", "trunk", "promiscuous", "isolated", "untagged"]
Copy link

Copilot AI Jul 11, 2025

Choose a reason for hiding this comment

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

[nitpick] Use explicit exception raising (e.g., ValueError) for input validation instead of assert, because asserts can be disabled with Python optimizations.

Suggested change
assert state.lower() in ["access", "trunk", "promiscuous", "isolated", "untagged"]
if state.lower() not in ["access", "trunk", "promiscuous", "isolated", "untagged"]:
raise ValueError(f"Invalid state '{state}'. Must be one of ['access', 'trunk', 'promiscuous', 'isolated', 'untagged'].")

Copilot uses AI. Check for mistakes.
Comment on lines +66 to +74
cmd = (
'powershell.exe "New-VMSwitch'
f" -Name '{final_vswitch_name}' -NetAdapterName {interface_names} -AllowManagementOS $true"
f' -EnableIov ${enable_iov}"'
)
if enable_teaming:
cmd = cmd[:-1]
cmd += ' -EnableEmbeddedTeaming $true"'

Copy link

Copilot AI Jul 11, 2025

Choose a reason for hiding this comment

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

[nitpick] Avoid slicing strings to modify command lines; consider building the PowerShell command from a list of components or using a template for clarity and robustness.

Suggested change
cmd = (
'powershell.exe "New-VMSwitch'
f" -Name '{final_vswitch_name}' -NetAdapterName {interface_names} -AllowManagementOS $true"
f' -EnableIov ${enable_iov}"'
)
if enable_teaming:
cmd = cmd[:-1]
cmd += ' -EnableEmbeddedTeaming $true"'
cmd_components = [
'powershell.exe "New-VMSwitch',
f" -Name '{final_vswitch_name}'",
f" -NetAdapterName {interface_names}",
" -AllowManagementOS $true",
f" -EnableIov ${enable_iov}",
]
if enable_teaming:
cmd_components.append(" -EnableEmbeddedTeaming $true")
cmd_components.append('"')
cmd = "".join(cmd_components)

Copilot uses AI. Check for mistakes.
Signed-off-by: Mateusz Chrominski <mateusz.chrominski@intel.com>
@mfd-intel-bot
Copy link

We don't publish DEVs .whl.
To build .whl, run 'pip install git+https://github.com/intel/mfd-hyperv@ini'

1 similar comment
@mfd-intel-bot
Copy link

We don't publish DEVs .whl.
To build .whl, run 'pip install git+https://github.com/intel/mfd-hyperv@ini'

@mchromin mchromin merged commit 93eeaac into main Jul 11, 2025
5 checks passed
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.

5 participants