Re: Patch to fix kldstat(2) / dtrace when booting FreeBSD

["Vladimir 'φ-coder/phcoder' Serbinenko" <phcoder@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2516 bytes --]

On 12.01.2015 17:11, Kris Moore wrote:
> On 01/11/2015 13:27, Andrei Borzenkov wrote:
>> В Fri, 09 Jan 2015 11:31:26 -0500
>> Kris Moore <kris@pcbsd.org> пишет:
>>
>>> The following patch fixes an important issue when booting FreeBSD.
>>> FreeBSD's kldstat(2) function expects that the full pathname will be
>>> provided to kernel / modules. The current GRUB was striping this out and
>>> only leaving the filename itself. This broke dtrace and other things
>>> which used the full pathname to locate the kernel or modules on disk.
>>>
>>> The attached patch fixes this behavior.
>>>
>>> diff --git a/grub-core/loader/i386/bsd.c b/grub-core/loader/i386/bsd.c
>>> index 8f691e0..fb47969 100644
>>> --- a/grub-core/loader/i386/bsd.c
>>> +++ b/grub-core/loader/i386/bsd.c
>>> @@ -415,11 +415,15 @@ grub_freebsd_add_meta_module (const char *filename, const char *type,
>>>  			      grub_addr_t addr, grub_uint32_t size)
>>>  {
>>>    const char *name;
>>> -  name = grub_strrchr (filename, '/');
>>> + /* Don't strip the full path, some FreeBSD functionality, such
>>> +  * as kldstat(2) / dtrace, rely on this. Instead we only need to remove
>>> +  * any ZFS dataset information first. */
>>> +  name = grub_strrchr (filename, '@');
>> What if filename itself contains '@'? Is it possible?
>>
> I don't see anything in the manpages that explicitly prohibits certain
> characters, however, all the modules FreeBSD uses, and ones in ports,
> don't use any special characters of any kind. I suspect that having a
> module with a '@' in it would cause other potential breakage as well.
> 
* dataset name never contains @. So it should be grub_strchr.
* You don't handle the case when name doesn't contain @
* Disk name needs to be stripped first.
* This code is ZFS specific. You need a separate code path for other
filesystems.
> 
>>>    if (name)
>>>      name++;
>>>    else
>>>      name = filename;
>>> +
>> Please, could we avoid unrelated formatting changes?
>>
>>>    if (grub_strcmp (type, "/boot/zfs/zpool.cache") == 0)
>>>      name = "/boot/zfs/zpool.cache";
>>>  
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
> 
> Here you go, without the formatting changes.
> 
> 
> 
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]