On Mon, 07 Feb 2022 14:15:07 +0100 Josselin Poiret via Grub-devel <grub-devel@gnu.org> wrote:
> Hi Fabian, > > Fabian Vogt <fv...@suse.de> writes: > > > Did you have a look at my approach? That effectively does the same, but > > using a > > single ioctl instead of anything complex with DM directly. I skipped this thread because I didn't really care about probing for LUKS2 (I don't use it). However, things change and now I'm evaluating the different patch series that implement this. I had missed Fabian's patch and I'm just now seeing it and this thread. Now that I've taken a look at Fabian's patch, I think its the better way to go for getting the sector size. The reason is that it uses grub_util_get_fd_size() to get the size and sector size of the cheat mount device. That function's implementation is platform dependant. So, for instance, if FreeBSD has a LUKS* implementation that is not libdevicemapper based, we'll correctly get the number of sectors and sector size. But yes, its very unlikely that FreeBSD will support LUKS. However, this same code should allow us to get the correct sector size for GELI devices also. I don't believe GRUB has GELI probe support, so ths would make things easier for whoever wants to develop it. The same goes for future cryptodisk backends. > I agree that it's sufficient for sector_size, but we still need the > cryptodisk algorithm so that grub-install will know which crypto modules > to include. libdm is actually a helper library around dm-specific I'm now thinking that GRUB should use Fabian's patch to get the sector size and number of sectors and Josselin's method of querying device mapper to get the other properies. We could have Josselin's patch first check if log_sector_size is set and if not then try to get it from DM. However, I think that's overkill and not really useful because grub_util_get_fd_size() should always give us the correct information, otherwise we've found a kernel bug. Also, by not having to parse out the sector size from the dm-crypt params string, we avoid much the complexity of that code, which has been the source of several bugs in reviewing a couple iterations of that patch. > ioctls, since they are apparently annoying to use, so in the end we're > still using the same interface :) Technically different interface, the code Fabian is using is not using device mapper related ioctls, they are generic block device ioctls (on Linux systems). > As for the 4 patches, since the actual sector_size is available to us, I > think using 512 in all cases or adding a specific 0 sector_size is > something we should avoid. I agree. Fabian, would you be able to send an updated patch with Patrick's suggestions in the near future? Glenn _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
