Skip to content
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

Netkvm: Fix multi_nics_verify test by adding NIC initialization check #4175

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

heywji
Copy link
Contributor

@heywji heywji commented Oct 11, 2024

The test failed because Windows didn't assign IPs to all NICs immediately after boot. A loop was added to check the NIC count until all are initialized, ensuring the test waits for all NICs to get their IP addresses before proceeding.

ID: 2393
Signed-off-by: wji [email protected]

@heywji
Copy link
Contributor Author

heywji commented Oct 11, 2024

Test Result:

1-Host_RHEL.m9.u6.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.Win11.x86_64.io-github-autotest-qemu.unattended_install.cdrom.extra_cdrom_ks.default_install.aio_threads.q35 Finshed 2024-10-11 13:33:30 2024-10-11 14:05:57 PASS 1947.032257
2-Host_RHEL.m9.u6.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.Win2022.x86_64.io-github-autotest-qemu.unattended_install.cdrom.extra_cdrom_ks.default_install.aio_threads.q35 Finshed 2024-10-11 14:05:58 2024-10-11 14:31:40 PASS 1541.565588
3-Host_RHEL.m9.u6.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.Win11.x86_64.io-github-autotest-qemu.multi_nics_verify.nic_virtio.q35 Finshed 2024-10-11 14:31:42 2024-10-11 14:39:08 PASS 446.045423
4-Host_RHEL.m9.u6.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.Win11.x86_64.io-github-autotest-qemu.multi_nics_verify.with_multiqueue.nic_virtio.q35 Finshed 2024-10-11 14:39:09 2024-10-11 15:01:53 PASS 1364.308224
5-Host_RHEL.m9.u6.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.Win2022.x86_64.io-github-autotest-qemu.multi_nics_verify.nic_virtio.q35 Finshed 2024-10-11 15:01:54 2024-10-11 15:09:09 PASS 435.263412
6-Host_RHEL.m9.u6.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.Win2022.x86_64.io-github-autotest-qemu.multi_nics_verify.with_multiqueue.nic_virtio.q35 Finshed 2024-10-11 15:09:10 2024-10-11 15:32:44 PASS 1413.966559
Summary:
Finshed=6, PASS=6

@heywji
Copy link
Contributor Author

heywji commented Oct 11, 2024

@leidwang Could you help review this patch? Many appreciations.

@heywji heywji force-pushed the fix_multi_nic2 branch 2 times, most recently from fe2fd53 to 1a59acf Compare October 23, 2024 08:30
Comment on lines 121 to 125
while True:
status, nics_num_checking_cmd_out = session.cmd_status_output(nics_num_checking_cmd, timeout=360)
if int(nics_num_checking_cmd_out) == nics_num:
break
time.sleep(30)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
while True:
status, nics_num_checking_cmd_out = session.cmd_status_output(nics_num_checking_cmd, timeout=360)
if int(nics_num_checking_cmd_out) == nics_num:
break
time.sleep(30)
utils_misc.wait_for(lambda: int(session.cmd__output(nics_num_checking_cmd, timeout=360)) == nics_num, ***, 30)

Copy link
Contributor

Choose a reason for hiding this comment

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

We need to setup an end condition for the while block, otherwise it might get stuck in an endless loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I will test its result ASAP.

@heywji heywji force-pushed the fix_multi_nic2 branch 5 times, most recently from 7c1cea4 to 8f7b411 Compare October 23, 2024 12:08
The test failed because Windows didn't assign IPs to all NICs
immediately after boot. A loop was added to check the NIC count until
all are initialized, ensuring the test waits for all NICs to get their
IP addresses before proceeding.

Signed-off-by: wji <[email protected]>
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.

2 participants