-
Notifications
You must be signed in to change notification settings - Fork 251
Replace WMIC with powershell cmd #4034
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
97073f0
to
194ab37
Compare
1c9d9ea
to
7c4813a
Compare
9f86791
to
d1bcda5
Compare
Test 271 acceptance with win2022,win11,win1032 and win2016 guests, this patch worked. |
Hi @XueqiangWei @nanliu-r @fbq815 Would you please help test this MR, it is mainly for wmic repalcement.Thanks. |
Acceptance test looks good. |
balloon_hotplug test cases now passed in Win2025
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Hi @yanan-fu @YongxueHong Would you please help review this MR?Thanks. |
return out if out else [] | ||
c_name, c_value = cond.split("=") | ||
cmd = ( | ||
'powershell -command "Get-CimInstance -ClassName Win32_LogicalDisk | Where-Object {$_.%s -like %s}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The disk needs to be under single quotes, same you've done in utils_netperf.py and replace the existing ones by double quotes
'powershell -command "Get-CimInstance -ClassName Win32_LogicalDisk | Where-Object {$_.%s -like %s}' | |
"powershell -command \"Get-CimInstance -ClassName Win32_LogicalDisk | Where-Object {$_.%s -like '%s'}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @mcasquer Thank you for your suggestion, actually, c_value
already has quotes, you can check line37.
virttest/utils_windows/drive.py
Outdated
return mapping | ||
return {} | ||
cmd = ( | ||
'powershell -command "Get-CimInstance -ClassName Win32_Diskdrive | Where-Object {$_.SerialNumber -eq %s}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
'powershell -command "Get-CimInstance -ClassName Win32_Diskdrive | Where-Object {$_.SerialNumber -eq %s}' | |
"powershell -command \"Get-CimInstance -ClassName Win32_Diskdrive | Where-Object {$_.SerialNumber -eq '%s'}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, let me confirm it and update the code.
results.append(vals[0]) | ||
else: | ||
results.append(dict(zip(keys, vals))) | ||
return results if results else [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@leidwang
Another comment here, this implementation seems to no longer return a map, so this will probably break all the calls to this function.
octets = subnet_mask.split(".") | ||
|
||
prefix_length = 0 | ||
for octet in octets: | ||
prefix_length += bin(int(octet)).count("1") | ||
|
||
return prefix_length |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
octets = subnet_mask.split(".") | |
prefix_length = 0 | |
for octet in octets: | |
prefix_length += bin(int(octet)).count("1") | |
return prefix_length | |
return sum(bin(int(octet)).count("1") for octet in subnet_mask.split(".")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please note that the netmask in set_guest_ip_addr
can be CIDR AND dot-decimal notation, only when users give a dot-decimal notation, we have to convert it to a bitmask.
However, the new function is a duplicate of convert_netmask
in utils_net
module, and set_guest_ip_addr
also used it.
avocado-vt/virttest/utils_net.py
Lines 1641 to 1647 in 731ecab
nic_ifname = get_linux_ifname(session, mac) | |
if session.cmd_status("which ip") != 0: | |
info_cmd = "ifconfig -a; ethtool -S %s" % nic_ifname | |
cmd = "ifconfig %s %s netmask %s" % (nic_ifname, ip_addr, netmask) | |
else: | |
if "." in netmask: | |
netmask = convert_netmask(netmask) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW, the current convert_netmask
can only handle dot-decimal notation, as we already import ipaddress
, an enhancement here is to use it to handle all types of netmask.
>>> import ipaddress
>>> ip_addr = "192.168.1.1"
>>> netmask = "24" # or "255.255.255.0"
>>> ipaddr = ipaddress.IPv4Network(f"{ip_addr}/{netmask}", strict=False)
>>> ipaddr.netmask
IPv4Address('255.255.255.0')
>>> ipaddr.prefixlen
24
And then you can use it for linux/windows
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @YongxueHong @PaulYuuu , let me update my code.
@@ -3941,19 +3968,22 @@ def update_mac_ip_address(vm, timeout=240): | |||
|
|||
|
|||
def get_windows_nic_attribute( | |||
session, key, value, target, timeout=240, global_switch="nic" | |||
session, key, value, target, timeout=240, global_switch="NetworkAdapter" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @xiagao
Could you help to review if it will affect the test case: https://github.com/autotest/tp-qemu/blob/6f0573097c77bb5d43b1c52574e12f5735714995/qemu/tests/win_virtio_driver_update_by_installer.py#L108-L109
Thanks.
fb627bc
to
e8aaf97
Compare
|
||
:return: volume ID. | ||
""" | ||
cmd = "wmic logicaldisk where (%s) get DeviceID" % condition | ||
c_name, c_value = condition.split("=") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@leidwang this logic seems to be incorrect. In the balloon_hotplug variants I am hitting the following:
[stdlog] 2025-03-03 15:09:30,148 avocado.test stacktrace L0050 ERROR| File "/usr/local/lib/python3.9/site-packages/avocado_framework_plugin_vt-109.0-py3.9.egg/virttest/utils_misc.py", line 2482, in get_win_disk_vol
[stdlog] 2025-03-03 15:09:30,148 avocado.test stacktrace L0050 ERROR| c_name, c_value = condition.split("=")
[stdlog] 2025-03-03 15:09:30,148 avocado.test stacktrace L0050 ERROR| ValueError: not enough values to unpack (expected 2, got 1)
Which leads to:
avocado.core.exceptions.TestError: Could not get virtio-win disk vol!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same for balloon_service and balloon_memhp variants
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add the full debug.log to issue3268, thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@leidwang sorry for the late reply, debug.log updated !
@@ -488,7 +488,7 @@ def get_guest_total_mem(cls, vm): | |||
:return: physical memory report by guest OS in MB | |||
""" | |||
if vm.params.get("os_type") == "windows": | |||
cmd = "wmic ComputerSystem get TotalPhysicalMemory" | |||
cmd = 'powershell -command "(Get-CimInstance -ClassName Win32_ComputerSystem).TotalPhysicalMemory"' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@leidwang also hit a timeout with the following command, either executing it on the test loop or individually:
ERROR: Timeout expired while waiting for shell command to complete: 'powershell -command "(Get-CimInstance -ClassName Win32_ComputerSystem).TotalPhysicalMemory"' (output: '')
Hello @leidwang is there any plan to update this patch ? I am still hitting wmic command issues in Win11 recent test runs 😕 |
Hi @mcasquer I will update this MR recently, sorry for the delay.Currently, if you hit the issue that wmic not installed, you can try to run unattended_install test case again, it should be helpful. Just want to explain more about why we will hit this issue. Win11-24H2 and Win2025 remove wmic, and does not provide installer for wmic, so we have to install it from MSFT, which then requires downloading something from the MSFT server,, so the wmic installation sometimes fails. |
@leidwang ok, I get the idea, but if I trigger an automated test loop I cannot run the unattended_install twice, which significantly affects automation purpose... |
Yes, we have to replace all wmic related command in our framework and test cases, then this issue can be fully fixed. |
From Win11-24H2, WMIC is an optional feature for Windows, And it will be fully removed in the future.So replace the related cmd Signed-off-by: Leidong Wang <[email protected]>
e8aaf97
to
4e02e10
Compare
Tested with vTPM test cases. LGTM |
From Win11-24H2, WMIC is an optional feature for Windows, And it will be fully removed in the future.So replace the related cmd
ID: 3268