Page MenuHomeFreeBSD

openzfs: use IN_BASE instead of IN_FREEBSD_BASE in spa.h
ClosedPublic

Authored by yuripv on May 12 2023, 12:09 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Apr 9, 4:27 AM
Unknown Object (File)
Mon, Apr 8, 10:26 PM
Unknown Object (File)
Mon, Apr 8, 11:15 AM
Unknown Object (File)
Sun, Mar 31, 6:53 AM
Unknown Object (File)
Mar 2 2024, 5:54 AM
Unknown Object (File)
Feb 28 2024, 8:16 PM
Unknown Object (File)
Feb 19 2024, 7:16 PM
Unknown Object (File)
Feb 19 2024, 6:17 AM
Subscribers

Details

Summary

This allows libzfs to pick up correct default value for autotrim property.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

imp requested changes to this revision.May 12 2023, 1:58 AM

Yea, we should change spa.h, honestly, rather than this stupid #define. There's already enough stupid, non-idempotent hacks in ZFS integration, and it would be better to fix this instance the right way. You've also missed the need for this in stand...

This revision now requires changes to proceed.May 12 2023, 1:58 AM
git diff
diff --git a/sys/contrib/openzfs/include/sys/spa.h b/sys/contrib/openzfs/include/sys/spa.h
index b96a9ef1d42f..a45d15ee423f 100644
--- a/sys/contrib/openzfs/include/sys/spa.h
+++ b/sys/contrib/openzfs/include/sys/spa.h
@@ -723,12 +723,12 @@ typedef enum spa_mode {
  * Send TRIM commands in-line during normal pool operation while deleting.
  *     OFF: no
  *     ON: yes
- * NB: IN_FREEBSD_BASE is defined within the FreeBSD sources.
+ * NB: IN_BASE is defined within the FreeBSD sources.
  */
 typedef enum {
        SPA_AUTOTRIM_OFF = 0,   /* default */
        SPA_AUTOTRIM_ON,
-#ifdef IN_FREEBSD_BASE
+#ifdef IN_BASE
        SPA_AUTOTRIM_DEFAULT = SPA_AUTOTRIM_ON,
 #else
        SPA_AUTOTRIM_DEFAULT = SPA_AUTOTRIM_OFF,

I *guess* it was done this way as it's the only FreeBSD-specific part in the "common" code, and everything using IN_BASE is located under sys/contrib/openzfs/lib/libzfs/os/freebsd and sys/contrib/openzfs/include/os/freebsd proper.

re: stand, I don't see a call to zpool_prop_init() in there anywhere, zpool_prop.c does not seem to be compiled as well; am I missing something obvious?

yuripv retitled this revision from libzfs: define IN_FREEBSD_BASE to get proper default value for autotrim to openzfs: use IN_BASE instead of IN_FREEBSD_BASE in spa.h.
yuripv edited the summary of this revision. (Show Details)
yuripv added a reviewer: mm.

modify spa.h instead as proposed by Warner

Love it. Thanks for following up on my suggestion

This revision is now accepted and ready to land.May 13 2023, 5:25 PM

Sadly, I had to revert this, with all these defines being ambiguously named I missed the fact that IN_BASE is only used in FreeBSD-specific code to signify userland/kernel differences, while IN_FREEBSD_BASE really means FreeBSD-specific code inside the common code. Will need to think more about proper approach (and naming?) here.