-
Notifications
You must be signed in to change notification settings - Fork 173
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
Virtiofs: Add test case support >=509 memslots #4204
base: master
Are you sure you want to change the base?
Conversation
Hi @hellohellenmao, please help review this patch, when you have time. Thanks! |
@XueqiangWei @qcheng-redhat I've seens no s390x here, not sure if this needs my review |
utils_disk.umount(fs_target, fs_dest, "virtiofs", session=session) | ||
utils_misc.safe_rmdir(fs_dest, session=session) | ||
if os_type == "windows": | ||
virtio_fs_utils.delete_viofs_serivce(test, params, session) |
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 stop virtiofs service first and then delete it in case bringing the unexpected error?
And suggest to add win_driver_utils.memory_leak_check(vm, test, params) function in the end.
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.
Thanks for review. Updated.
fs_driver_props = {"queue-size": 1024} | ||
fs_source_type = mount | ||
fs_source_dir = /var/tmp/virtio_fs_test | ||
force_create_fs_source = no |
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.
All the fs paths are created by user, how about setting fs_source_user_config = yes, then it could save time in env_process.py
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.
A test file is created before test execution. So don't need create fs source again here.
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.
Make sense.
Signed-off-by: qcheng-redhat <[email protected]>
2aae8df
to
cb2cb19
Compare
@fbq815 Please refer to the polarion test case for more information about this patch. Thanks. |
""" | ||
1) Boot a guest VM with virtio-fs device | ||
2) Check virtiofs basic functionality works | ||
3) Validate the virtio-mem device has the correct number of memslots |
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.
@hellohellenmao For the steps there is memslot check in polarion, I think it's better to have it here, what do you think ?
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.
Sorryt that I do not totally understand, from my understanding, it is checking the memslots here, what are the difference with yours?
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.
From my understanding, this case is adding virtio memory device to extend the memory, while virtiofs rely on the memory backend to work, so when the memory was plugged, I think it's necessary to checking the writing function.
fs_driver_props = {"queue-size": 1024} | ||
fs_source_type = mount | ||
fs_source_dir = /var/tmp/virtio_fs_test | ||
force_create_fs_source = no |
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.
Make sense.
Add test case support >=509 memslots
ID: 3024