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

add case for period config of memballoon #5206

Merged

Conversation

nanli1
Copy link
Contributor

@nanli1 nanli1 commented Sep 26, 2023

VIRT-299028: Period config of memory balloon

Signed-off-by: nanli [email protected]

avocado-framework/avocado-vt#3768

RHE9 & RHEL8+x86

 avocado run --vt-type libvirt --test-runner=runner --vt-machine-type q35  memory.balloon.period

 (1/6) type_specific.io-github-autotest-libvirt.memory.balloon.period.memory_allocation.undefined_period.virtio_model: PASS (26.70 s)
 (2/6) type_specific.io-github-autotest-libvirt.memory.balloon.period.memory_allocation.undefined_period.virtio_trans_model: PASS (32.40 s)
 (3/6) type_specific.io-github-autotest-libvirt.memory.balloon.period.memory_allocation.undefined_period.virtio_non_trans_model: PASS (32.96 s)
 (4/6) type_specific.io-github-autotest-libvirt.memory.balloon.period.memory_allocation.int_period.virtio_model: PASS (32.76 s)
 (5/6) type_specific.io-github-autotest-libvirt.memory.balloon.period.memory_allocation.int_period.virtio_trans_model: PASS (32.96 s)
 (6/6) type_specific.io-github-autotest-libvirt.memory.balloon.period.memory_allocation.int_period.virtio_non_trans_model: PASS (32.71 s)

RHE9+Aarch64

avocado run --vt-type libvirt --test-runner=runner --vt-machine-type arm64-mmio  memory.balloon.period 

(1/6) type_specific.io-github-autotest-libvirt.memory.balloon.period.memory_allocation.undefined_period.virtio_model: PASS (60.59 s)
(2/6) type_specific.io-github-autotest-libvirt.memory.balloon.period.memory_allocation.undefined_period.virtio_trans_model: PASS (66.13 s)
(3/6) type_specific.io-github-autotest-libvirt.memory.balloon.period.memory_allocation.undefined_period.virtio_non_trans_model: PASS (63.73 s)
(4/6) type_specific.io-github-autotest-libvirt.memory.balloon.period.memory_allocation.int_period.virtio_model: PASS (64.28 s)
(5/6) type_specific.io-github-autotest-libvirt.memory.balloon.period.memory_allocation.int_period.virtio_trans_model: PASS (64.29 s)
(6/6) type_specific.io-github-autotest-libvirt.memory.balloon.period.memory_allocation.int_period.virtio_non_trans_model: PASS (60.25 s)

@nanli1 nanli1 force-pushed the add_case_for_period_config_of_mem_balloon branch 4 times, most recently from 07b51e8 to 27b0b7c Compare September 26, 2023 13:42
@nanli1 nanli1 marked this pull request as draft November 6, 2023 00:15
@nanli1 nanli1 marked this pull request as ready for review November 6, 2023 09:45
time.sleep(5)
else:
usable_mem = re.findall(r'usable (\d+)', res)[0]
last_update = re.findall(r'last_update (\d+)', res)[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you consider wait_for()? Something like:

def func():
    res = virsh.dommemstat(vm_name, ignore_status=False,
                            debug=True).stdout_text
    return re.findall(r'usable (\d+)', res) is not None

if not utils_misc.wait_for(func, 25, 5):
    return 
usable_mem = re.findall(r'usable (\d+)', res)[0]
last_update = re.findall(r'last_update (\d+)', res)[0]
disk_caches = re.findall(r'disk_caches (\d+)', res)[0]

return xxxx

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, what happened when we couldn't get the value we expect after 25s?

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 haijiao ,Sorry for late updating ,Updated as you said.

check_same_value(last_update_new, last_update, "last update")
check_same_value(disk_caches_new, disk_caches, "disk caches")

elif period_config.isdigit() and period_config.isdigit() != "0":
Copy link
Contributor

Choose a reason for hiding this comment

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

period_config != "0" ?

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 haijiao , Updated

Comment on lines 100 to 106
vmxml.del_device('memballoon', by_tag=True)
mem_balloon = memballoon.Memballoon()
mem_balloon.setup_attrs(**device_dict)
vmxml.devices = vmxml.devices.append(mem_balloon)

vmxml.setup_attrs(**mem_attrs)
vmxml.sync()
Copy link
Contributor

Choose a reason for hiding this comment

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

How about after removing memballoon device, we use libvirt_vmxml.modify_vm_device() to add new memballoon device, and maybe move vmxml.setup_attrs(**mem_attrs) to the front to avoid sync failure

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 haijiao , Updated :D

@nanli1 nanli1 marked this pull request as draft November 17, 2023 02:00
@nanli1 nanli1 marked this pull request as ready for review November 19, 2023 01:21
@nanli1 nanli1 force-pushed the add_case_for_period_config_of_mem_balloon branch from 27b0b7c to 57d60fb Compare November 20, 2023 04:39
@nanli1 nanli1 requested a review from chloerh November 20, 2023 04:43
@nanli1 nanli1 force-pushed the add_case_for_period_config_of_mem_balloon branch from 57d60fb to 8336071 Compare November 20, 2023 04:43
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.

Others LGTM

test.log.info("TEST_STEP5: Do memory operation and check dommemstat")
session = vm.wait_for_login()
status, stdout = session.cmd_status_output(mem_operation)
session.close()
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to repeat vm.wait_for_login() and session.close() ? Could we reuse a vm session in line 104?

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 dzheng, Done

usable_mem_3, last_update_3, disk_caches_3 = get_memory_statistics()
session = vm.wait_for_login()
status, stdout = session.cmd_status_output(mem_operation)
session.close()
Copy link
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
Contributor Author

Choose a reason for hiding this comment

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

Thanks dzheng, Done

Define and start guest
Check memTotal and memory allocation
"""
test.log.info("TEST_STEP1: Define guest")
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not accuate.

test.log.info("TEST_STEP6: Change the period setting")
virsh.dommemstat(vm_name, period_cmd, ignore_status=False, debug=True)

test.log.info("TEST_STEP7: Repeat consume mem and check memory "
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this log correct?

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 dzheng, Done

@nanli1 nanli1 force-pushed the add_case_for_period_config_of_mem_balloon branch from 8336071 to 11231e6 Compare November 30, 2023 10:22
@nanli1 nanli1 requested a review from dzhengfy November 30, 2023 10:24
usable_mem_1, last_update_1, disk_caches_1 = get_memory_statistics()

test.log.info("TEST_STEP5: Do memory operation and check dommemstat")
session = vm.wait_for_login()
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we reuse the session in line 104?

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 dzheng, updated, :D

test.log.info("TEST_STEP6: Change the period setting")
virsh.dommemstat(vm_name, period_cmd, ignore_status=False, debug=True)

test.log.info("TEST_STEP7: Repeat TEST_STEP3-5")
Copy link
Contributor

Choose a reason for hiding this comment

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

For below repeating steps 3-5, please wrap up the steps into another function instead of duplicating many codes.

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 dzheng, updated, :D

    VIRT-299028: Period config of memory balloon
Signed-off-by: nanli <[email protected]>
@nanli1 nanli1 force-pushed the add_case_for_period_config_of_mem_balloon branch from 11231e6 to f10246a Compare December 5, 2023 05:34
@nanli1 nanli1 requested a review from dzhengfy December 5, 2023 05:35
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

@dzhengfy dzhengfy merged commit 02e1499 into autotest:master Dec 13, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants