utils_net/utils_misc: add PowerShell support for windows nic attribute #4284
Conversation
WalkthroughAdds WMIC availability detection and PowerShell fallbacks in two utility modules. Introduces is_wmic_available(session) in virttest/utils_misc.py. get_win_disk_vol(session, condition) now uses WMIC when available or falls back to a PowerShell CIM query with Where-Object (converting "=" to "-eq"). virttest/utils_net.py gains a WMIC-unavailable path that uses a Get-NetAdapter PowerShell command and adjusts parsing to handle WMIC (headered) vs PowerShell (headerless) outputs. Error handling still raises on non-zero command results. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
virttest/utils_net.py (1)
1-4960: Black formatting check failed.The CI pipeline reports that this file would be reformatted by Black. Please run Black to ensure consistent code formatting.
Run the following to fix formatting:
#!/bin/bash # Format the file with Black black virttest/utils_net.py
🧹 Nitpick comments (1)
virttest/utils_misc.py (1)
2339-2350: Consider more specific exception handling.The bare
Exceptioncatch could hide authentication failures, network issues, or other meaningful errors. While this may be acceptable for a simple availability check, consider:
- Catching specific exceptions like
aexpect.ShellErrororaexpect.ShellTimeoutError- Or at minimum, logging the exception before returning False
Apply this diff if you want more specific exception handling:
def is_wmic_available(session): """ Check if 'wmic' is available on the Windows guest. :param session: Windows guest session :return: True if wmic can be executed, False otherwise """ try: status, _ = session.cmd_status_output("wmic /?", timeout=10) return status == 0 - except Exception: + except (aexpect.ShellError, aexpect.ShellTimeoutError) as e: + LOG.debug("WMIC availability check failed: %s", e) return FalseBased on static analysis hints.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
virttest/utils_misc.py(2 hunks)virttest/utils_net.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
virttest/utils_net.py (1)
virttest/utils_misc.py (1)
is_wmic_available(2339-2349)
🪛 GitHub Actions: CI
virttest/utils_net.py
[error] 1-1: Black formatting check failed. The following files would be reformatted by black: virttest/utils_net.py.
virttest/utils_misc.py
[error] 1-1: Black formatting check failed. The following files would be reformatted by black: virttest/utils_misc.py.
🪛 Ruff (0.14.8)
virttest/utils_misc.py
2347-2347: Consider moving this statement to an else block
(TRY300)
2348-2348: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: fedora_40 AVOCADO_SRC: SETUP:-m pip install PYPI_UPLOAD/*.whl VT_TYPE:libvirt
- GitHub Check: fedora_40 AVOCADO_SRC: SETUP:-m pip install . VT_TYPE:qemu
| def is_wmic_available(session): | ||
| """ | ||
| Check if 'wmic' is available on the Windows guest. | ||
| :param session: Windows guest session | ||
| :return: True if wmic can be executed, False otherwise | ||
| """ | ||
| try: | ||
| status, _ = session.cmd_status_output("wmic /?", timeout=10) | ||
| return status == 0 | ||
| except Exception: | ||
| return False | ||
|
|
There was a problem hiding this comment.
Fix Black formatting issues.
The pipeline indicates this file would be reformatted by Black. Please run black virttest/utils_misc.py to fix formatting issues before merging.
Also applies to: 2490-2497
🧰 Tools
🪛 Ruff (0.14.8)
2347-2347: Consider moving this statement to an else block
(TRY300)
2348-2348: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
In virttest/utils_misc.py around lines 2339-2350 (and also check 2490-2497), the
file has Black formatting issues flagged by CI; run the Black formatter (e.g.,
black virttest/utils_misc.py) or apply Black's style rules to reformat the
affected regions so the file conforms to Black's line breaks, spacing, and
string formatting conventions, then re-run tests/linters and commit the
reformatted file.
| if is_wmic_available(session): | ||
| cmd = "wmic logicaldisk where (%s) get DeviceID" % condition | ||
| else: | ||
| cmd = ( | ||
| 'PowerShell -NoProfile -Command ' | ||
| '"(Get-CimInstance Win32_LogicalDisk | Where-Object { $_.%s }).DeviceID"' | ||
| % condition.replace("=", "-eq") | ||
| ) |
There was a problem hiding this comment.
Fix missing spaces in PowerShell operator conversion.
The string replacement condition.replace("=", "-eq") will produce invalid PowerShell syntax. For example, with condition="VolumeName='WIN_UTILS'", the result is:
Where-Object { $_.VolumeName-eq'WIN_UTILS' }PowerShell requires spaces around operators:
Where-Object { $_.VolumeName -eq 'WIN_UTILS' }Apply this diff to fix the syntax error:
if is_wmic_available(session):
cmd = "wmic logicaldisk where (%s) get DeviceID" % condition
else:
cmd = (
'PowerShell -NoProfile -Command '
'"(Get-CimInstance Win32_LogicalDisk | Where-Object { $_.%s }).DeviceID"'
- % condition.replace("=", "-eq")
+ % condition.replace("=", " -eq ")
)🤖 Prompt for AI Agents
In virttest/utils_misc.py around lines 2490-2497, the PowerShell operator
conversion uses condition.replace("=", "-eq") which removes required spaces and
produces invalid PowerShell like "VolumeName-eq'WIN_UTILS'"; change the
replacement so it inserts spaces around the operator (for example replace "="
with " -eq "), or use a small regex to replace '=' with ' -eq ' only when not
already surrounded by spaces, so the generated Where-Object expression becomes "
$_.VolumeName -eq 'WIN_UTILS' ".
| if utils_misc.is_wmic_available(session): | ||
| cmd = 'wmic %s where %s="%s" get %s' % (global_switch, key, value, target) | ||
| else: | ||
| value_ps = value.replace(":", "-").upper() | ||
| cmd = ( | ||
| "PowerShell -NoProfile -Command " | ||
| "\"(Get-NetAdapter | Where-Object {{$_.{key} -eq '{value}'}}).Name\"" | ||
| ).format(key=key, value=value_ps) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n virttest/utils_net.py | sed -n '3940,3975p'Repository: avocado-framework/avocado-vt
Length of output: 1599
🏁 Script executed:
rg -n "get_windows_nic_attribute" virttest/utils_net.py | head -20Repository: avocado-framework/avocado-vt
Length of output: 411
🏁 Script executed:
cat -n virttest/utils_net.py | sed -n '1527,1536p'
cat -n virttest/utils_net.py | sed -n '1740,1750p'
cat -n virttest/utils_net.py | sed -n '4028,4035p'
cat -n virttest/utils_net.py | sed -n '4509,4518p'
cat -n virttest/utils_net.py | sed -n '4576,4585p'Repository: avocado-framework/avocado-vt
Length of output: 2233
🏁 Script executed:
cat -n virttest/utils_net.py | sed -n '1518,1540p'Repository: avocado-framework/avocado-vt
Length of output: 889
🏁 Script executed:
rg -n "wmic nic\|wmic nicconfig" virttest/ -A 2 -B 2 | head -40Repository: avocado-framework/avocado-vt
Length of output: 54
🌐 Web query:
WMIC nic vs nicconfig difference Windows
💡 Result:
Short answer: In WMIC the alias "nic" corresponds to the Win32_NetworkAdapter WMI class (the hardware/adapter object), while "nicconfig" corresponds to Win32_NetworkAdapterConfiguration (the TCP/IP–related settings for an adapter). "nic" exposes adapter/hardware properties and enable/disable methods; "nicconfig" exposes IP/DHCP/DNS/WINS and other TCP/IP configuration properties and methods (EnableDHCP, EnableStatic, EnableDNS, RenewDHCPLease, etc.). See the Microsoft docs. [1][2][3]
Sources:
- Win32_NetworkAdapterConfiguration (nicconfig). [1]
- Win32_NetworkAdapterSetting (association between adapter and configuration). [2]
- Win32_NetworkAdapter (nic). [3]
🌐 Web query:
PowerShell Get-NetAdapter vs Get-NetIPConfiguration WMIC equivalent
💡 Result:
Short answer
- Modern PowerShell equivalents for WMIC network queries are Get-NetAdapter (roughly WMIC nic) and Get-NetIPConfiguration (roughly WMIC nicconfig). [1][2]
More detail + examples
-
Get-NetAdapter — returns adapter objects (name, index, status, description). Use instead of wmic nic. Example:
Get-NetAdapter
[2] -
Get-NetIPConfiguration — returns IP addresses, gateways, DNS per interface. Use instead of wmic nicconfig. Example:
Get-NetIPConfiguration
[1][2] -
If you need the underlying WMI/CIM classes, use Get-CimInstance (preferred) or Get-WmiObject against Win32_NetworkAdapter and Win32_NetworkAdapterConfiguration:
Get-CimInstance -ClassName Win32_NetworkAdapter
Get-CimInstance -ClassName Win32_NetworkAdapterConfiguration
(these map to the old WMIC classes). [3]
References
- Get-NetIPConfiguration / Get-Net* cmdlets documentation. [1][2]
- Win32_NetworkAdapterConfiguration (WMI class) — Microsoft Learn. [3]
🌐 Web query:
Get-NetAdapter PowerShell netconnectionid property Windows
💡 Result:
Short answer: Get-NetAdapter doesn't expose the WMI NetConnectionID property directly — it shows the NetAdapter module's Name/InterfaceAlias. To read or change the NetConnectionID you must use WMI/CIM (Win32_NetworkAdapter) or use the NetAdapter cmdlets to rename the adapter (InterfaceAlias).
Useful commands
-
Show NetAdapter properties (Module name / InterfaceAlias):
Get-NetAdapter | Format-List * -
Read the WMI NetConnectionID:
Get-CimInstance -ClassName Win32_NetworkAdapter | Select-Object DeviceID, Name, NetConnectionID -
Rename (update NetConnectionID via WMI/CIM):
$a = Get-CimInstance -ClassName Win32_NetworkAdapter -Filter "NetConnectionID='OldName'"
$props = @{NetConnectionID='NewName'}
Set-CimInstance -InputObject $a -Property $props -
Or use the NetAdapter cmdlet to rename the interface (changes InterfaceAlias shown by Get-NetAdapter):
Rename-NetAdapter -Name "OldAlias" -NewName "NewAlias"
References: Microsoft blog (renaming adapters / NetConnectionID via WMI) [1]; Get-NetAdapter overview / examples [2].
🏁 Script executed:
cat -n virttest/utils_net.py | sed -n '3955,3974p'Repository: avocado-framework/avocado-vt
Length of output: 1072
PowerShell fallback doesn't respect global_switch parameter and uses wrong cmdlet for multiple call cases.
The WMIC path uses global_switch to choose between wmic nic (Win32_NetworkAdapter) and wmic nicconfig (Win32_NetworkAdapterConfiguration), but the PowerShell path always uses Get-NetAdapter and hardcodes output to .Name.
Critical issues:
- When
global_switch="nicconfig"withtarget="IPAddress"(line 1530-1531), the function should return IP configuration data but instead returns the adapter name. - When
target="netconnectionid"(line 1743-1744),Get-NetAdapterdoesn't expose theNetConnectionIDproperty at all—this requiresGet-CimInstance Win32_NetworkAdapteror similar. - The
targetparameter is entirely ignored, hardcoded to.Namein line 3961.
The PowerShell fallback must conditionally use different cmdlets:
- For
global_switch="nic": useGet-CimInstance Win32_NetworkAdapter(has NetConnectionID, DeviceID, etc.) - For
global_switch="nicconfig": useGet-CimInstance Win32_NetworkAdapterConfiguration(has IPAddress, DNSServerSearchOrder, etc.)
Both must support the target parameter dynamically instead of hardcoding to .Name.
🤖 Prompt for AI Agents
In virttest/utils_net.py around lines 3955 to 3962, the PowerShell fallback
ignores global_switch and hardcodes the property to .Name; change it to pick the
correct CIM class and property dynamically: if global_switch == "nic" use
Get-CimInstance Win32_NetworkAdapter, if global_switch == "nicconfig" use
Get-CimInstance Win32_NetworkAdapterConfiguration, and build the PowerShell
command to compare the requested key to the processed value and return the
requested target property (not always .Name). Preserve the value normalization
as needed (e.g., replacing ":" with "-"/upper for MAC checks) and ensure the
target parameter is inserted into the output expression so the command returns
the same data WMIC would for the corresponding global_switch/target
combinations.
| value_ps = value.replace(":", "-").upper() | ||
| cmd = ( | ||
| "PowerShell -NoProfile -Command " | ||
| "\"(Get-NetAdapter | Where-Object {{$_.{key} -eq '{value}'}}).Name\"" | ||
| ).format(key=key, value=value_ps) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n virttest/utils_net.py | sed -n '3940,3980p'Repository: avocado-framework/avocado-vt
Length of output: 1810
🏁 Script executed:
rg -n "get_windows_nic_attribute" --type=py -B2 -A2Repository: avocado-framework/avocado-vt
Length of output: 2027
🏁 Script executed:
cat -n virttest/utils_net.py | sed -n '1525,1535p'Repository: avocado-framework/avocado-vt
Length of output: 560
🏁 Script executed:
cat -n virttest/utils_net.py | sed -n '3958,3963p'Repository: avocado-framework/avocado-vt
Length of output: 408
Critical: PowerShell command hardcodes .Name instead of using the target parameter.
The PowerShell command on line 3961 always returns the adapter name, ignoring the target parameter passed to the function. This breaks calls with target="IPAddress" (line 1530), target="netconnectionid" (lines 1743, 4512, 4579), and other non-Name attributes.
Fix line 3961 to use the target parameter:
value_ps = value.replace(":", "-").upper()
cmd = (
"PowerShell -NoProfile -Command "
- "\"(Get-NetAdapter | Where-Object {{$_.{key} -eq '{value}'}}).Name\""
+ "\"(Get-NetAdapter | Where-Object {{$_.{key} -eq '{value}'}}).{target}\""
- ).format(key=key, value=value_ps)
+ ).format(key=key, value=value_ps, target=target)Additionally, the PowerShell path does not respect the global_switch parameter. Line 1531 calls this function with global_switch="nicconfig", but the PowerShell fallback always uses Get-NetAdapter (nic context). Consider whether a different PowerShell cmdlet should be used for the nicconfig context.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| value_ps = value.replace(":", "-").upper() | |
| cmd = ( | |
| "PowerShell -NoProfile -Command " | |
| "\"(Get-NetAdapter | Where-Object {{$_.{key} -eq '{value}'}}).Name\"" | |
| ).format(key=key, value=value_ps) | |
| value_ps = value.replace(":", "-").upper() | |
| cmd = ( | |
| "PowerShell -NoProfile -Command " | |
| "\"(Get-NetAdapter | Where-Object {{$_.{key} -eq '{value}'}}).{target}\"" | |
| ).format(key=key, value=value_ps, target=target) |
🤖 Prompt for AI Agents
In virttest/utils_net.py around lines 3958-3962, the PowerShell string is
hardcoded to return the adapter Name (".Name") instead of using the function's
target parameter and it always calls Get-NetAdapter regardless of global_switch;
change the formatted property access to use the target parameter (i.e. use
".{target}" not ".Name") and add logic to select the proper PowerShell cmdlet
when global_switch indicates "nicconfig" (for example use Get-NetIPConfiguration
for IP-related targets or Get-NetAdapterAdvancedProperty/Get-NetAdapterBinding
when querying adapter configuration attributes) so the constructed command uses
the correct cmdlet and property based on target/global_switch.
| if utils_misc.is_wmic_available(session): | ||
| cmd = 'wmic %s where %s="%s" get %s' % (global_switch, key, value, target) | ||
| else: | ||
| value_ps = value.replace(":", "-").upper() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let me find and examine the function around line 3958
head -3970 virttest/utils_net.py | tail -30Repository: avocado-framework/avocado-vt
Length of output: 1174
🏁 Script executed:
# Get more context around line 3958
sed -n '3940,3980p' virttest/utils_net.pyRepository: avocado-framework/avocado-vt
Length of output: 1523
🏁 Script executed:
# Find the function name and signature
sed -n '3920,3960p' virttest/utils_net.pyRepository: avocado-framework/avocado-vt
Length of output: 1354
🏁 Script executed:
# Search for all calls to get_windows_nic_attribute
rg -n 'get_windows_nic_attribute\(' --type=py -A2 -B2Repository: avocado-framework/avocado-vt
Length of output: 2027
🏁 Script executed:
# Check the function context around line 4031
sed -n '4000,4035p' virttest/utils_net.pyRepository: avocado-framework/avocado-vt
Length of output: 1400
🏁 Script executed:
# Check the complete function that calls get_windows_nic_attribute at line 4031
sed -n '3985,4040p' virttest/utils_net.pyRepository: avocado-framework/avocado-vt
Length of output: 2108
🏁 Script executed:
# Search for documentation or comments about WMIC properties that might contain colons
rg -n "macaddress|pnpdeviceid|netconnectionid" virttest/utils_net.py | head -20Repository: avocado-framework/avocado-vt
Length of output: 696
🏁 Script executed:
# Check if there are test cases that call restart_windows_guest_network_by_key with non-MAC keys
rg -r 'restart_windows_guest_network_by_key' --type=py -A3 -B1Repository: avocado-framework/avocado-vt
Length of output: 468
MAC address format conversion applied unconditionally to all values.
Line 3958 in the PowerShell execution path converts all values by replacing colons with dashes and uppercasing, but the function accepts arbitrary WMIC property keys. While all current call sites use key="macaddress", the function signature allows any key, and non-MAC properties containing colons would be corrupted by this conversion.
Apply the conversion only for MAC address-related keys:
else:
- value_ps = value.replace(":", "-").upper()
+ value_ps = value.replace(":", "-").upper() if "mac" in key.lower() else value
cmd = (🤖 Prompt for AI Agents
In virttest/utils_net.py around line 3958, the code unconditionally transforms
value with value.replace(":", "-").upper(), which corrupts non-MAC WMIC
properties; change it so the colon-to-dash and uppercasing is only applied for
MAC address keys (compare key.lower() against a small whitelist such as
{"macaddress", "mac"} or otherwise detect MAC-related names), leaving other
keys' values untouched and preserving None/empty values.
virttest/utils_net.py
Outdated
| lines = [l.strip() for l in out.splitlines() if l.strip()] | ||
| # First line is header, return second line | ||
| return lines[1] | ||
| # WMIC output has a header (value is in lines[1]) | ||
| #PowerShell output has no header, so return out.strip() | ||
| return lines[1] if len(lines) > 1 else out.strip() |
There was a problem hiding this comment.
Output parsing logic is unclear for WMIC vs PowerShell.
The parsing logic tries to handle both output formats with a single conditional, but the semantics are different:
- WMIC: header + value → return
lines[1] - PowerShell: no header → return first line or full output
If PowerShell output has multiple lines (e.g., comma-separated IPAddress list), returning lines[1] may skip the first value. Also, for WMIC with only a header (no data), len(lines) == 1 returns out.strip() instead of handling the missing data case.
Consider parsing based on which command was executed:
status, out = session.cmd_status_output(cmd, timeout=timeout)
if status != 0:
err_msg = "Execute guest shell command('%s' "failed with error: '%s'" % (
cmd,
out,
)
raise exceptions.TestError(err_msg)
lines = [l.strip() for l in out.splitlines() if l.strip()]
- # WMIC output has a header (value is in lines[1])
- #PowerShell output has no header, so return out.strip()
- return lines[1] if len(lines) > 1 else out.strip()
+ if utils_misc.is_wmic_available(session):
+ # WMIC output has a header (value is in lines[1])
+ return lines[1] if len(lines) > 1 else None
+ else:
+ # PowerShell output has no header
+ return out.strip()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| lines = [l.strip() for l in out.splitlines() if l.strip()] | |
| # First line is header, return second line | |
| return lines[1] | |
| # WMIC output has a header (value is in lines[1]) | |
| #PowerShell output has no header, so return out.strip() | |
| return lines[1] if len(lines) > 1 else out.strip() | |
| lines = [line.strip() for line in out.splitlines() if line.strip()] | |
| # WMIC output has a header (value is in lines[1]) | |
| #PowerShell output has no header, so return out.strip() | |
| return lines[1] if len(lines) > 1 else out.strip() |
🧰 Tools
🪛 Ruff (0.14.8)
3970-3970: Ambiguous variable name: l
(E741)
Signed-off-by: Wenli Quan <wquan@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
virttest/utils_net.py (1)
3955-3962: PowerShell fallback still ignorestarget/global_switchand over-normalizesvalueIn WMIC-less Windows guests this branch is effectively the only path, but it has several behavioral problems:
global_switchis only honored in the WMIC branch. For callers such asget_net_if_addrs_win()(Line 1530) that passglobal_switch="nicconfig"andtarget="IPAddress", the PowerShell path still usesGet-NetAdapter, which doesn’t exposeIPAddressin the same way asWin32_NetworkAdapterConfiguration. So IP discovery on WMIC-less guests will not work.targetis ignored in the PowerShell path: the command is hardcoded to return.Name. All call sites that expect other attributes (IPAddress,netconnectionid,pnpdeviceid) will receive the adapter name instead, causing misbehavior in:
get_net_if_addrs_win()(expects an IPAddress-like string, gets a name instead).restart_guest_network()/restart_windows_guest_network_by_key()/set_netkvm_param_value()(expect a connection ID or PNP device ID).value_ps = value.replace(":", "-").upper()is applied unconditionally; while it’s correct for MAC addresses (matchingGet-NetAdapter’sMacAddressformat), it will corrupt any non-MAC keys that contain colons (for example PNP device ids or other future keys).These issues mean that most existing uses of
get_windows_nic_attribute()will misbehave when WMIC is unavailable, which defeats the purpose of the fallback.At minimum, I recommend:
- Use
targetinstead of hardcoding.Namein the PowerShell expression.- Only normalize MAC-address formatting when the key is MAC-like.
- (Preferable but larger change) Respect
global_switchby using the appropriate CIM class instead of alwaysGet-NetAdapter:
global_switch == "nic"→Get-CimInstance -ClassName Win32_NetworkAdapterglobal_switch == "nicconfig"→Get-CimInstance -ClassName Win32_NetworkAdapterConfiguration
and still project.{target}from the resulting object, keeping semantics aligned with the WMIC path.A minimal, incremental fix inside the current design would look like:
- else: - value_ps = value.replace(":", "-").upper() - cmd = ( - "PowerShell -NoProfile -Command " - "\"(Get-NetAdapter | Where-Object {{$_.{key} -eq '{value}'}}).Name\"" - ).format(key=key, value=value_ps) + else: + # Only normalize MAC-like keys for Get-NetAdapter matching + if key and "mac" in key.lower(): + value_ps = value.replace(":", "-").upper() + else: + value_ps = value + cmd = ( + "PowerShell -NoProfile -Command " + "\"(Get-NetAdapter | Where-Object {{$_.{key} -eq '{value}'}}).{target}\"" + ).format(key=key, value=value_ps, target=target)That still leaves the
nicconfig/IPAddresscase without a proper PowerShell/CIM equivalent (you’ll likely need aGet-CimInstance Win32_NetworkAdapterConfiguration-based path there), but at least prevents obviously wrong attribute selection and over-aggressive normalization.In modern PowerShell on Windows (10/11 and Server), what are the recommended cmdlets/CIM classes to replace `wmic nic` and `wmic nicconfig` queries, and which properties map to `NetConnectionID`, `PNPDeviceID`, and `IPAddress`? Do `Get-NetAdapter` objects expose `NetConnectionID` or IP address data directly, or should `Get-CimInstance Win32_NetworkAdapter*` / `Get-NetIPConfiguration` be used instead?virttest/utils_misc.py (1)
2490-2497: Fix missing spaces in PowerShell operator conversion.This issue was previously identified but remains unresolved. The
condition.replace("=", "-eq")produces invalid PowerShell syntax by removing required spaces around the operator.Example with
condition="VolumeName='WIN_UTILS'":# Current (invalid): Where-Object { $_.VolumeName-eq'WIN_UTILS' } # Required (valid): Where-Object { $_.VolumeName -eq 'WIN_UTILS' }Apply this diff to fix the syntax error:
if is_wmic_available(session): cmd = "wmic logicaldisk where (%s) get DeviceID" % condition else: cmd = ( "PowerShell -NoProfile -Command " '"(Get-CimInstance Win32_LogicalDisk | Where-Object { $_.%s }).DeviceID"' - % condition.replace("=", "-eq") + % condition.replace("=", " -eq ") )Additionally, the Black formatter flagged formatting issues in this file. Please run
black virttest/utils_misc.pybefore merging.
🧹 Nitpick comments (2)
virttest/utils_net.py (1)
3971-3973: Output parsing conflates WMIC and PowerShell semantics; comments are misleadingThe comments say WMIC has a header and PowerShell does not, but the return logic:
lines = [l.strip() for l in out.splitlines() if l.strip()] return lines[1] if len(lines) > 1 else out.strip()doesn’t actually branch on which backend was used. It happens to work for the common WMIC case (header + single value), and for single-line PowerShell outputs, but it’s brittle when:
- WMIC returns only a header or no data (you’ll return the header instead of a “no value” indicator).
- PowerShell returns multiple values (arrays like IPAddress) — you silently drop the first item by returning
lines[1].Given you now explicitly choose between WMIC and PowerShell above, consider carrying a
wmic_availableflag and parsing accordingly, for example:
- WMIC path: expect and use
lines[1]when present, otherwise treat as “no value”.- PowerShell path: return
out.strip()(or possibly first line) and let callers handle multi-valued properties explicitly.Right now this is more of a robustness/clarity issue than a functional bug for the basic single-value cases.
virttest/utils_misc.py (1)
2339-2350: Enhance exception handling and add debug logging.While the broad exception catch is acceptable for an availability check, consider these improvements:
- Add debug logging when exceptions occur to aid troubleshooting
- The static analysis suggests restructuring per TRY300
Apply this diff to improve the implementation:
def is_wmic_available(session): """ Check if 'wmic' is available on the Windows guest. :param session: Windows guest session :return: True if wmic can be executed, False otherwise """ try: status, _ = session.cmd_status_output("wmic /?", timeout=10) - return status == 0 except Exception: + LOG.debug("Exception occurred while checking WMIC availability", exc_info=True) return False + else: + return status == 0
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
virttest/utils_misc.py(2 hunks)virttest/utils_net.py(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-31T09:32:06.334Z
Learnt from: leidwang
Repo: avocado-framework/avocado-vt PR: 4034
File: virttest/utils_windows/drive.py:38-38
Timestamp: 2025-07-31T09:32:06.334Z
Learning: In virttest/utils_windows/drive.py, the `_logical_disks` function processes condition strings by splitting on "=" and converting them to PowerShell Where-Object filters using the `-like` operator, which preserves pattern matching functionality even when the input condition uses "=" format.
Applied to files:
virttest/utils_misc.py
🧬 Code graph analysis (2)
virttest/utils_net.py (1)
virttest/utils_misc.py (1)
is_wmic_available(2339-2349)
virttest/utils_misc.py (1)
virttest/remote.py (1)
cmd_status_output(718-727)
🪛 Ruff (0.14.8)
virttest/utils_misc.py
2347-2347: Consider moving this statement to an else block
(TRY300)
2348-2348: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: fedora_40 AVOCADO_SRC: SETUP:-m pip install PYPI_UPLOAD/*.whl VT_TYPE:qemu
- GitHub Check: fedora_40 AVOCADO_SRC:avocado-framework<104.0 SETUP:-m pip install . VT_TYPE:qemu
- GitHub Check: fedora_40 AVOCADO_SRC: SETUP:-m pip install . VT_TYPE:qemu
1 participant
Signed-off-by: Wenli Quan wquan@redhat.com
id: 4835, 4838
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.