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

Handle long device names when creating QEMU device tags #13516

Merged

Conversation

hamistao
Copy link
Contributor

@hamistao hamistao commented May 29, 2024

Fixes #13495.
Helps to avoid problems with these tests.
The old blockNodeName function was modified, renamed and repurposed to also help generating device tags for QEMU.

@hamistao hamistao force-pushed the issue13495/handle_long_device_names branch 4 times, most recently from 31795e6 to 1513425 Compare June 2, 2024 17:13
@hamistao hamistao marked this pull request as ready for review June 2, 2024 17:52
@hamistao hamistao requested a review from tomponline as a code owner June 2, 2024 17:52
@hamistao hamistao changed the title Handle long device names Handle long device names when creating QEMU device tags Jun 2, 2024
@tomponline
Copy link
Member

Please can this be rebased too

@hamistao hamistao force-pushed the issue13495/handle_long_device_names branch 2 times, most recently from d0a325e to 17826c4 Compare June 4, 2024 13:58
@github-actions github-actions bot added the Documentation Documentation needs updating label Jun 4, 2024
@hamistao hamistao force-pushed the issue13495/handle_long_device_names branch from 17826c4 to 441572b Compare June 4, 2024 13:59
@github-actions github-actions bot removed the Documentation Documentation needs updating label Jun 4, 2024
@hamistao
Copy link
Contributor Author

hamistao commented Jun 4, 2024

@tomponline This is ready again. Sorry @ru-fu this was my mistake, I believe your help is not needed for this one.

Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

@hamistao please can you confirm that no device name that previously worked has been changed when passed to qemu, and only longer device names that didn't work previously are now hashed?

Signed-off-by: hamistao <pedro.ribeiro@canonical.com>
…tdevs

Signed-off-by: hamistao <pedro.ribeiro@canonical.com>
Signed-off-by: hamistao <pedro.ribeiro@canonical.com>
Signed-off-by: hamistao <pedro.ribeiro@canonical.com>
@hamistao hamistao force-pushed the issue13495/handle_long_device_names branch from 441572b to 342bef9 Compare June 5, 2024 12:45
@hamistao
Copy link
Contributor Author

hamistao commented Jun 5, 2024

@tomponline Yes, I can. I ran some tests to confirm and any name up to 27 chars is not being hashed.

@hamistao
Copy link
Contributor Author

hamistao commented Jun 5, 2024

@tomponline To be more specific, some tags have a limit of 31 chars and others have a limit of 36. The shorter tags are only used during cold-plugging, so a hot-plugged device could work with the limit of 36 (until VM restart) but not a cold-plugged one. So I chose 31 as the limit to maintain a standard and keep the code simpler. If you think it is best, I could use different limits for each case as to not alter the behavior in the hot-plugging scenario.

@tomponline tomponline merged commit 39599ea into canonical:main Jun 6, 2024
28 checks passed
@hamistao hamistao deleted the issue13495/handle_long_device_names branch June 6, 2024 16:13
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.

VMs can't handle disks with names longer than 27 bytes
2 participants