Hi Scott, I took a look at the code and here are some very high level comments: - we only call into block_device_operations.sec_ops from the ioctl handlers. So instead of adding it to the block layer I'd rather structure the code so that the driver itself calls a new common blkdev_sed_ioctl handler implemented in lib/sed.c, which then gets callbacks passed directly from the calling, similar to how opal_unlock_from_suspend works. And the callbacks might actually be condensed to one I think, given that all potential implementations would basically just dispatch to two different opcode but otherwise use the same implementation. - talking about lib/sed*.c - I'd move it to block/ - there are a lot of levels of indirection in the code, I think we can condense them down a bit to basically just having the main blkdev_sed_ioctl entry point, which should check bdev_sec_capable first, and then dispatch to the security types, probably through a little method table. - what's so special about request_user_key that it can't be inline into the only caller but needs a separate file? - please don't use pointer indirections in your userspace ABI, struct sed_key will be a pain to handle for 32-bit userspace on 64-bit kernels. I don't fully understand what the key_type is for anyway - it seems like exactly one type is supported per call anyway. -- To unsubscribe from this list: send the line "unsubscribe linux-block" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
