On Thu, Jun 26, 2025 at 12:14:43PM +0200, Christoph Hellwig wrote: > On Thu, Jun 26, 2025 at 10:38:11AM +0200, Christian Brauner wrote: > > > + if (sb->s_op->remove_bdev) > > > + sb->s_op->remove_bdev(sb, bdev, surprise); > > > + else if (sb->s_op->shutdown) > > > sb->s_op->shutdown(sb); > > > > This makes ->remove_bdev() and ->shutdown() mutually exclusive. I really > > really dislike this pattern. It introduces the possibility that a > > filesystem accidently implement both variants and assumes both are > > somehow called. That can be solved by an assert at superblock initation > > time but it's still nasty. > > > > The other thing is that this just reeks of being the wrong api. We > > should absolutely aim for the methods to not be mutually exclusive. I > > hate that with a passion. That's just an ugly api and I want to have as > > little of that as possible in our code. > > Yes. As I mentioned before I think we just want to transition everyone > to the remove_bdev API. But our life is probably easier if Qu's series > only adds the new one so that we can do the transitions through the > file system trees instead of needing a global API change. Or am I > overthinking this? I think it's fine to do it right now. If we can avoid having transitional stages then let's do that.
