Skip to content

Fix logging on virtiofs shutdown #11359

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
Merged
merged 1 commit into from
Jun 16, 2025
Merged

Fix logging on virtiofs shutdown #11359

merged 1 commit into from
Jun 16, 2025

Conversation

pawelbeza
Copy link
Contributor

@pawelbeza pawelbeza commented Jun 3, 2025 •
edited
Loading

Fixes a confusing log message shown when Virtio-FS is disabled.
Currently, we log “The virtiofsd had stopped” regardless of whether Virtio-FS was actually enabled or not.

justxuewei
justxuewei reviewed Jun 3, 2025
View reviewed changes
@pawelbeza pawelbeza force-pushed the fix-logs-on-virtiofs-shutdown branch from e598eef to 2e7b608 Compare June 3, 2025 11:34
@justxuewei
Copy link
Member

justxuewei commented Jun 3, 2025 •
edited
Loading

Please add a subsystem name before your PR title and commit title, like runtime: xxxxx. FYI, please follow our guide regarding the patch format.

@pawelbeza pawelbeza force-pushed the fix-logs-on-virtiofs-shutdown branch from 2e7b608 to 255dd76 Compare June 3, 2025 12:51
@pawelbeza
Copy link
Contributor Author

Please add a subsystem name before your PR title and commit title, like runtime: xxxxx. FYI, please follow our guide regarding the patch format.

sure, should be good now

justxuewei
justxuewei reviewed Jun 3, 2025
View reviewed changes
@pawelbeza pawelbeza force-pushed the fix-logs-on-virtiofs-shutdown branch 2 times, most recently from aaace9c to 3398fdd Compare June 6, 2025 08:37
@pawelbeza
Copy link
Contributor Author

Hey @justxuewei could you have a look at it? 🙏

@justxuewei
Copy link
Member

Hi @pawelbeza, I am out of office until next week. Sorry:(

@pawelbeza
Copy link
Contributor Author

Hi @pawelbeza, I am out of office until next week. Sorry:(

ahh sure no worries, let's revisit it next week then

have a great holidays!:)

justxuewei
justxuewei reviewed Jun 8, 2025
View reviewed changes
@pawelbeza pawelbeza force-pushed the fix-logs-on-virtiofs-shutdown branch from 3398fdd to 04136f7 Compare June 13, 2025 13:17
@pawelbeza
Copy link
Contributor Author

@justxuewei sorry for late reply, agree with your comments, it should be good now:)

justxuewei
justxuewei approved these changes Jun 13, 2025
View reviewed changes
Copy link
Member

@justxuewei justxuewei left a comment

Choose a reason for hiding this comment

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

Lgtm, thanks!

@pawelbeza
runtime: Fix logging on virtiofs shutdown
91db412 
Fixes a confusing log message shown when Virtio-FS is disabled.
Previously we logged “The virtiofsd had stopped” regardless of whether Virtio-FS was actually enabled or not.

Signed-off-by: Paweł Bęza <pawel.beza99@gmail.com>
@pawelbeza pawelbeza force-pushed the fix-logs-on-virtiofs-shutdown branch from 04136f7 to 91db412 Compare June 13, 2025 14:00
@justxuewei justxuewei requested review from Apokleos, gkurz and pmores June 15, 2025 02:58
pmores
pmores approved these changes Jun 16, 2025
View reviewed changes
Copy link
Contributor

@pmores pmores left a comment

Choose a reason for hiding this comment

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

With the caveat that I don't actually work with runtime-go or clh, this looks good to me by inspection. Thanks @pawelbeza!

@justxuewei justxuewei merged commit 9b4518f into kata-containers:main Jun 16, 2025
502 of 529 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justxuewei justxuewei justxuewei approved these changes

@pmores pmores pmores approved these changes

@Apokleos Apokleos Awaiting requested review from Apokleos

@gkurz gkurz Awaiting requested review from gkurz

Assignees
No one assigned
Labels
ok-to-test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pawelbeza @justxuewei @pmores