Memory : Fix incorrect block size on arm #5227

Aakanksha0311 · 2023-10-18T07:50:10Z

Fix memory device attachment and update errors.
The block size was too small, requiring a minimum of 524288KiB.
This commit corrects the issue, allowing successful memory device
attachment and updates.

Tested with libvirt version 9.5.0

Results for x86_64
avocado run --vt-type libvirt --test-runner=runner --vt-machine-type q35 memory_update_device.virtio_mem --vt-connect-uri qemu:///system
JOB ID : 270d985bae570287fd62983eeb9c661b13e36868
JOB LOG : /var/lib/avocado/job-results/job-2023-10-23T03.12-270d985/job.log
(1/1) type_specific.io-github-autotest-libvirt.memory_update_device.virtio_mem: PASS (89.62 s)
RESULTS : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB HTML : /var/lib/avocado/job-results/job-2023-10-23T03.12-270d985/results.html
JOB TIME : 91.62 s

avocado run --vt-type libvirt --test-runner=runner --vt-machine-type q35 memory_attach_device.virtio_mem.cold_plug --vt-connect-uri qemu:///system
JOB ID : 1b475552a9fee4f0efbc0acab56d2fe4559e591b
JOB LOG : /var/lib/avocado/job-results/job-2023-10-23T03.18-1b47555/job.log
(1/1) type_specific.io-github-autotest-libvirt.memory_attach_device.virtio_mem.cold_plug: PASS (87.82 s)
RESULTS : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB HTML : /var/lib/avocado/job-results/job-2023-10-23T03.18-1b47555/results.html
JOB TIME : 89.61 s

avocado run --vt-type libvirt --test-runner=runner --vt-machine-type q35 memory_attach_device.virtio_mem.hot_plug --vt-connect-uri qemu:///system
JOB ID : 263d277e5fda66ccae8324a75e4eaf8eedb9e542
JOB LOG : /var/lib/avocado/job-results/job-2023-10-23T03.21-263d277/job.log
(1/1) type_specific.io-github-autotest-libvirt.memory_attach_device.virtio_mem.hot_plug: PASS (83.21 s)
RESULTS : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB HTML : /var/lib/avocado/job-results/job-2023-10-23T03.21-263d277/results.html
JOB TIME : 84.92 s

Results for aarch64:
avocado run --vt-type libvirt --test-runner=runner --vt-machine-type arm64-mmio memory_update_device.virtio_mem --vt-connect-uri qemu:///system
Before fix:
(1/1) type_specific.io-github-autotest-libvirt.memory_update_device.virtio_mem: ERROR: Command '/bin/virsh attach-device avocado-vt-vm1 /tmp/xml_utils_temp_qwvfgzbw.xml --config' failed.\nstdout: b'\n'\nstderr: b'error: Failed to attach device from /tmp/xml_utils_temp_qwvfgzbw.xml\nerror: block size too small, must be at least 524288KiB\n'\nad... (49.79 s)
After fix:
(1/1) type_specific.io-github-autotest-libvirt.memory_update_device.virtio_mem: PASS (87.76 s)

avocado run --vt-type libvirt --test-runner=runner --vt-machine-type arm64-mmio memory_attach_device.virtio_mem.cold_plug --vt-connect-uri qemu:///system
Before fix:
(1/1) type_specific.io-github-autotest-libvirt.memory_attach_device.virtio_mem.cold_plug: ERROR: Command '/usr/bin/virsh attach-device avocado-vt-vm1 /tmp/xml_utils_temp_6p00k16z.xml --config' failed.\nstdout: b'\n'\nstderr: b'error: Failed to attach device from /tmp/xml_utils_temp_6p00k16z.xml\nerror: block size too small, must be at least 524288KiB\n... (64.57 s)
After fix:
(1/1) type_specific.io-github-autotest-libvirt.memory_attach_device.virtio_mem.cold_plug: PASS (97.48 s)

avocado run --vt-type libvirt --test-runner=runner --vt-machine-type arm64-mmio memory_attach_device.virtio_mem.hot_plug --vt-connect-uri qemu:///system
Before fix:
(1/1) type_specific.io-github-autotest-libvirt.memory_attach_device.virtio_mem.hot_plug: ERROR: Command '/bin/virsh attach-device avocado-vt-vm1 /tmp/xml_utils_temp_09c3brgk.xml ' failed.\nstdout: b'\n'\nstderr: b'error: Failed to attach device from /tmp/xml_utils_temp_09c3brgk.xml\nerror: block size too small, must be at least 524288KiB\n'\nadditional... (112.82 s)
After fix:
(1/1) type_specific.io-github-autotest-libvirt.memory_attach_device.virtio_mem.hot_plug: PASS (95.58 s)

Yingshun · 2023-10-19T02:12:21Z

plz provide test results on arm and x86_64, thanks!

Yingshun · 2023-10-23T02:10:43Z

libvirt/tests/src/memory/memory_attach_device.py

+        if params.get("start_vm") == "yes":
+            time.sleep(30)


May I ask why we need to sleep 30s for a running vm here?

This portion of the code was actually not required. Sorry for the confusion.

Yingshun · 2023-10-23T02:14:23Z

libvirt/tests/src/memory/memory_attach_device.py


        test.log.info("TEST_STEP1: Attach a virtio-mem device.")
        options = '' if vm.is_alive() else '--config'
        virsh.attach_device(vm_name, mem_device.xml, flagstr=options,
                            debug=True, ignore_status=False)
+        time.sleep(30)


Do all test scenarios need to wait 30s?

No, Just did some checks in depth. It is only required for hot_plug test case and that too minimum sleep time required is 10s. So updated this one as well

Yingshun · 2023-10-23T02:15:27Z

libvirt/tests/src/memory/memory_update_device.py

+        if params.get("start_vm") == "yes":
+            time.sleep(30)


Can you plz explain why we need to wait 30s here?

This portion of the code was actually not required. Sorry for the confusion.

Yingshun · 2023-10-23T02:18:00Z

libvirt/tests/cfg/memory/memory_attach_device.cfg

-            mem_device_attrs = {'mem_model': 'virtio-mem', 'target': {'requested_unit': 'KiB', 'size': 262144, 'node': 0, 'size_unit': 'KiB', 'requested_size': 131072, 'block_unit': 'KiB', 'block_size': 2048}, 'source': {'pagesize_unit': 'KiB', 'pagesize': 2048, 'nodemask': '0'}}
+            mem_device_attrs = {'mem_model': 'virtio-mem', 'target': {'requested_unit': 'KiB', 'size': 1048576, 'node': 0, 'size_unit': 'KiB', 'requested_size': 524288, 'block_unit': 'KiB', 'block_size': 52488}}


I wonder if 52488 works on x86_64 arch, plz upload test results on both x86_64 and aarch.
If it does not work, plz keep the original mem_device_attrs values for x84_64 arch, thx!

Actually the value should be 524288 there was a typo and it is fixed now. Also uploaded the test results for both arm and x86_64 in the comment. Added the original value of mem_device_attrs for x86_64.

Yingshun · 2023-10-23T02:18:12Z

libvirt/tests/cfg/memory/memory_update_device.cfg

-            mem_device_attrs = {'mem_model': 'virtio-mem', 'target': {'requested_unit': 'KiB', 'size': 262144, 'node': 0, 'size_unit': 'KiB', 'requested_size': 131072, 'block_unit': 'KiB', 'block_size': 2048}, 'source': {'pagesize_unit': 'KiB', 'pagesize': 2048, 'nodemask': '0'}}
-            requested_size = 160MiB
+            mem_device_attrs = {'mem_model': 'virtio-mem', 'target': {'requested_unit': 'KiB', 'size': 1048576, 'node': 0, 'size_unit': 'KiB', 'requested_size': 524288, 'block_unit': 'KiB', 'block_size': 524288}}
+            requested_size = 1048576KiB


Same as above.

I checked on the x86_64 machine and here this doesn't require the original values. On multiple checking the tests passed on both the system without any modification.

Aakanksha0311 · 2023-10-23T19:41:51Z

libvirt/tests/src/memory/memory_update_device.py

Lines 88-89 and 114-115 were changed when used the inspekt tool for checking style and indent

Yingshun · 2023-10-26T00:07:12Z

Please wrap the body of your commit at 72 characters. Others LGTM.

Yingshun

LGTM

dzhengfy · 2023-10-30T07:39:18Z

@Aakanksha0311 Please pay attention to the sensitive information, like user name, machine name and so on. We need to avoid above information included in the upstream PR description and comments. Thanks.

dzhengfy · 2023-10-30T07:42:51Z

@Aakanksha0311 For your testing result, we'd better keep them simple. And please also include your libvirt upstream version,

For example,

Tested with libvirt 9.5.0

Results for aarch64:
# avocado run --vt-type libvirt --test-runner=runner --vt-machine-type arm64-mmio memory_attach_device.virtio_mem.hot_plug --vt-connect-uri
(1/1) type_specific.io-github-autotest-libvirt.memory_attach_device.virtio_mem.hot_plug: PASS (76.54 s)

Results for x86:
(1/1) type_specific.io-github-autotest-libvirt.memory_attach_device.virtio_mem.hot_plug: PASS (76.54 s)

dzhengfy · 2023-10-30T07:50:26Z

@Aakanksha0311 For a pr to fix a failure, usually our pr title is recommended like this:
<python_file_name>: fix xxx issue/failure
For example,
memory_attach_device: fix a memory attach error or memory_attach_device: fix <detailed error message>

And please also include the failure message before your fix. Usually we write like below:

Before the fix:
<what is the detailed error message for which test case>
After the fix:
(1/1) type_specific.io-github-autotest-libvirt.memory_attach_device.virtio_mem.hot_plug: PASS (76.54 s)

dzhengfy

Others LGTM

dzhengfy · 2023-10-31T03:24:31Z

libvirt/tests/src/memory/memory_attach_device.py

@@ -7,6 +7,7 @@
 from virttest.staging import utils_memory
 from virttest.utils_libvirt import libvirt_vmxml
 from virttest.utils_version import VersionInterval
+import time

 VIRSH_ARGS = {'debug': True, 'ignore_status': False}
 ORG_HP = utils_memory.get_num_huge_pages()


If you are sure this case does not need hugepage setting, please also remove this line because there is no referrence in the codes.

Yes, removed.

dzhengfy · 2023-10-31T03:33:16Z

libvirt/tests/src/memory/memory_update_device.py

@@ -59,10 +59,6 @@ def setup_test_virtio_mem():
        """
        Setup vmxml for test
        """
-        set_num_huge_pages = params.get("set_num_huge_pages")


Please also remove line 13 ORG_HP = utils_memory.get_num_huge_pages() because there is not referrence.

Yes, done removed the line.

Aakanksha0311 · 2023-10-31T04:25:37Z

@Aakanksha0311 For a pr to fix a failure, usually our pr title is recommended like this: <python_file_name>: fix xxx issue/failure For example, memory_attach_device: fix a memory attach error or memory_attach_device: fix <detailed error message>

And please also include the failure message before your fix. Usually we write like below:

Before the fix: <what is the detailed error message for which test case> After the fix: (1/1) type_specific.io-github-autotest-libvirt.memory_attach_device.virtio_mem.hot_plug: PASS (76.54 s)

Ok, understood. Will update according to this. Thanks.

Aakanksha0311 · 2023-10-31T04:26:34Z

@Aakanksha0311 For your testing result, we'd better keep them simple. And please also include your libvirt upstream version,

For example,

Tested with libvirt 9.5.0

Results for aarch64:
# avocado run --vt-type libvirt --test-runner=runner --vt-machine-type arm64-mmio memory_attach_device.virtio_mem.hot_plug --vt-connect-uri
(1/1) type_specific.io-github-autotest-libvirt.memory_attach_device.virtio_mem.hot_plug: PASS (76.54 s)

Results for x86:
(1/1) type_specific.io-github-autotest-libvirt.memory_attach_device.virtio_mem.hot_plug: PASS (76.54 s)

Ok, got it.

Aakanksha0311 · 2023-10-31T04:28:10Z

@Aakanksha0311 Please pay attention to the sensitive information, like user name, machine name and so on. We need to avoid above information included in the upstream PR description and comments. Thanks.

Oh okay thanks will be careful and update here as well. Thanks.

Fix memory device attachment and update errors. The block size was too small, requiring a minimum of 524288KiB.This commit corrects the issue, allowing successful memory device attachment and updates. Signed-off-by: Aakanksha Tripathi <[email protected]>

dzhengfy · 2023-11-01T09:40:34Z

Sorry, I forgot to submit my approval.

dzhengfy

Approved

Aakanksha0311 force-pushed the NewPR-14749 branch from b18852a to bf45690 Compare October 18, 2023 12:21

Yingshun reviewed Oct 23, 2023

View reviewed changes

Aakanksha0311 force-pushed the NewPR-14749 branch 2 times, most recently from 72ca7bf to 432066b Compare October 23, 2023 19:13

Aakanksha0311 commented Oct 23, 2023

View reviewed changes

Aakanksha0311 force-pushed the NewPR-14749 branch 2 times, most recently from beba576 to c9d41cb Compare October 27, 2023 04:46

Yingshun approved these changes Oct 30, 2023

View reviewed changes

Yingshun requested a review from dzhengfy October 30, 2023 02:21

dzhengfy reviewed Oct 31, 2023

View reviewed changes

Aakanksha0311 force-pushed the NewPR-14749 branch 4 times, most recently from 27fddb4 to 1738e65 Compare November 1, 2023 06:28

Aakanksha0311 requested a review from Yingshun November 1, 2023 06:29

Aakanksha0311 force-pushed the NewPR-14749 branch from 1738e65 to 5719594 Compare November 1, 2023 06:37

Aakanksha0311 requested a review from dzhengfy November 1, 2023 06:40

Yingshun approved these changes Nov 1, 2023

View reviewed changes

Aakanksha0311 changed the title ~~Resolve memory attachment & update issues~~ Memory : Fix incorrect block size on arm Nov 1, 2023

Aakanksha0311 force-pushed the NewPR-14749 branch from 5719594 to 869c29e Compare November 1, 2023 07:03

dzhengfy merged commit 4a7f65f into autotest:master Nov 1, 2023
5 checks passed

dzhengfy reviewed Nov 1, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Memory : Fix incorrect block size on arm #5227

Memory : Fix incorrect block size on arm #5227

Aakanksha0311 commented Oct 18, 2023 •

edited

Loading

Yingshun commented Oct 19, 2023

Yingshun Oct 23, 2023

Aakanksha0311 Oct 23, 2023

Yingshun Oct 23, 2023

Aakanksha0311 Oct 23, 2023

Yingshun Oct 23, 2023

Aakanksha0311 Oct 23, 2023

Yingshun Oct 23, 2023

Aakanksha0311 Oct 23, 2023

Yingshun Oct 23, 2023

Aakanksha0311 Oct 23, 2023

Aakanksha0311 Oct 23, 2023

Yingshun commented Oct 26, 2023

Yingshun left a comment

dzhengfy commented Oct 30, 2023

dzhengfy commented Oct 30, 2023

dzhengfy commented Oct 30, 2023

dzhengfy left a comment

dzhengfy Oct 31, 2023

Aakanksha0311 Nov 1, 2023

dzhengfy Oct 31, 2023

Aakanksha0311 Nov 1, 2023

Aakanksha0311 commented Oct 31, 2023 •

edited

Loading

Aakanksha0311 commented Oct 31, 2023

Aakanksha0311 commented Oct 31, 2023

dzhengfy commented Nov 1, 2023

dzhengfy left a comment

		mem_device_attrs = {'mem_model': 'virtio-mem', 'target': {'requested_unit': 'KiB', 'size': 262144, 'node': 0, 'size_unit': 'KiB', 'requested_size': 131072, 'block_unit': 'KiB', 'block_size': 2048}, 'source': {'pagesize_unit': 'KiB', 'pagesize': 2048, 'nodemask': '0'}}
		mem_device_attrs = {'mem_model': 'virtio-mem', 'target': {'requested_unit': 'KiB', 'size': 1048576, 'node': 0, 'size_unit': 'KiB', 'requested_size': 524288, 'block_unit': 'KiB', 'block_size': 52488}}

Memory : Fix incorrect block size on arm #5227

Memory : Fix incorrect block size on arm #5227

Conversation

Aakanksha0311 commented Oct 18, 2023 • edited Loading

Yingshun commented Oct 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yingshun commented Oct 26, 2023

Yingshun left a comment

Choose a reason for hiding this comment

dzhengfy commented Oct 30, 2023

dzhengfy commented Oct 30, 2023

dzhengfy commented Oct 30, 2023

dzhengfy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aakanksha0311 commented Oct 31, 2023 • edited Loading

Aakanksha0311 commented Oct 31, 2023

Aakanksha0311 commented Oct 31, 2023

dzhengfy commented Nov 1, 2023

dzhengfy left a comment

Choose a reason for hiding this comment

Aakanksha0311 commented Oct 18, 2023 •

edited

Loading

Aakanksha0311 commented Oct 31, 2023 •

edited

Loading