Le lun. 7 mars 2016 20:00, Peter Jones <pjo...@redhat.com> a écrit :
> On Sat, Mar 05, 2016 at 11:38:00AM +0300, Andrei Borzenkov wrote: > > 04.03.2016 23:06, Peter Jones пишет: > > > On Wed, Mar 02, 2016 at 03:01:03PM +0000, Vladimir 'phcoder' > Serbinenko wrote: > > >> Hello, all. I went through the list of bugs and created a shortlist > of bugs > > >> that need to be looked at for 2.02. I have marked them with > plan_release_id > > >> set to 2.02. > > >> Statistics: [1] > > >> Search (with loads of false positives unfortunately): [2] > > >> Not every bug there is a release blocker, for some of them it just > would be > > >> nice to know status before releasing. Some of them are probably > already > > >> fixed. > > >> > > >> Additionally I created a category "Hardware specific" [3]. Bugs there > are > > >> not release blockers but fixing them could benefit the release. > > > > > > In the interest of fixing them up eventually, here's a chunk > > > of ones that look reasonably well-suited for upstream without much work > > > on them, which I've rebased against your master branch today. > > > > > > https://github.com/vathpela/grub2-fedora/tree/for-upstream > > > > > > Most of these are not critical for this release - really only 3 of > them. > > > Here are some notes on each; I can send them individually to the list > if > > > you think it's worthwhile. > > > > > > There are only a couple that are "critical", and we really want in this > > > release: > > > > > > bf4d216 Fix crash on http > > > > Fixed differently in 92c8f58d973a621890e302cba3d80fe0bbc208d7 > > Oh, thanks, I missed that. > > > > 78b3509 Update to minilzo-2.08 > > > > I know. But the sheer size of update makes me afraid of doing it now. > > Yeah, I see that concern, I'm just worried there's a CVE here we're > missing. FWIW we've been carrying that patch for 15 months or so > without seeing any issue whatsoever, but then we're not building for > more extravagant platforms - it's really just the x86, arm, and ppc > families. > > > > eaa05aa Failed config now returns exit code (#1252311) > > > > I think it makes sense. > > > > > Then these are just generic network handling patches: > > > > > > 836b528 DHCP client ID and UUID options added. > > > > Use case would be interesting. You can query arbitrary option already. > > > > > eb1adf5 trim arp packets with abnormal size > > > > > > > nb is not used anywhere in this branch, so I do not understand what this > > code does at all. > > Fair enough; I'll move these two for later and try to figure them out. > I think I do have another patch that uses the DHCP one that I didn't put on > this list, but it's completely likely that I'm just carrying an old > patch that's been supplanted by something else. > > > > And these are hardware specific. They're not critical, in the sense > > > that I can keep carrying them if you have any problems and we can work > > > it out for the /next/ release. > > > > > > beee9fc Add vlan-tag support on IBM PPC machines > > > > Yes, openSUSE (and I think SUSE) also carries variant of this patch. We > > probably need to revisit it. > > > > > 93a6fae IBM client architecture (CAS) reboot support > > > > Is in (open)SUSE as well > > > > > 297d32d for ppc, reset console display attr when clear screen > > > > Does not apply cleanly to upstream (is on top of some other patch?) > > I'm not seeing any reason this should not apply. I think probably the > literal nonprintable ^L in the string breaks "git am" (or "git apply"), > and if you "git fetch" + "git cherry-pick" it works? Or add the file to > .gitattributes as: > > grub-core/term/terminfo.c binary > > And then do "git format-patch $commitid" and apply that result. But > that's a blunt enough hammer that it's not something we really want to > /commit/ for a .c file, I would think, since it makes reading the > patches difficult. Also you have to fetch the remote anyway, so > cherry-pick makes more sense. > > At the same time, we probably ought to change it to \x0c instead of the > ^L. So I've done that in my current version of the patch, but it won't > help this time. > > > > 0ca5375 Disable GRUB video support for IBM power machines > > > > Is in (open)SUSE as well > > > > > 2f3c666 Add support for UEFI operating systems returned by os-prober > > > > We already support it for quite some time, although differently (based > > on os-prober support). > > Ah, yeah, looks like f25870887b7. Thanks. > > > > 05f2dc3 Make efi machines load an env block from a variable > > > > This needs discussion. As well, as openSUSE "store environment block in > > btrfs reserved area" patch. And there was intention to use PV reserved > > areas for it as well. > > Completely fair. I picked an EFI variable because I was using this to > control some debugging code that's still in progress, and it's nice that > you can set it before the bootloader starts (or restore it using > dmpstore, etc.) > > > > 1f1a695 Make "exit" take a return code. > > > > > > > What about > > https://lists.gnu.org/archive/html/grub-devel/2016-01/msg00049.html and > > followup? > > Well, that's a good question. The code in this patch is sort of a > middle ground there - it makes GRUB_EFI_LOAD_ERROR the default, and > makes "exit" and "exit N" both work. So if you do "exit 0", you get no > fallback to the next item, but "exit" alone, "exit N" for any N but 0, > and all the failure paths in the C code all wind up showing the next > menu item. > > I can make it another command if you like, but it seems like a pretty > natural semantic for exit to have. So the issue is that it won't do > anything on a bunch of platforms other than what they're already doing. > Is that a big deal? We have plenty of commands that perform a level of > functionality based on the underlying support. > > > > The rest are all about the git repo and compilers and similar. These > > > are well into "nice to have" for 2.02. I can re-send them after the > > > release, they're just what's left on my list that's pretty close to > > > ready for upstream. > > > > > > cb62c40 Mark po/exclude.pot as binary so git won't try to diff > nonprintables. > > > > Does it cause a problem? It does not look like there are many of them. > > Well, in the glorious world where we have a tarball newer than > 0d6498a67d4 it should cause a lot fewer problems :) But if your > build does: > > tar xf grub-2.02~beta2.tar.xz > cd grub-2.02~beta2 > git init > git add . > git commit -a -m grub-2.02-beta2 > git am <all the patches we want since then> > > Then that last step fails because nonprintables in diff really do not > work well. This patch makes "git format-patch" output that file's diff > as a binary diff instead, and so it works. > > I wouldn't want to do this for a C file, but exclude.pot doesn't see a > lot of changes. And it has a lot of junk in it to begin with - that's > what it exists for. > > Still, if we have actual regular releases, this isn't particularly > important. It's just if distros (or users) wind up applying a pile of > patches from the repo to make their packages that it becomes an issue. > > > > e0bb91a Fix bzr's ignore artificats in .gitignore > > Did you accidentally skip this one? > > > > ecaecc9 Add some __unused__ where gcc 5.x is more picky about it. > > > > How this can become unused? > > > > > grub_gettext_env_write_lang (struct grub_env_var *var > > > __attribute__ ((unused)), const char *val) > > > { > > > - grub_err_t err; > > > + grub_err_t __attribute__((__unused__)) err; > > > err = grub_gettext_init_ext (&main_context, val, grub_env_get > ("locale_dir"), > > > grub_env_get ("prefix")); > > > if (err) > > > > And in normal, entry is used. So more detailed explanation how either of > > them become unused is needed (and BTW it is compiled with gcc 5.x on > > openSUSE and apparently without errors). > > Yep, you're right - sorry about that, the last one should have been > stripped out - it's the artifact of another patch that's not really > upstreamable in its current form. The whole first file looks valid to > me, though? I'll rebase it as two patches and leave one of them in > for-upstream. > > > > e704140 Move bash completion script (#922997) > > > > Well, this is obvious compatibility question. Is there any way to detect > > it at configure time? Does bash have pkg-config or similar? > > I don't see anything obviously like that, unfortunately, and I'm not > really sure in what version they switched it. > > > > bc5d351 Allow "fallback" to include entries by title, not just number. > > > > What about multiple entries? fallback allows them. > > I'm not sure I understand your question. This still allows that if you > treat them numerically or by id. I suppose it's possible to break the > value up as a list of quoted strings to test by title, but it gets messy > with corner cases pretty quickly. FWIW the documentation *doesn't* say > that it allows multiple entries, but *does* say, more or less by > accident, that you can use titles: > ------------------------------------------------- > @node fallback > @subsection fallback > > If this variable is set, it identifies a menu entry that should be > selected if the default menu entry fails to boot. Entries are > identified in the same way as for @samp{default} (@pxref{default}). > ------------------------------------------------- > and then: > ------------------------------------------------- > @node default > @subsection default > > If this variable is set, it identifies a menu entry that should be > selected by default, possibly after a timeout (@pxref{timeout}). The > entry may be identified by number or by id. > > For example, if you have: > > @verbatim > menuentry 'Example GNU/Linux distribution' --class gnu-linux --id > example-gnu-linux { > ... > } > @end verbatim > > then you can make this the default using: > > @example > default=example-gnu-linux > @end example > > If the entry is in a submenu, then it must be identified using the > titles of each of the submenus starting from the top level followed by > the number or title of the menu entry itself, separated by @samp{>}. > For example, take the following menu structure: > > @example > Submenu 1 > Menu Entry 1 > Menu Entry 2 > Submenu 2 > Submenu 3 > Menu Entry 3 > Menu Entry 4 > Menu Entry 5 > @end example > > ``Menu Entry 3'' would then be identified as > @samp{Submenu 2>Submenu 3>Menu Entry 3}. > ... > ------------------------------------------------- > (mildly reformatted for mail purposes.) > > So this patch actually brings it closer into conformance with the > documentation, but still allows multiple fallbacks by index or id to > work as they currently do. > > > > 7401bf6 Honor a symlink when generating configuration by grub2-mkconfig > > > > Makes sense > > > > > 5212412 Fix bad test on GRUB_DISABLE_SUBMENU. > > > > What is bad here? The documented valued is "y". > > Actually the real issue is that we're inconsistent among many variables, > where we use (and document) "yes", "y", and "true". So we discovered a > tendency to put the wrong thing in, and I got tired of it and made this > one take either of those. Maybe this should be addressed more > systemically with a function to judge true or false that recognizes > 1/true/y/yes as true? > > > > 73545c7 Add GRUB_DISABLE_UUID. > > > > If name as detected by GRUB is correct, there will be no search because > > hints will be correct (just direct verification that device is indeed > > correct). If name is wrong you need search, otherwise you fail to boot > > or boot wrong binary. I do not see what we gain here. > > So, the bug report from our QA dept believed > GRUB_DISABLE_LINUX_UUID=true should accomplish this, and that it's > pointless without it. And I think they've kind of got a point, since if > the user has the problem GRUB_DISABLE_LINUX_UUID was meant to solve, > there's no reason to believe they can't have the same problem with the > other filesystem. We made them separate settings because one is about > /boot and one is about / , but fundamentally they're both doing parts of > the same thing. > > I'm not really disagreeing with your logic here - but if we have it > avoiding UUIDs for one of those, it would seem like the other would also > be something we can avoid. > > > >> I would also appreciate if distros would tell which patches they would > > >> carry if 2.02 was released as it is now. If some patches are in more > than 1 > > >> distro we probably need to look into including them. > > > > > > Well, I have a bunch of patches that need to be clean up (or even > > > re-examined), and I've also got the secure-boot branch here: > > > > > > https://github.com/vathpela/grub2-fedora/tree/sb > > > > > > Which is all the patches distros should be carrying to work with Secure > > > Boot correctly. This branch is also recently rebased against master, > > > though I'm not sure what the current thinking is regarding their path > > > upstream. > > > > > > > Personally I'd rather include support for it. I'm tired of linux vs. > > linuxefi nightmare, and patches have been in the wild long enough. > > So what's the path forward, then? Just make all efi use linuxefi, like > linux vs linux16? That's pretty close to what I've got already, except > on arm where it's just "linux" in EFI mode as well. But we could make > those aliases for the same thing on that platform easily enough. Or do > you have something else in mind? RedHat/Fedora config is too platform-dependent and platform is detected at mkconfig time rather than at runtime. This is a problem as runtime and mkconfig can be different. Case that I see often is coreboot failing due to use of Linux16 (which is a valid protocol for coreboot and is used for memtest but Linux crashes with it) but other cases exist, like enabling or disabling of SCM or moving disk to another computer. Can we fix this by introducing some helper to detect it on runtime? It can either be a function or a real command > > Anyway, I've moved the patches that clearly need some more work to a > branch called "for-upstream-wip", and fixed up the ones still in > "for-upstream" as I've mentioned above. > > Thanks! > > -- > Peter >
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
