Re: [PATCH] linux16 module: code cleanup
On Tue, Aug 07, 2018 at 02:57:53PM +0800, Cao jin wrote: > 1. move relocator related code more close to each other > 2. use variable "len" since it has correct assignment, and keep coding > style with upper code > > Signed-off-by: Cao jin Reviewed-by: Daniel Kiper Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] relocator16.S: comments update
On Tue, Aug 14, 2018 at 03:03:25PM +0800, Cao jin wrote: > Signed-off-by: Cao jin Reviewed-by: Daniel Kiper Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2] ofnet: Initialize structs in bootpath parser
On Mon, Sep 03, 2018 at 10:09:15AM +0200, Julian Andres Klode wrote: > Code later on checks if variables inside the struct are > 0 to see if they have been set, like if there were addresses > in the bootpath. > > The variables were not initialized however, so the check > might succeed with uninitialized data, and a new interface > with random addresses and the same name is added. This causes > $net_default_mac to point to the random one, so, for example, > using that variable to load per-mac config files fails. > > Bug-Ubuntu: https://bugs.launchpad.net/bugs/1785859 > Signed-off-by: Julian Andres Klode Reviewed-by: Daniel Kiper Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] grub-probe: Sort lists of enums and targets
On Fri, Aug 31, 2018 at 10:15:20AM -0700, Elliott Mitchell wrote: > This makes it distinctly easier to find things in these tables. Both for > humans trying to add entries, and for computers trying to find entries. This patch does more then commit message says. So, please improve the commit message and/or split the patch into logical parts. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] grub-reboot: Warn when "for the next boot only" promise cannot be kept
On Fri, Aug 17, 2018 at 05:33:44PM -0600, dann frazier wrote: > The "for the next boot only" property of grub-reboot is dependent upon > GRUB being able to clear the next_entry variable in the environment > block. However, GRUB cannot write to devices using the diskfilter > and lvm abstractions. > > Ref: https://lists.gnu.org/archive/html/grub-devel/2009-12/msg00276.html > Ref: https://bugs.launchpad.net/bugs/788298 > > Signed-off-by: dann frazier Reviewed-by: Daniel Kiper Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] ahci: Increase time-out from 10 s to 32 s
On Thu, Aug 09, 2018 at 06:10:51PM +0200, Paul Menzel wrote: > Date: Thu, 9 Aug 2018 07:27:35 +0200 > > Currently, the GRUB payload for coreboot does not detect the Western > Digital hard disk WDC WD20EARS-60M AB51 connected to the ASRock E350M1, > as that takes over ten seconds to spin up. > > ``` > disk/ahci.c:533: port 0, err: 0 > disk/ahci.c:539: port 0, err: 0 > disk/ahci.c:543: port 0, err: 0 > disk/ahci.c:549: port 0, offset: 120, tfd:80, CMD: 6016 > disk/ahci.c:552: port 0, err: 0 > disk/ahci.c:563: port 0, offset: 120, tfd:80, CMD: 6016 > disk/ahci.c:566: port: 0, err: 0 > disk/ahci.c:593: port 0 is busy > disk/ahci.c:621: cleaning up failed devs > ``` > > GRUB detects the drive, when either unloading the module *ahci*, and > then loading it again, or when doing a warm reset. > > As the ten second time-out is too short, increase it to 32 seconds, > used by SeaBIOS. which detects the drive successfully. > > The AHCI driver in libpayload uses 30 seconds, and that time-out was > added in commit 354066e1 (libpayload: ahci: Increase timeout for > signature reading) with the description below. > > > We can't read the drives signature before it's ready, i.e. spun up. > > So set the timeout to the standard 30s. Also put a notice on the > > console, so the user knows why the signature reading failed. > > Signed-off-by: Paul Menzel Reviewed-by: Daniel Kiper Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] tests: Fix qemu options for UHCI test
On Mon, Jul 30, 2018 at 12:37:42PM +0100, Colin Watson wrote: > qemu 2.12 removed the -usbdevice option. Use a more modern spelling > instead, in line with other USB-related tests. > > Signed-off-by: Colin Watson Reviewed-by: Daniel Kiper Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] tests: Disable sercon in SeaBIOS
On Mon, Jul 30, 2018 at 12:37:22PM +0100, Colin Watson wrote: > SeaBIOS 1.11.0 added support for VGA emulation over a serial port, which > interferes with grub-shell. Turn it off. > > Signed-off-by: Colin Watson Reviewed-by: Daniel Kiper Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] hostdisk: Fix linux disk cache workaround on multipath disks
On Wed, Jul 25, 2018 at 04:49:15PM +0800, Michael Chang wrote: > In hostdisk.c::grub_util_fd_open_device, there's a workaround to linux disk s#hostdisk.c::grub_util_fd_open_device#grub-core/osdep/linux/hostdisk.c:grub_util_fd_open_device()# > cache described below. > > "Linux has a bug that the disk cache for a whole disk is not consistent with > the one for a partition of the disk." > > The workaround will result in using the partition device for writing the image Which workaround are you referring to? > of which the address offset is calculated to be within it's range, to avoid > the > cache problem of the whole disk device. This sentence is convoluted. Please fix this. > This patch fixed the linux disk cache workaround not being effective for s/fixed/fixes/ > multipath/dm device because its partition device cannot be correctly > determined > by grub_hostdisk_linux_find_partition function. In general I am not happy with the commit message. It is difficult to understand where the problem is and why and how it is fixed. And lack of SOB... Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2] ieee1275: obdisk driver
> On Sep 1, 2018, at 11:10 AM, Daniel Kiper wrote:
>
> On Thu, Aug 30, 2018 at 09:28:52AM -0600, Eric Snowberg wrote:
>>> On Aug 30, 2018, at 8:06 AM, Daniel Kiper wrote:
>>> On Tue, Jul 17, 2018 at 09:59:33AM -0600, Eric Snowberg wrote:
> On Jul 17, 2018, at 7:38 AM, Daniel Kiper wrote:
> On Mon, Jul 16, 2018 at 09:33:17AM -0600, Eric Snowberg wrote:
>>> On Jul 16, 2018, at 7:51 AM, Daniel Kiper
>>> wrote:
>>>
>>> Sorry for late reply but I was busy with other stuff.
>>>
>>> On Thu, Jun 21, 2018 at 01:46:46PM -0600, Eric Snowberg wrote:
> On Jun 21, 2018, at 10:58 AM, Daniel Kiper
> wrote:
> On Fri, Jun 15, 2018 at 09:58:56AM -0600, Eric Snowberg wrote:
>>> On Jun 15, 2018, at 6:02 AM, Daniel Kiper
>>> wrote:
>>> On Wed, May 30, 2018 at 04:55:22PM -0700, Eric Snowberg wrote:
>>>
>>> [...]
>>>
+static char *
+replace_escaped_commas (char *src)
+{
+ char *iptr;
+
+ if (src == NULL)
+return NULL;
+ for (iptr = src; *iptr; )
+{
+ if ((*iptr == '\\') && (*(iptr + 1) == ','))
+{
+ *iptr++ = '_';
+ *iptr++ = '_';
+}
+ iptr++;
+}
+
+ return src;
+}
+
+static int
+count_commas (const char *src)
+{
+ int count = 0;
+
+ for ( ; *src; src++)
+if (*src == ',')
+ count++;
+
+ return count;
+}
+
+static void
+escape_commas (const char *src, char *dest)
>>>
>>> I am confused by this play with commas. Could explain somewhere
>>> where this commas are needed unescaped, escaped, replaced, etc.
>>> Could not we simplify this somehow?
>>
>> I’m open for recommendations.
>
> Great! However, I need more info which layer need what WRT ",”,
AFAIK all layers above expect it:
https://www.gnu.org/software/grub/manual/grub/grub.html#Device-syntax
Everything above this driver expects it to be escaped. Obviously when
>>>
>>> Do you mean from the command line?
>>
>> This goes much further than the command line. For example, it is
>> built right into the disk driver. Look at grub-core/kern/disk.c: 544
>
> This is the last line of the file. So, I am not sure what exactly you
> mean.
>
>> /* Return the location of the first ',', if any, which is not
>> escaped by a '\'. */
>> static const char *
>> find_part_sep (const char *name)
>> {
>> const char *p = name;
>> char c;
>>
>> while ((c = *p++) != '\0')
>> {
>>if (c == '\\' && *p == ',')
>> p++;
>>else if (c == ',')
>> return p - 1;
>> }
>> return NULL;
>> }
>
> OK, this one.
>
>> When the obdisk driver discovers a disk, it must put the disk name in
>> the proper format. Otherwise when grub_disk_open takes place later
>> on, the wrong disk name will eventually get sent back to the obdisk
>> driver.
>
> Then we need proper escaping. And AIUI your driver does that.
>
>>> If yes could you give an example with
>>> proper escaping?
>>>
the driver talks to the actual hardware these devices can not have the
escaped names.
>>>
>>> OK but this is not clear. So, please add a comment explaining it in
>>> the code somewhere.
>>
>> Ok
>>
>>>
> how often this conversions must be done, why you have chosen that
> solution, etc. Then I will try to optimize solution a bit.
Under normal circumstances it only takes place once per disk as they
are enumerated. All disks are cached within this driver so it should
not happen often. That is why I store both versions so I don’t have
to go back and forth. Look at the current driver ofdisk. It has a
function called compute_dev_path which does this conversion on every
single open (grub_ofdisk_open). That does not happen with this new
driver. IMHO this is a much more optimized solution than the current
driver.
>>>
>>> Then OK. However, this have to be explained somewhere in the code.
>>> Additionally, I think that proper variable naming would help too,
>>> e.g. name and name_esc. And I would do this:
>>> - s/decode_grub_devname/unescape_grub_devnam/,
>>> - s/encode_grub_devname/escape_grub_devname/,
>>
>>> - extract unscaping code to the unescape_commas() function;
>>> e
grub-mkrescue: Option --core-compress is in help text but not recognized
Hi, on occasion of https://unix.stackexchange.com/questions/466495 i wonder whether it is a mistake that grub-mkrescue option --core-compress= is declared in include/grub/util/install.h, or whether it is a mistake that there is no code to see anywhere which would recognize the associated key GRUB_INSTALL_OPTIONS_INSTALL_CORE_COMPRESS. (I'd expect it in util/grub-install-common.c where other options from install.h have their key interpreted.) I have no idea what the problem poster hopes to achieve by his options. It's just that --core-compress is listed in the help text and leads to a self-accusation of grub-mkrescue if it is used: $ grub-mkrescue -o output.iso minimal_dir --core-compress=xz grub-mkrescue: --core-compress: (PROGRAM ERROR) Option should have been recognized!? Try 'grub-mkrescue --help' or 'grub-mkrescue --usage' for more information. A run of grub-mkrescue --usage, too, mentions the option. Have a nice day :) Thomas ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: grub-mkrescue: Option --core-compress is in help text but not recognized
Hi, can it be that in commit f23bc65103d045f899f02ade6dd9d1f964cdb707 "Transform -C option to grub-mkstandalone to --core-compress available" it was forgotten to change the old 'C' to GRUB_INSTALL_OPTIONS_INSTALL_CORE_COMPRESS in grub_install_parse() of util/grub-install-common.c ? At least the function understands the arguments of --core-compress but only if key is 'C' rather than GRUB_INSTALL_OPTIONS_INSTALL_CORE_COMPRESS. The function recognizes the other options by their long enum names. Have a nice day :) Thomas ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
