Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1088933
Summary: | update grubby to support device tree options for arm | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Dennis Gilmore <dennis> |
Component: | grubby | Assignee: | Brian Lane <bcl> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | unspecified | Docs Contact: | |
Priority: | unspecified | ||
Version: | rawhide | CC: | awilliam, bcl, dennis, djones-proj, kparal, mjg59, pbrobinson, pjones, pwhalen, robatino, satellitgo, swarren |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
Whiteboard: | AcceptedBlocker AcceptedFreezeException | ||
Fixed In Version: | grubby-8.35-7.fc21 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2014-10-20 23:02:34 UTC | Type: | Bug |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 245418, 1043122, 1043124, 1078911 | ||
Attachments: |
Description
Dennis Gilmore
2014-04-17 13:06:02 UTC
Created attachment 888179 [details]
add --devtree support to extlinux
Created attachment 889181 [details]
add support to new-kernel-pkg to update fdtdir on 32 bit arm
Additionally this patch is needed for everything to work right
Are you sure this is right? + if [ "x$devtreefile" != "x" -a -f "$devtreefile" -o -d "$devtreefile" ]; then This is meant to be if $devtree && (is file or is dir), but I think it's actually (if $devtree && is file) or is dir Also, allowing $devtreefile to be a directory is a touch confusing. fdtdir in extlinux.conf tells u-boot a directory to find the dtb files. fdt in extlinux.conf tells u-boot a specific dtb file to use. we want to tell u-boot here to find the dtb files and then let u-boot use its internal knowledge to select the specific dtb file to use. perhaps the better way to deal with it is to have another option to grubby --devtreedir to update fdtdir and have --devtree update a fdt entry if we keep the existing logic then we should make it clearer what it is supposed to do. Created attachment 913961 [details]
patch to update fdt entries in extlinux.conf on arm
Created attachment 913962 [details]
patch to update fdtdir entries in extlinux.conf on arm
I have tested these two patches and I think they are a better way to deal with things. I do need to do some testing on highbank to make sure fdtdir entries don't get added on new kernels as their u-boot will stop processing when encountering an unknown entry. You don't need to use x in the checks: if [ "x$devtreefile" != "x" this should work fine: -n "$devtreefile" And in the devtreedir patch you set DEVTREE instead of DEVTREEDIR *** Bug 1119937 has been marked as a duplicate of this bug. *** Discussed at the 2014-07-16 blocker review meeting: http://meetbot.fedoraproject.org/fedora-blocker-review/2014-07-16/f21-blocker-review.2014-07-16-15.59.log.txt . Accepted as a blocker per criterion https://fedoraproject.org/wiki/Fedora_21_Alpha_Release_Criteria#Release-blocking_images_must_boot , "All release-blocking images must boot in their supported configurations." Discussed at 2014-07-30 blocker review meeting: http://meetbot.fedoraproject.org/fedora-blocker-review/2014-07-30/f21-blocker-review.2014-07-30-15.59.log.txt . Work on this bug seems to have stalled: does anyone have a progress update? Thanks! Dennis,
In attachment 913962 [details] "patch to update fdtdir entries in extlinux.conf on arm", I don't think that making both entries in extlinuxKeywords[] be LT_DEVTREE is correct.
If you do this, there's no way to direct getKeywordByType() to return "fdt" or "fdtdir", since they have the same type. Instead, the first entry, "fdt", will always be returned.
In turn, this means that the code in addNewKernel() which adds an LT_DEVTREE entry in the case where there's a template, but not fdt/fdtdir entry in the template, will always add an "fdt" entry rather than sometimes and "fdt" and sometimes and "fdtdir" entry:
} else if (tmplLine->type == LT_ENTRY_END && needs & NEED_DEVTREE) {
const char *ndtp = newDevTreePath;
if (!strncmp(newDevTreePath, prefix, strlen(prefix)))
ndtp += strlen(prefix);
newLine = addLine(new, config->cfi, LT_DEVTREE,
config->secondaryIndent,
ndtp);
needs &= ~NEED_DEVTREE;
newLine = addLineTmpl(new, tmplLine, newLine, NULL, config->cfi);
} else
I think fixing this would require a new LT_DEVTREEDIR in lineType_e, and perhaps logic like:
when copying fdt from template to new:
if newDevTree is set:
emit an updated fdt line
else:
remove the line /* or error? */
when copying fdtdir from template to new:
if newDevTreeDir is set:
emit an updated fdtdir line
else:
remove the line /* or error? */
at LT_ENTRY_END, if newDevTree was set but not yet copied from template:
emit a new fdt line
at LT_ENTRY_END, if newDevTreeDir was set but not yet copied from template:
emit a new fdtdir line
That probably also requires a new NEED_DEVTREEDIR flag too.
?
Discussed at the 2014-08-13 blocker review meeting: http://meetbot.fedoraproject.org/fedora-blocker-review/2014-08-13/ <dgilmore> ive started on updating things but ive not yet gotten it right <dgilmore> it will be done when i can get time to finish it <pschindl> #info dgilmore has this on his probably veeery full todo list. When there will be time he will finish it. satellit, did you mean to remove the needinfo flag, or just to add yourself to CC? add to cc only Restoring the needinfo flag. This has been sitting around on the Alpha blocker list for like two months now, we really need some clarification on the status here. Is it blocking Alpha? Is it not? Is it fixed? Thanks! I propose we drop this back to a beta blocker and document what needs to be done to work around it. It affects two things: 1) kernel updates of images (primarily once the initial kernel is rotated out) 2) network installs (can be worked around if done using a kickstart) This was discussed at today's go/no-go meeting: http://meetbot.fedoraproject.org/fedora-meeting-2/2014-09-11/f21_alpha_gono-go_meeting_-_2.2014-09-11-17.03.log.html . We agreed to make it BetaBlocker, AlphaFreezeException, and document the workaround. Peter, can you please either write this into CommonBugs or send me a plain text commonbugs-ish note I can add to the page? I can tweak the language and formatting and links and so on, but I need the details about what the bug actually involves and what people have to do. Thanks! Adam, Peter, Added to common bugs under https://fedoraproject.org/wiki/Common_F21_bugs#ARM_issues Discussed at 2014-10-08 blocker review meeting: http://meetbot.fedoraproject.org/fedora-blocker-review/2014-10-08/f21-blocker-review.2014-10-08-15.58.log.txt . pwhalen says pbrobinson is working on this and it's expected soon. Created attachment 946192 [details]
add devtree support to extlinux
Created attachment 946193 [details]
add support to new-kernel-pkg to update fdtdir on 32 bit arm
Created attachment 946195 [details]
cleanup the dtb option handling
with the latest 3 patches everything on the grubby side works as expected in all supported use cases.
grubby-8.35-6.fc21 has been submitted as an update for Fedora 21. https://admin.fedoraproject.org/updates/grubby-8.35-6.fc21 There seems to be a problem in one of Dennis' patches for this. Building a live image with the new grubby I get these errors: /sbin/new-kernel-pkg: line 194: [: too many arguments /sbin/new-kernel-pkg: line 200: [: too many arguments No '/dev/log' or 'logger' included for syslog logging /sbin/new-kernel-pkg: line 460: [: too many arguments /sbin/new-kernel-pkg: line 466: [: too many arguments No '/dev/log' or 'logger' included for syslog logging /sbin/new-kernel-pkg: line 194: [: too many arguments /sbin/new-kernel-pkg: line 200: [: too many arguments The offending bits seem to be these blocks of Dennis' "add support for devicetree directories for use on arm": @@ -191,11 +191,17 @@ install() { fi DEVTREE="" - if [ "x$devtreefile" != "x" -a -f "$devtreefile" ]; then + if [ -n $devtreefile -a -f "$devtreefile" ]; then [ -n "$verbose" ] && echo "found $devtreefile and using it with grubby" DEVTREE="--devtree $devtreefile" fi + DEVTREEDIR="" + if [ -n $devtreedir -a -d "$devtreedir" ]; then + [ -n "$verbose" ] && echo "found $devtreedir and using it with grubby" + DEVTREEDIR="--devtreedir $devtreedir" + fi + # FIXME: is this a good heuristic to find out if we're on iSeries? if [ -d /proc/iSeries ]; then [ -n "$verbose" ] && echo "On an iSeries, just making img file" and: @@ -451,11 +457,17 @@ update() { fi DEVTREE="" - if [ "x$devtreefile" != "x" -a -f "$devtreefile" ]; then + if [ -n $devtreefile -a -f "$devtreefile" ]; then [ -n "$verbose" ] && echo "found $devtreefile and using it with grubby" DEVTREE="--devtree $devtreefile" fi + DEVTREEDIR="" + if [ -n $devtreedir -a -d "$devtreedir" ]; then + [ -n "$verbose" ] && echo "found $devtreedir and using it with grubby" + DEVTREEDIR="--devtreedir $devtreedir" + fi + if [ -n "$cfgGrub" ]; then [ -n "$verbose" ] && echo "updating $version from $grubConfig" ARGS="--grub -c $grubConfig --update-kernel=$kernelImage $INITRD \ So I think this is a non-fatal error, but it should still be fixed. I did a quick test, and it looks like statements of that form break if the variable is unassigned. this works, whether /tmp/foo exists or not: devtreefile="/tmp/foo" if [ -n $devtreefile -a -f "$devtreefile" ]; then echo "True!" else echo "False!" fi but if you comment out the assignment of devtreefile: #devtreefile="/tmp/foo" if [ -n $devtreefile -a -f "$devtreefile" ]; then echo "True!" else echo "False!" fi you get: [adamw@adam grubby (f21 %)]$ sh /tmp/test.sh /tmp/test.sh: line 4: [: too many arguments False! so it seems to continue executing (or else False! wouldn't be printed), but does show an error. Oh, now I see the problem, it's simple - the first occurrence of $devtreefile is unquoted in every case. This works: #devtreefile="/tmp/foo" if [ -n "$devtreefile" -a -f "$devtreefile" ]; then echo "True!" else echo "False!" fi Well, there's a more serious bug in the new grubby - it writes an invalid grub2.cfg and the installed system doesn't boot. There's no initramfs line, and all the LV locations are wrong - it has "root=/dev/mapper/fedora-root" , "rd.lvm.lv=fedora/swap" , "rd.lvm.lv=fedora/root" when the correct location is fedora_localhost/ in each case, i.e. the hostname is missing from the VG path, I guess. If I manually edit all three locations and add the initramfs16 line, the system boots. I don't know if this is somehow caused by the issue I noted above (even though in my test script execution seemed to continue after the 'error'), I can check that by editing new-kernel-pkg in place, I guess. correction - it's just the missing initrd16 line that's incorrect, the VG paths are correct (different between my two test VMs for some reason). I've filed the new bug as https://bugzilla.redhat.com/show_bug.cgi?id=1153410 , since it doesn't look like it's directly related to the ARM device tree stuff. Package grubby-8.35-6.fc21: * should fix your issue, * was pushed to the Fedora 21 testing repository, * should be available at your local mirror within two days. Update it with: # su -c 'yum update --enablerepo=updates-testing grubby-8.35-6.fc21' as soon as you are able to. Please go to the following url: https://admin.fedoraproject.org/updates/FEDORA-2014-12932/grubby-8.35-6.fc21 then log in and leave karma (feedback). Package grubby-8.35-7.fc21: * should fix your issue, * was pushed to the Fedora 21 testing repository, * should be available at your local mirror within two days. Update it with: # su -c 'yum update --enablerepo=updates-testing grubby-8.35-7.fc21' as soon as you are able to. Please go to the following url: https://admin.fedoraproject.org/updates/FEDORA-2014-12932/grubby-8.35-7.fc21 then log in and leave karma (feedback). grubby-8.35-7.fc21 has been pushed to the Fedora 21 stable repository. If problems still persist, please make note of it in this bug report. fixed, commonbugs no longer needed. |