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

collection: don't copy btf.Spec in CollectionSpec.Copy() #584

Merged
merged 1 commit into from Feb 23, 2022

Conversation

ti-mo
Copy link
Collaborator

@ti-mo ti-mo commented Feb 23, 2022

Copying btf.Spec requires updating Map/Program.BTF.(S,s)pec, which gets complicated due to btf.Map/Program being pointers.

Instead of implementing a full deep-copy, avoid copying btf.Spec for now, since it's considered read-only. Since we're planning to get rid of btf.Program/Map anyway, this would not be worth the investment.

Add a test that loads both a copy and an original CollectionSpec.

@ti-mo ti-mo requested a review from lmb February 23, 2022 15:27
@lmb
Copy link
Collaborator

lmb commented Feb 23, 2022

Ci errors are real, you're missing a testutils.SkipIfNotSupported.

Copying btf.Spec requires updating Map/Program.BTF.(S,s)pec, which gets
complicated due to btf.Map/Program being pointers.

Instead of implementing a full deep-copy, avoid copying btf.Spec for now,
since it's considered read-only. Since we're planning to get rid of
btf.Program/Map anyway, this would not be worth the investment.

Add a test that loads both a copy and an original CollectionSpec.

Signed-off-by: Timo Beckers <timo@isovalent.com>
@ti-mo ti-mo merged commit bf5a6cd into cilium:master Feb 23, 2022
@ti-mo ti-mo deleted the tb/collspec-btf-copy branch February 23, 2022 19:22
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

2 participants