Skip to content

automate VIRT-303225 - [vIOMMU] virsh reset when booting guest for 10+ times#6249

Merged
dzhengfy merged 1 commit intoautotest:masterfrom
hholoubk:XXX-303225-virsh-reset-10-times-2
Jun 10, 2025
Merged

automate VIRT-303225 - [vIOMMU] virsh reset when booting guest for 10+ times#6249
dzhengfy merged 1 commit intoautotest:masterfrom
hholoubk:XXX-303225-virsh-reset-10-times-2

Conversation

@hholoubk
Copy link
Copy Markdown
Contributor

@hholoubk hholoubk commented Mar 25, 2025

!!! the test is using new functions added to the avocado-vt framework in
avocado-framework/avocado-vt#4095

the test is about running starting VM according matrix setting,

  1. start VM and wait till booted, then reboot the guest ad
  2. do guest reset during guest rebooting required times (with some randomized sleep time to reset in different part of reboot)
  3. log into the VM
  4. check the /var/log/libvirt/qemu/vm.log for virtio: zero sized buffers are not allowed

the test can be parametrized:

  • max_wait_ms = 3000 # max number of miliseconds to wait between resets to provide some randomness in execution, the number should be less then expected time needed for reboot
  • max_repeat = 10 # number of repetition
  • log_messages # single string or list of strings to be checked in the log
  • fail_if_found = True # parameter to switch if the message should be there or not

Note:
The test is based (copied - yes I know this sucks) from VIRT-294579 (with some parts removed due smaller matrix) and to be honest I don't know if the rows 26-40 are written the best way as I am not the author of it.

Comment thread libvirt/tests/src/sriov/vIOMMU/iommu_repeated_reset.py
Comment thread libvirt/tests/src/sriov/vIOMMU/iommu_repeated_reset.py
Comment thread provider/vm_log_file/vm_log_file.py Outdated
@@ -0,0 +1,94 @@
import os
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I used libvirt.check_logfile() to check is some strings exist in the log file. If you want to create a class for it, I think it can be used for some other tests, so it should be added in avocado-vt.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@hholoubk hholoubk force-pushed the XXX-303225-virsh-reset-10-times-2 branch 2 times, most recently from 7f3aaf1 to f09ec7e Compare March 27, 2025 15:04
@Yingshun Yingshun added the depend on The PR has dependency on other PRs label Mar 31, 2025

from provider.viommu import viommu_base

#from provider.vm_log_file import vm_log_file
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please delete the commented line here, thx!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

DOne

Comment thread libvirt/tests/src/sriov/vIOMMU/iommu_repeated_reset.py
test.log.debug(vm_xml.VMXML.new_from_dumpxml(vm.name))

test.log.debug("TEST_STEP: verify user is able to login also after restarts")
vm.wait_for_login()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same as above.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@hholoubk hholoubk force-pushed the XXX-303225-virsh-reset-10-times-2 branch from 2a58ad4 to d9b7144 Compare April 16, 2025 13:33
@hholoubk
Copy link
Copy Markdown
Contributor Author

retested
(1/2) type_specific.io-github-autotest-libvirt.vIOMMU.iommu_repeated_reset.scsi_controller.virtio: STARTED
(1/2) type_specific.io-github-autotest-libvirt.vIOMMU.iommu_repeated_reset.scsi_controller.virtio: PASS (126.52 s)
(2/2) type_specific.io-github-autotest-libvirt.vIOMMU.iommu_repeated_reset.scsi_controller.smmuv3: STARTED
(2/2) type_specific.io-github-autotest-libvirt.vIOMMU.iommu_repeated_reset.scsi_controller.smmuv3: FAIL: The string 'virtio: zero sized buffers are not allowed' is included in /var/log/libvirt/qemu/avocado-vt-vm1.log (152.60 s)

... test properly failing for smmuv3 as the log contains the message

Comment thread libvirt/tests/cfg/sriov/vIOMMU/iommu_repeated_reset.cfg Outdated
dev_dict = test_obj.update_disk_addr(dev_dict)
if dev_dict["target"].get("bus") != "virtio":
libvirt_vmxml.modify_vm_device(
vm_xml.VMXML.new_from_dumpxml(vm.name), 'disk', {'driver': None})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I do not understand what we need to set driver to None when bus is not virtio?
And then call the function again on Line 33.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The default guest's xml may contain viommu enabled virtio disk. If we modify it to non-virtio disk in the test, it may report an error because of the extra driver attrs. In order to cover test env with/without viommu virtio disk, we'd better to cleanup the driver setting before the test.

Comment thread libvirt/tests/src/sriov/vIOMMU/iommu_repeated_reset.py Outdated
Comment thread libvirt/tests/src/sriov/vIOMMU/iommu_repeated_reset.py Outdated
Comment thread libvirt/tests/cfg/sriov/vIOMMU/iommu_repeated_reset.cfg Outdated
Comment thread libvirt/tests/cfg/sriov/vIOMMU/iommu_repeated_reset.cfg Outdated
dev_dict = eval(params.get('disk_dict', '{}'))
test.log.debug(f"disk_dict: {dev_dict}")
if dev_dict:
dev_dict = test_obj.update_disk_addr(dev_dict)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We do not need to update the disk address, instead, we just remove the disk address, then set disk bus to scsi. That will be easier.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is the only thing I don't know how to do ... Why is the disk address update worse?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

test_obj.update_disk_addr is a wrapper function to update the disk attrs according to the test cfg and the prepared controller. In this case, it's the easiest way.

libvirt_vmxml.modify_vm_device(
vm_xml.VMXML.new_from_dumpxml(vm.name), 'disk', dev_dict)

if cleanup_ifaces:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I do not see any interface setting in the manual case. What is the reason we modify interface setting here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This setting was in previous test. I expected that there can be case, that it is needed, so I let there this settings

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have removed this, there just has to be the
test_obj.setup_iommu_test(iommu_dict=iommu_dict, cleanup_ifaces=False), (or True, doesn't matter)
if the value is not passed will fail with No DHCP ipv.

Copy link
Copy Markdown
Contributor

@Yingshun Yingshun left a comment

Choose a reason for hiding this comment

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

Commit a looks like a rebase? can you squash the commits? Others LGTM.

@hholoubk hholoubk force-pushed the XXX-303225-virsh-reset-10-times-2 branch from 6f71d05 to 4045092 Compare June 5, 2025 09:42
@hholoubk hholoubk requested review from Yingshun and dzhengfy June 6, 2025 12:34
@dzhengfy dzhengfy merged commit 4a289b6 into autotest:master Jun 10, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

depend on The PR has dependency on other PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants