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

memory/gmap: add test for shadow memory counters #5287

Merged
merged 1 commit into from
Mar 5, 2024

Conversation

smitterl
Copy link
Contributor

KVM records shadow memory events for a nested guest. Confirm the counters go up.

@smitterl
Copy link
Contributor Author

JOB ID     : e33efa43b7334838e06ee6c5638568d669825e1b
JOB LOG    : /var/lib/avocado/job-results/job-2023-11-20T08.41-e33efa4/job.log
 (1/1) type_specific.io-github-autotest-libvirt.gmap.l3_shadow_table_counters: PASS (25.72 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-11-20T08.41-e33efa4/results.html
JOB TIME   : 26.12 s

@smitterl smitterl force-pushed the gmap_counters branch 3 times, most recently from 0d0f655 to 06f9575 Compare November 21, 2023 12:18
@smitterl smitterl force-pushed the gmap_counters branch 2 times, most recently from 6e712ee to 67213cd Compare December 13, 2023 09:37
@smitterl
Copy link
Contributor Author

@fbq815 Please can you help review?

@fbq815
Copy link
Contributor

fbq815 commented Jan 3, 2024

I have a question: is l2_guest in this code still a level 1 guest? Others LGTM

@smitterl
Copy link
Contributor Author

I have a question: is l2_guest in this code still a level 1 guest? Others LGTM

It is a level 1 kvm guest but L2 guest here because an LPAR is L1 as it's also virtualized.

@fbq815
Copy link
Contributor

fbq815 commented Jan 11, 2024

I have a question: is l2_guest in this code still a level 1 guest? Others LGTM

It is a level 1 kvm guest but L2 guest here because an LPAR is L1 as it's also virtualized.

if I understand correctly, I prefer we assume the virtual machine as L1 in confgiure file case you also name a functiona as "l1_counters_go_up". There would be some mis-understanding for others? How about we add comment like "we assume the test scenario as virtual machine which is a L2 guest in LPAR" and we only have L1 and L2 as the name?

@smitterl
Copy link
Contributor Author

I have a question: is l2_guest in this code still a level 1 guest? Others LGTM

Depends on the language, a mainframe LPAR is a virtualization L1, therefore a KVM guest on the LPAR is an L2. You could count the first KVM guest as L1 but that's not how it's usually counted on IBM side IIUC.

@smitterl
Copy link
Contributor Author

I have a question: is l2_guest in this code still a level 1 guest? Others LGTM

It is a level 1 kvm guest but L2 guest here because an LPAR is L1 as it's also virtualized.

if I understand correctly, I prefer we assume the virtual machine as L1 in confgiure file case you also name a functiona as "l1_counters_go_up". There would be some mis-understanding for others? How about we add comment like "we assume the test scenario as virtual machine which is a L2 guest in LPAR" and we only have L1 and L2 as the name?

I get your point. I updated the run function's comment to clarify. Please check.

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

},
"target": {"dir": target_tag},
"binary": {"flock": "off", "lock_posix": "off"}
})
Copy link
Contributor

Choose a reason for hiding this comment

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

Just FYI.
libvirt_device_utils.create_fs_xml() is seldom used in current tp-libvirt. I did only find one occurrence.
Now the popular codes are like below. And the hardcoded 7 lines in python module are not recommended. I think it is better if we put the parameters and values into cfg.

fs_dict = {'accessmode':'passthrough',  'source':{'dir': '${source_dir}'}, "target": {'dir': 'mount_tag0'}, 'binary': {'lock_posix':'off','flock':'off'}}
dev_type = "filesystem"

fs_dict = eval(params.get('fs_dict', '{}'))
fs = libvirt_vmxml.create_vm_device_by_type(dev_type, fs_dict)
virsh.attach_device(vm_name, fs.xml, debug=True, ignore_status=False)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it. Thanks for the example. Updated.

test.fail("Gmap counters didn't go up.")
finally:
cleanup_actions.reverse()
for a in cleanup_actions:
Copy link
Contributor

Choose a reason for hiding this comment

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

This naming (a) seems informal. Could you give a meaningful name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here the scope is really small just this and the next line so I think the name is ok. However, I updated it to 'action'.

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 I addressed all your comments, please have a look, thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

JOB ID     : 064246135562db85db932c63662d75fb237c078a                                                                                                                                                               
JOB LOG    : /var/lib/avocado/job-results/job-2024-02-19T05.51-0642461/job.log                                                                                                                                      
 (1/1) type_specific.io-github-autotest-libvirt.gmap.l3_shadow_table_counters: PASS (27.17 s)                                                                                                                       
RESULTS    : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0                                                                                                                                   
JOB HTML   : /var/lib/avocado/job-results/job-2024-02-19T05.51-0642461/results.html                                                                                                                                 
JOB TIME   : 27.49 s    

KVM records shadow memory events for a nested guest.
Confirm the counters go up.

Signed-off-by: Sebastian Mitterle <smitterl@redhat.com>
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 a630c5c into autotest:master Mar 5, 2024
5 checks passed
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.

None yet

3 participants