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

virtio_fs_share_data_hugepage: correct cache argument #4241

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zhaojunyao7
Copy link

change cache argument of virtiofsd from',cache=' to ' --cache=' and update the cache mode 'none' to 'never' to match with the latest production behavior.

ID:2917
Signed-off-by: Junyao Zhao [email protected]

@zhaojunyao7
Copy link
Author

@hellohellenmao , could you help review this one ? Thank you.

@zhaojunyao7
Copy link
Author

(2/4) Host_RHEL.m9.u6.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.x86_64.io-github-autotest-qemu.virtio_fs_share_data_hugepage.basic_test.with_cache.auto.q35: PASS (70.81 s)
(3/4) Host_RHEL.m9.u6.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.x86_64.io-github-autotest-qemu.virtio_fs_share_data_hugepage.basic_test.with_cache.always.q35: PASS (72.37 s)
(4/4) Host_RHEL.m9.u6.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.x86_64.io-github-autotest-qemu.virtio_fs_share_data_hugepage.basic_test.with_cache.none.q35: PASS (72.96 s)

@hellohellenmao
Copy link
Contributor

LGTM

@hellohellenmao
Copy link
Contributor

@YongxueHong Could you please help to take a review here? THanks

@YongxueHong
Copy link
Contributor

Hi @MiriamDeng @fbq815 @xiagao
Could you help review it to see if it affects your feature?
Thanks.

Comment on lines 71 to 79
- none:
fs_binary_extra_options += ",cache=none"
fs_binary_extra_options += " --cache=never"
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @zhaojunyao7
Do we consider updating the variant name to never?
I prefer to update it since it is more earlier to understand the case purpose.
Pls let me know your opinions. Thanks.

@zhaojunyao7 zhaojunyao7 force-pushed the KVMAUTOMA-2917 branch 2 times, most recently from 1a525be to d22f731 Compare January 9, 2025 02:09
@zhaojunyao7
Copy link
Author

@YongxueHong Thanks for you advice, updated the variant name to never.

@xiagao
Copy link
Contributor

xiagao commented Jan 9, 2025

It looks good to me. ACK.

YongxueHong
YongxueHong previously approved these changes Jan 13, 2025
Copy link
Contributor

@YongxueHong YongxueHong left a comment

Choose a reason for hiding this comment

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

LGTM.

@zhaojunyao7
Copy link
Author

@YongxueHong , could you merge this if no other comment?

@YongxueHong
Copy link
Contributor

Hi @MiriamDeng @fbq815 @xiagao Could you help review it to see if it affects your feature? Thanks.

Hi @zhaojunyao7
I think we need other feature owner's acking since this PR will also affect other platforms.
Hi @MiriamDeng @fbq815
Could you help to review it?
Thanks.

@MiriamDeng
Copy link
Contributor

@maxujun could you please help to check it on ppc side ?

@zhaojunyao7
Copy link
Author

@fbq815 @maxujun , could you help review this?

@maxujun
Copy link
Contributor

maxujun commented Feb 6, 2025

@fbq815 @maxujun , could you help review this?

For PPC RHEL8.10 qemu,it's not availible. The usage is "-o cache=<auto, always, none>".

change cache argument of virtiofsd from',cache=' to ' --cache='
and update the cache mode 'none' to 'never' to match with the
latest production behavior.

Signed-off-by: Junyao Zhao <[email protected]>
@zhaojunyao7
Copy link
Author

JOB LOG : /root/avocado/job-results/job-2025-02-06T21.42-865daab/job.log
(1/4) Host_RHEL.m9.u6.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.x86_64.io-github-autotest-qemu.unattended_install.cdrom.extra_cdrom_ks.default_install.aio_threads.q35: STARTED
(1/4) Host_RHEL.m9.u6.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.x86_64.io-github-autotest-qemu.unattended_install.cdrom.extra_cdrom_ks.default_install.aio_threads.q35: PASS (805.91 s)
(2/4) Host_RHEL.m9.u6.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.x86_64.io-github-autotest-qemu.virtio_fs_share_data_hugepage.basic_test.with_cache.auto.q35: STARTED
(2/4) Host_RHEL.m9.u6.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.x86_64.io-github-autotest-qemu.virtio_fs_share_data_hugepage.basic_test.with_cache.auto.q35: PASS (117.40 s)
(3/4) Host_RHEL.m9.u6.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.x86_64.io-github-autotest-qemu.virtio_fs_share_data_hugepage.basic_test.with_cache.always.q35: STARTED
(3/4) Host_RHEL.m9.u6.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.x86_64.io-github-autotest-qemu.virtio_fs_share_data_hugepage.basic_test.with_cache.always.q35: PASS (98.35 s)
(4/4) Host_RHEL.m9.u6.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.x86_64.io-github-autotest-qemu.virtio_fs_share_data_hugepage.basic_test.with_cache.never.q35: STARTED
(4/4) Host_RHEL.m9.u6.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.x86_64.io-github-autotest-qemu.virtio_fs_share_data_hugepage.basic_test.with_cache.never.q35: PASS (104.45 s)
RESULTS : PASS 4 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB HTML : /root/avocado/job-results/job-2025-02-06T21.42-865daab/results.html
JOB TIME : 1134.05 s

@zhaojunyao7
Copy link
Author

@maxujun , I have updated the code to support RHEL 8 and versions later than RHEL 8. Could you help confirm if this works for your case?

@maxujun
Copy link
Contributor

maxujun commented Feb 7, 2025

@fbq815 @maxujun , could you help review this?

For PPC RHEL8.10 qemu,it's not availible. The usage is "-o cache=<auto, always, none>".

@maxujun , I have updated the code to support RHEL 8 and versions later than RHEL 8. Could you help confirm if this works for your case?

(1/6) Host_RHEL.m8.u10.product_rhel.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.ppc64le.io-github-autotest-qemu.virtio_fs_share_data_hugepage.basic_test.with_cache.auto: STARTED
(1/6) Host_RHEL.m8.u10.product_rhel.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.ppc64le.io-github-autotest-qemu.virtio_fs_share_data_hugepage.basic_test.with_cache.auto: PASS (169.81 s)
(2/6) Host_RHEL.m8.u10.product_rhel.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.ppc64le.io-github-autotest-qemu.virtio_fs_share_data_hugepage.basic_test.with_cache.always: STARTED
(2/6) Host_RHEL.m8.u10.product_rhel.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.ppc64le.io-github-autotest-qemu.virtio_fs_share_data_hugepage.basic_test.with_cache.always: PASS (119.73 s)
(3/6) Host_RHEL.m8.u10.product_rhel.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.ppc64le.io-github-autotest-qemu.virtio_fs_share_data_hugepage.basic_test.with_cache.never: STARTED
(3/6) Host_RHEL.m8.u10.product_rhel.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.ppc64le.io-github-autotest-qemu.virtio_fs_share_data_hugepage.basic_test.with_cache.never: PASS (112.87 s)
(4/6) Host_RHEL.m8.u10.product_rhel.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.ppc64le.io-github-autotest-qemu.virtio_fs_share_data_hugepage.with_numa.basic_test.with_cache.auto: STARTED
(4/6) Host_RHEL.m8.u10.product_rhel.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.ppc64le.io-github-autotest-qemu.virtio_fs_share_data_hugepage.with_numa.basic_test.with_cache.auto: PASS (124.24 s)
(5/6) Host_RHEL.m8.u10.product_rhel.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.ppc64le.io-github-autotest-qemu.virtio_fs_share_data_hugepage.with_numa.basic_test.with_cache.always: STARTED
(5/6) Host_RHEL.m8.u10.product_rhel.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.ppc64le.io-github-autotest-qemu.virtio_fs_share_data_hugepage.with_numa.basic_test.with_cache.always: PASS (101.30 s)
(6/6) Host_RHEL.m8.u10.product_rhel.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.ppc64le.io-github-autotest-qemu.virtio_fs_share_data_hugepage.with_numa.basic_test.with_cache.never: STARTED
(6/6) Host_RHEL.m8.u10.product_rhel.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.6.0.ppc64le.io-github-autotest-qemu.virtio_fs_share_data_hugepage.with_numa.basic_test.with_cache.never: PASS (164.39 s)

LGTM.

@hellohellenmao
Copy link
Contributor

@YongxueHong Could you please help to take a review again? Please help to merge if it's okay for you.

Comment on lines +77 to +81
- never:
!Host_RHEL.m8:
fs_binary_extra_options += " --cache=never"
Host_RHEL.m8:
fs_binary_extra_options += " -o cache=none"
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @zhaojunyao7 @hellohellenmao @xiagao
Does it make sense that we set the fs_binary_extra_options += " -o cache=none" for RHEL 8 host mandatory?
And from the @maxujun 's comment, just there is no such usage -o cache=<auto, always, none> on PPC, so do we need to extend this limitation to all the platforms? or this usage does work with other platforms neither.
Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, for all the platform.

Comment on lines +68 to +71
!Host_RHEL.m8:
fs_binary_extra_options += " --cache=auto"
Host_RHEL.m8:
fs_binary_extra_options += " -o cache=auto"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
!Host_RHEL.m8:
fs_binary_extra_options += " --cache=auto"
Host_RHEL.m8:
fs_binary_extra_options += " -o cache=auto"
fs_binary_extra_options += " --cache=auto"
Host_RHEL.m8:
fs_binary_extra_options += " -o cache=auto"

Hi @xiagao @maxujun @fbq815
Could you help to review it? Thanks a lot

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.

6 participants