On Feb 14, 2016, at 5:26 AM, Vladimir 'phcoder' Serbinenko < address@hidden> wrote: This patch uses grub_file_open, but the android bootimg is a disk, not
a file. You mentioned something about file_offset_open, but that also
expects an input file, not a disk. Should I modify your patch with my
code I wrote to create a grub_file_t from an android_bootimg disk
device, or is there another approach?
We already have syntax (hd0,1)+<number of sectors> that we use for i.a. chainloader perhaps we should extend it to have (hd0,1)+ meaning whole disk as file? Or even allow the disk to be opened with GRUB_file_open? I'd like a second opinion on this. Andrei, what do you think?
I think syntax that allows a whole disk to be specified (e.g. To the multiboot module command so a disk image can be passed that way) is a great idea.
--S
On 2016-02-12 16:16, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> On 12.02.2016 20:34, Shea Levy wrote:
>> OK. Do you have any thoughts on how best to extract the "load the
>> kernel
>> and set the command line to a specific string" functionality out of
>> the
>> linux command, then?
>>
> At first I wanted to write a short skeleton patch to show what I
> meant
> but once skeleton is done, there is little actual code involved.
> Please
> try attached patch
>> On 2016-02-12 14:22, Vladimir 'phcoder' Serbinenko wrote:
>>> Separate command is better as it keeps interface tidy and
>>> unpoluted,
>>> decreasing maintenance cost. Correct me if I'm wrong but it should
>>> be
>>> clear from context of file is Android image or usual linux one?
>>>
>>> Le ven. 12 févr. 2016 20:19, Shea Levy <address@hidden> a écrit
>>> :
>>>
>>>> On 2016-02-12 12:50, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>>>> > On 08.02.2016 21:47, Shea Levy wrote:
>>>> >> ---
>>>> >> grub-core/loader/i386/linux.c | 27 +++++++++++++++++++++------
>>>> >> 1 file changed, 21 insertions(+), 6 deletions(-)
>>>> >>
>>>> >> diff --git a/grub-core/loader/i386/linux.c
>>>> >> b/grub-core/loader/i386/linux.c
>>>> >> index fddcc46..6ab8d3c 100644
>>>> >> --- a/grub-core/loader/i386/linux.c
>>>> >> +++ b/grub-core/loader/i386/linux.c
>>>> >> @@ -35,6 +35,7 @@
>>>> >> #include <grub/i18n.h>
>>>> >> #include <grub/lib/cmdline.h>
>>>> >> #include <grub/linux.h>
>>>> >> +#include <grub/android_bootimg.h>
>>>> >>
>>>> >> GRUB_MOD_LICENSE ("GPLv3+");
>>>> >>
>>>> >> @@ -695,7 +696,13 @@ grub_cmd_linux (grub_command_t cmd
>>>> >> __attribute__ ((unused)),
>>>> >> goto fail;
>>>> >> }
>>>> >>
>>>> >> - file = grub_file_open (argv[0]);
>>>> >> + char android_cmdline[BOOT_ARGS_SIZE];
>>>> >> + android_cmdline[0] = '';
>>>> >> + if (grub_memcmp (argv[0], "android_bootimg:", sizeof
>>>> >> "android_bootimg:" - 1) == 0)
>>>> >> + grub_android_bootimg_load_kernel (argv[0] + sizeof
>>>> >> "android_bootimg:" - 1,
>>>> >> + &file, android_cmdline);
>>>> >> + else
>>>> >> + file = grub_file_open (argv[0]);
>>>> > I hoped more for autodetection. This gets a bit hairy and proper
>>>> > separation is better. Sorry for confusion. I think it's simpler
>>>> with
>>>> > commands like
>>>> > android_bootimg [--no-cmdline] [--no-initrd] IMAGE
>>>> [EXTRA_ARGUMENTS]
>>>> > by default it will load both IMAGE, with cmdline and initrd.
>>>> With
>>>> > --no-initrd you can use initrd for custom initrd.
>>>>
>>>> Autodetection would be possible actually, I didn't think of that.
>>>> If
>>>> grub_file_open fails, we can try grub_android_bootimg_load_kernel
>>>> on the
>>>> same file. Would that be preferable or do we still want a separate
>>>> command?
>>>>
>>>> >
>>>> >> if (! file)
>>>> >> goto fail;
>>>> >>
>>>> >> @@ -1008,12 +1015,20 @@ grub_cmd_linux (grub_command_t cmd
>>>> >> __attribute__ ((unused)),
>>>> >> linux_cmdline = grub_zalloc (maximal_cmdline_size + 1);
>>>> >> if (!linux_cmdline)
>>>> >> goto fail;
>>>> >> - grub_memcpy (linux_cmdline, LINUX_IMAGE, sizeof
>>>> (LINUX_IMAGE));
>>>> >> + grub_size_t cmdline_offset = 0;
>>>> >> + if (android_cmdline[0])
>>>> >> + {
>>>> >> + cmdline_offset = grub_strlen (android_cmdline) + 1;
>>>> >> + grub_memcpy (linux_cmdline, android_cmdline,
>>>> cmdline_offset -
>>>> >> 1);
>>>> >> + linux_cmdline[cmdline_offset - 1] = ' ';
>>>> >> + }
>>>> >> + grub_memcpy (linux_cmdline + cmdline_offset, LINUX_IMAGE,
>>>> sizeof
>>>> >> (LINUX_IMAGE));
>>>> >> + cmdline_offset += sizeof LINUX_IMAGE - 1;
>>>> > LINUX_IMAGE must be at the beginning. don't forget brackets
>>>> around
>>>> > sizeof.
>>>> >> grub_create_loader_cmdline (argc, argv,
>>>> >> - linux_cmdline
>>>> >> - + sizeof (LINUX_IMAGE) - 1,
>>>> >> - maximal_cmdline_size
>>>> >> - - (sizeof (LINUX_IMAGE) - 1));
>>>> >> + linux_cmdline
>>>> >> + + cmdline_offset,
>>>> >> + maximal_cmdline_size
>>>> >> + - cmdline_offset);
>>>> >>
>>>> >> len = prot_file_size;
>>>> >> if (grub_file_read (file, prot_mode_mem, len) != len &&
>>>> >> !grub_errno)
>>>> >>
>>>> >
>>>> >
>>>> >
>>>> > _______________________________________________
>>>> > Grub-devel mailing list
>>>> > address@hidden
>>>> > https://lists.gnu.org/mailman/listinfo/grub-devel [1]
>>>>
>>>> _______________________________________________
>>>> Grub-devel mailing list
>>>> address@hidden
>>>> https://lists.gnu.org/mailman/listinfo/grub-devel [1]
>>>
>>>
>>> Links:
>>> ------
>>> [1] https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>> _______________________________________________
>>> Grub-devel mailing list
>>> address@hidden
>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>
>>
>> _______________________________________________
>> Grub-devel mailing list
>> address@hidden
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>
>
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel
_______________________________________________
Grub-devel mailing list
address@hidden
https://lists.gnu.org/mailman/listinfo/grub-devel
|