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.

Sign up for GitHub

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

Jump to bottom

virtiofs : add case for restart service after config shared memory #5004

Merged
merged 1 commit into from
Jul 19, 2023
Merged

virtiofs : add case for restart service after config shared memory #5004

merged 1 commit into from
Jul 19, 2023

Conversation

nanli1
Copy link
Contributor

@nanli1 nanli1 commented Jul 3, 2023 

VIRT-298224: Verify the libvirt can identify shared memory after restart libvirtd/virtqemud


avocado run --vt-type libvirt --test-runner=runner --vt-machine-type q35 virtual_devices.filesystem_device.positive_test.file_backed.lifecycle.restart_service.fs_test.xattr_on.flock_off.lock_posix_off.cache_mode_none.thread_pool_noset.one_fs.one_guest

 (1/1) type_specific.io-github-autotest-libvirt.virtual_devices.filesystem_device.positive_test.file_backed.lifecycle.restart_service.fs_test.xattr_on.flock_off.lock_posix_off.cache_mode_none.thread_pool_noset.one_fs.one_guest: PASS (39.41 s)

@nanli1 nanli1 marked this pull request as draft July 3, 2023 07:50
@nanli1 nanli1 marked this pull request as ready for review July 4, 2023 02:20
chunfuwen
chunfuwen reviewed Jul 4, 2023
View reviewed changes
libvirt/tests/src/virtual_device/filesystem_device.py Outdated
@@ -245,6 +251,26 @@ def check_filesystem_in_guest(vm, fs_dev):
test.cancel("Bug %s is not fixed on current build" % bug_url)

try:
if setup_mem:
vmxml = vm_xml.VMXML.new_from_inactive_dumpxml(vm_names[int(guest_num)-1])
Copy link
Contributor

Choose a reason for hiding this comment

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

below line start 255 to line 273 can be wrapped into one independent method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks Chunfu, Done

@nanli1 nanli1 changed the title add case for restart service after config shared memory virtiofs : add case for restart service after config shared memory Jul 11, 2023
@nanli1 nanli1 force-pushed the add_case_for_restart_service_after_config_shared_memory branch from 36d29b7 to ea3dd10 Compare July 11, 2023 02:15
@nanli1 nanli1 requested a review from chunfuwen July 11, 2023 02:17
@nanli1 nanli1 force-pushed the add_case_for_restart_service_after_config_shared_memory branch from ea3dd10 to c32fa7d Compare July 11, 2023 02:18
dzhengfy
dzhengfy reviewed Jul 11, 2023
View reviewed changes
Copy link
Contributor

@dzhengfy dzhengfy left a comment

Choose a reason for hiding this comment

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

LGTM

iccaszhulili
iccaszhulili reviewed Jul 11, 2023
View reviewed changes
libvirt/tests/cfg/virtual_device/filesystem_device.cfg
@@ -86,6 +86,14 @@
bug_url = "https://bugzilla.redhat.com/show_bug.cgi?id=1940276"
destroy_start = "yes"
stress_script = "#!/usr/bin/python3;import os;while True:; os.open("%s/moo", os.O_CREAT | os.O_RDWR); os.unlink("%s/moo");"
- restart_service:
only file_backed
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, the manual case is indicated explicitly. The scenario can be expanded to all the memory backed type, not just the file type. We can remove this only condition. Thanks

Copy link
Contributor Author

@nanli1 nanli1 Jul 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

But we just write this code instead of modular cases ,Do you update this case later?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I made a mistake
"Sorry, the manual case is NOT indicated explicitly". I lost "NOT"

libvirt/tests/cfg/virtual_device/filesystem_device.cfg Outdated
@@ -86,6 +86,14 @@
bug_url = "https://bugzilla.redhat.com/show_bug.cgi?id=1940276"
destroy_start = "yes"
stress_script = "#!/usr/bin/python3;import os;while True:; os.open("%s/moo", os.O_CREAT | os.O_RDWR); os.unlink("%s/moo");"
- restart_service:
only file_backed
func_supported_since_libvirt_ver = (8, 0, 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

I check the scenario in bug 2078693 with libvirt-9.0.0-10.2.el9_2.x86_64, the issue still exists. So the fix for this bug was just backported to libvirt-8.0.0. For the version between libvirt-9.2.0 and libvirt-8.0.0, the issue still exists.
Hi, @dzhengfy How can deal with such situation in automation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dzhengfy For the version between libvirt-9.2.0 and libvirt-8.0.0, How about just leave it failed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Confirmed with dzheng and lizhu, We decided updated supported libvirt version is newer than 9.2.0

@nanli1
add case for restart service after config shared memory
9795e9a 
    VIRT-298224: Verify the libvirt can identify shared memory after restart libvirtd/virtqemud

Signed-off-by: nanli <[email protected]>
@nanli1 nanli1 force-pushed the add_case_for_restart_service_after_config_shared_memory branch from c32fa7d to 9795e9a Compare July 18, 2023 09:30
@nanli1 nanli1 requested a review from iccaszhulili July 18, 2023 09:32
chunfuwen
chunfuwen approved these changes Jul 19, 2023
View reviewed changes
Copy link
Contributor

@chunfuwen chunfuwen left a comment

Choose a reason for hiding this comment

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

lgtm

@iccaszhulili
Copy link
Contributor

LGTM

@Yingshun Yingshun merged commit a5ee21c into autotest:master Jul 19, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dzhengfy dzhengfy dzhengfy approved these changes

@iccaszhulili iccaszhulili iccaszhulili approved these changes

@chunfuwen chunfuwen chunfuwen approved these changes

@Yingshun Yingshun Yingshun approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@nanli1 @iccaszhulili @dzhengfy @chunfuwen @Yingshun