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

win_virtio_driver_install_by_installer: support viomem test #4234

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

Conversation

menli820
Copy link
Contributor

@menli820 menli820 commented Dec 17, 2024

Add supoport to test viomem related from
the installer.
ID: 3171, 3191, 3192, 3193, 3194

@menli820 menli820 force-pushed the KVMAUTOMA-3171 branch 2 times, most recently from b11a025 to 731e176 Compare December 17, 2024 02:47
@menli820
Copy link
Contributor Author

@xiagao @heywji please help to review it.
@xiagao for the viofs change part, please have a check, the change just make sure we can start the viomem device normally, maybe other parameters should be deleted as well.

@menli820
Copy link
Contributor Author

menli820 commented Dec 24, 2024

@vivianQizhu A quick question is How do think we won't consider the rpm version for viomem, so the cfg will be more easier to organize for now and in the future if new feature coming ?

Currently we organize the viomem part in the cfg file considering the rpm version, the question is that as xiaogao comment "in the future when there are more drivers support, I think it's hard to extend the variants."

Both are okay for me, so just to confirm we all agree the same method before I take action to modify it again.

@heywji
Copy link
Contributor

heywji commented Dec 24, 2024

I confirmed with @menli820 about some failure test results offline discussion, and the root cause is a known product bug. @menli820 will post the new patch to avoid the effect of the product bug. And then, I will review it again.

I have my Kar cmdline here for you to refer to.

python3 ConfigTest.py --testcase=win_virtio_driver_install_by_installer..with_viomem,win_virtio_driver_installer_uninstall..with_viomem --guestname=Win11.x86_64  --machines=q35 --customsparams="cdrom_virtio = isos/windows/virtio-win-1.9.44-0.el9_5.iso" --firmware=ovmf --clone=yes  --driveformat=ahci

@menli820
Copy link
Contributor Author

@xiagao @heywji call for your review again when you free, thanks

Copy link
Contributor

@heywji heywji left a comment

Choose a reason for hiding this comment

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

Kar encountered a product bug unrelated to this patch(the installer code related has passed) and confirmed with @menli820, so LGTM.

# mainly to cover win10.i386 test without viomem
boot_with_viomem = no
Host_RHEL.m7, Host_RHEL.m8, Host_RHEL.m9.u0, Host_RHEL.m9.u1, Host_RHEL.m9.u2, Host_RHEL.m9.u3, Host_RHEL.m9.u4:
boot_with_viomem = no
Copy link
Contributor

Choose a reason for hiding this comment

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

It was defined in above.

Copy link
Contributor Author

@menli820 menli820 Jan 3, 2025

Choose a reason for hiding this comment

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

Actually in my understanding it is not a conflict,the first ‘boot_with_viomem = no’ is for i386.
the second if for both x86_64 and i386.

qemu/tests/win_virtio_driver_update_by_installer.py Outdated Show resolved Hide resolved
Add supoport to test viomem related from
the installer.

Signed-off-by: menli <[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.

4 participants