Skip to content

spa: fix signature mismatch for spa_boot_init as eventhandler required #17088

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

Merged
merged 1 commit into from
Feb 25, 2025

Conversation

aokblast
Copy link
Contributor

@aokblast aokblast commented Feb 23, 2025

Motivation and Context

I am working on KCFI for FreeBSD. As KCFI required all function signature matched.

We should have this patch for changing signature.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

Copy link
Member

@amotin amotin left a comment

Choose a reason for hiding this comment

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

Generally the direction is right, just compilers don't appreciate it:

  module/zfs/spa_misc.c:2552:21: error: unused parameter 'dummy' [-Werror,-Wunused-parameter]
   2552 | spa_boot_init(void *dummy)
        |                     ^

Probably needs __unused.

@amotin amotin added the Status: Revision Needed Changes are required for the PR to be accepted label Feb 23, 2025
@aokblast
Copy link
Contributor Author

Emm, I actually add __unused before I force-push it. Here is the Github Action log.

Don't have any idea what happens.

@amotin
Copy link
Member

amotin commented Feb 24, 2025

@aokblast I am not sure where have you pushed it, but I don't see it in the only commit of the branch: aokblast@9eacc1e or master...aokblast:zfs:fix_spa_sig . How about figuring it out and force pushing it again?

@github-actions github-actions bot removed the Status: Revision Needed Changes are required for the PR to be accepted label Feb 25, 2025
@aokblast
Copy link
Contributor Author

I modified it and you may see the compile error. I think it is because the __unused is not defined and it is defined in <sys/cdefs.h>. Should I include it? I assume that all kernel .c file include it. Also it compiles without error in zfs in FreeBSD base.

@ixhamza
Copy link
Member

ixhamza commented Feb 25, 2025

@aokblast - You can explicitly mark parameter as unused like this, same way it's already done in other places:

-spa_boot_init(void)
+spa_boot_init(void* unused)
 {
+       (void) unused;
        spa_config_load();
 }

Also, it would be nice if you can push your patch in a single commit.

@aokblast
Copy link
Contributor Author

Oh, thank you!
I push it separately because I just want to trigger ci for testing.

@amotin amotin added the Status: Accepted Ready to integrate (reviewed, tested) label Feb 25, 2025
@amotin amotin merged commit a5fb5c5 into openzfs:master Feb 25, 2025
23 of 25 checks passed
ixhamza pushed a commit to truenas/zfs that referenced this pull request Feb 25, 2025
Reviewed-by: Ameer Hamza <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Signed-off-by: SHENGYI HONG <[email protected]>
Closes openzfs#17088
ixhamza pushed a commit to ixhamza/zfs that referenced this pull request Feb 27, 2025
Reviewed-by: Ameer Hamza <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Signed-off-by: SHENGYI HONG <[email protected]>
Closes openzfs#17088
lundman pushed a commit to openzfsonosx/openzfs-fork that referenced this pull request Jun 2, 2025
Reviewed-by: Ameer Hamza <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Signed-off-by: SHENGYI HONG <[email protected]>
Closes openzfs#17088
lundman pushed a commit to openzfsonosx/openzfs-fork that referenced this pull request Jun 2, 2025
Reviewed-by: Ameer Hamza <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Signed-off-by: SHENGYI HONG <[email protected]>
Closes openzfs#17088
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants