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.