[merged] ostree-prepare-root as init #403

wmanley · 2016-07-19T23:13:28Z

These commits generalise ostree-prepare-root for the purposes of using it to boot Linux for Tegra (Ubuntu 14.04) on the Nvidia Tegra TK1 dev-boards. On the Tegra we don't use an initrd or systemd so the existing way that ostree-prepare-root is integrated is insufficient.

This has required 3 main generalisations. They are mostly separable:

After setup exec a new init rather than exiting.
Cope with /proc not having been mounted for the purposes of reading /proc/cmdline
Allow the eventual sysroot to already be mounted on / rather than /sysroot by using pivot_root where appropriate

I'm aware this has been discussed on the mailing list last October.

I do rather like this ostree model. The main ostree binary itself takes advantage of various complex dependencies for it's advanced behaviour, but I can easily build ostree-prepare-root statically for boot integration.

These changes seem to work well on the device. I've not completed the bootloader integration for this board but I think this work on ostree-prepare-root stands for itself.

I've opened this PR to gain some early feedback such as:

Are these changes within the scope of the tool?
Would they in principle be accepted?
What other TODO items do I need to add to the list before this can be merged?

I'm not sure that this is the workflow that ostree follows but it seemed to be consistent with some of the other PRs open at the moment.

TODO:

Test that I haven't broken the existing systemd+dracut integration
Test that I haven't broken overlayfs integration
Test that we don't reintroduce the bug fixed in 4f75d4e where there are two filesystems mounted at /sysroot
Add automated tests

For future PRs

Build ostree-prepare-root as a static binary

rh-atomic-bot · 2016-07-19T23:13:45Z

Can one of the admins verify this patch?
I understand the following comments:

bot, add author to whitelist
bot, test pull request
bot, test pull request once

giuseppe · 2016-07-20T07:35:14Z

bot, test pull request

gatispaeglis · 2016-07-20T10:06:27Z

Awesome stuff. I have this item on my TODO list as well (https://bugreports.qt.io/browse/QTBUG-52758). I can help with testing the patch set on my reference devices. I should have some time for this during the next week.

cgwalters · 2016-07-22T01:50:37Z

src/switchroot/ostree-prepare-root.c

@@ -221,16 +220,6 @@ main(int argc, char *argv[])

  deploy_path = resolve_deploy_path (root_mountpoint);

-  /* Create a temporary target for our mounts in the initramfs; this will
-   * be moved to the new system root below.


This code was written for a reason in 4f75d4e - are you sure it won't reintroduce that bug?

This code was written for a reason in 4f75d4e - are you sure it won't reintroduce that bug?

I've pushed a fixup that should fix this but haven't yet tested it explicitly.

I've added a TODO list item to test this.

cgwalters · 2016-07-22T01:54:21Z

Your commit messages are excellent and the code style/changes look really good at a high level. What I'm a lot less certain about is whether this is (as you have on your checklist) going to break the initrd case.

I'll do some testing with this too myself.

(Semi-related; I'm guessing if you're not using an initrd you also don't have a /boot partition; have you hit the issues in #215 ?)

gatispaeglis · 2016-07-25T13:02:48Z

The current patch set on systemd+dracut based images results in:
Resolved OSTree target.....
Failed to MS_MOVE /sysroot to 'sysroot': Too many levels of symbolic links

cgwalters · 2016-07-25T14:12:41Z

Maybe it's simplest in the end to basically fork the current ostree-prepare-root.c into ostree-prepare-root-initramfs.c and ostree-prepare-root-bare.c, and add --enable-switchroot-noinitramfs which selects between the two?

gatispaeglis · 2016-07-25T17:36:36Z

I think that could result in too much duplicated code - kernel command line parsing, mounting /boot when single partition system, the overlay (ostree admin unlock) handling logic, and etc.

gatispaeglis · 2016-07-26T13:50:15Z

Verified also the initramfs-less booting, worked seamlessly.

gatispaeglis · 2016-07-26T13:54:23Z

src/switchroot/ostree-prepare-root.c

+  struct stat stbuf;
+
+  if (argc < 2)
+    root_mountpoint = "/sysroot";


Would it make more sense to have root_mountpoint = "/" as the default value, when ostree-prepare-root called without args?

I think this kernel command line looks better:
init=/ostree/boot.1/qt-os/aad0fb830ecea8c807dcdbbd93a88c65dd7489bb6532ef48eb3dc19cae3a7b22/0/usr/sbin/ostree-prepare-root
then:
init=/ostree/boot.1/qt-os/aad0fb830ecea8c807dcdbbd93a88c65dd7489bb6532ef48eb3dc19cae3a7b22/0/usr/sbin/ostree-prepare-root /

All the existing code does ostree-prepare-root sysroot/ from initramfs, so it won't be affected.

Would it make more sense to have root_mountpoint = "/" as the default value, when ostree-prepare-root called without args?

I've remembered why this wasn't a problem for me. I have a symlink /sysroot -> . on my rootfs, so this was being resolved to /.

gatispaeglis · 2016-07-26T14:17:22Z

Also for kernel to be able to use ostree-prepare-root as PID-1, it needs to be a static binary. As you have already mentioned. Further questions are:

Should this binary always be build as static?

Sounds ok to me.
Have a command line arg "--with-static-switchroot"?

Not ideal for installers. As I would prefer to leave it up to users to choose initramfs VS initramfs-less approach. Without requiring users to rebuild the binary, if the opposite approach was selected at compile time.
Build 2 binaries (from the same source code) by default - static and dynamic version of ostree-prepare-root:
ostree-prepare-root (dynamic)
ostree-prepare-root-init (static)

cgwalters · 2016-07-26T15:18:12Z

@gatispaeglis Are you aware of a distribution which supports both initrd and no-initrd that doesn't build from source (i.e. isn't like OpenEmbedded/Buildroot)? At least for my use case (Fedora/CentOS) we're pretty initrd-centric. I guess one could manually set it up with our binaries, but it's not supported by the installer (Anaconda) for example.

@wmanley Do you think you can pick up debugging the initrd case? If not, I can probably give it a shot sometime later this week...let me know so we don't conflict.

wmanley · 2016-07-26T15:38:00Z

Would it make more sense to have root_mountpoint = "/" as the default value, when ostree-prepare-root called without args?

Agreed, fixed.

wmanley · 2016-07-26T16:27:25Z

have you hit the issues in #215 ?

I haven't because I haven't completed bootloader integration yet. For playing with this I've been booting with init=/bin/bash and running exec ostree-prepare-root /.

The Tegra TK1 uses u-boot but is configured to boot from an extlinux configuration file in /boot/extlinux/extlinux.conf so I'll need to do a little more work on it before it'll "just work". The TK1 uses a single ext4 partition for / including /boot, although IIRC the bootloader is configured to first try booting /boot/extlinux/extlinux.conf and then if that fails it falls back to booting /extlinux/extlinux.conf.

wmanley · 2016-07-26T16:33:11Z

Maybe it's simplest in the end to basically fork the current ostree-prepare-root.c into ostree-prepare-root-initramfs.c and ostree-prepare-root-bare.c, and add --enable-switchroot-noinitramfs which selects between the two?

I would disagree. There would be a lot of common code between the two, and this will be a pure generalisation. This means that it's more likely to work or behave helpfully for other more exotic configurations, e.g. booting distros that use an initrd but not systemd/dracut.

wmanley · 2016-07-26T16:39:15Z

The current patch set on systemd+dracut based images results in:
Resolved OSTree target.....
Failed to MS_MOVE /sysroot to 'sysroot': Too many levels of symbolic links

I suspected this might be the case. I've reintroduced /sysroot.tmp in 96801af, although I've not yet had a chance to test it as my connectivity is rather poor this week so can't afford to download a VM for testing.

I'll have to fiddle with the commit messages when squashing at the end.

wmanley · 2016-07-26T17:01:49Z

src/switchroot/ostree-prepare-root.c

-    {
-      perrorv ("Failed to MS_MOVE %s to '%s'", root_mountpoint, destpath);
-      exit (EXIT_FAILURE);
+      if (chdir ("/sysroot.tmp") < 0)


I'm not sure if this chdir is necessary. I haven't tried this yet. I'd like to remove it if it works without.

My automated test passes without it so I've removed it.

wmanley · 2016-07-26T17:09:54Z

Also for kernel to be able to use ostree-prepare-root as PID-1, it needs to be a static binary.

Some data on file sizes built for AMD64 with -O2 and stripped:

configuration	size
glibc static	724K
glibc shared	12K
musl static	30K

wmanley · 2016-07-26T17:13:14Z

@wmanley Do you think you can pick up debugging the initrd case? If not, I can probably give it a shot sometime later this week...let me know so we don't conflict.

I've given it a crack, but I haven't had a chance to test it yet. I'm going to have a play around with using unshare for testing these different use-cases, but I'm not sure how pivot_root will interact with mount namespaces.

wmanley · 2016-07-26T23:53:56Z

All these fixups are getting a little silly so I'll do a rebase and push -f unless anyone has any objections? This will be necessary eventually to drop 3d935aa: "ostree-prepare-root: Don't bother creating /sysroot.tmp"

cgwalters · 2016-07-27T01:04:44Z

Agreed it's force push 🔨 time. (Also, note one can't fixup! fixup! anyways. I use https://github.com/cgwalters/homegit/blob/master/bin/git-addfixup )

gatispaeglis · 2016-07-27T09:04:36Z

Test that I haven't broken the existing systemd+dracut integration

With the latest patch set this works as expected.

Test that we don't reintroduce the bug fixed in 4f75d4e where there are two filesystems mounted at /sysroot

Checked this as well. The patch does not reintroduce the bug fixed in 4f75d4e.

Are you aware of a distribution which supports both initrd and no-initrd that doesn't build from source (i.e. isn't like OpenEmbedded/Buildroot)?

I had a specific (corner case) use case in mind, but probably this is not relevant for the upstream ostree project. I mentioned "an installer", but maybe my use case is better described as a "converter" - a script that converts OpemEmbedded distro into ostree-based distro. Therefore I wanted to provide statically and dynamically build binaries, but I start to think that I can use a static binary for both - initramfs and initramfs-less booting.

wmanley · 2016-07-27T14:23:23Z

Also, note one can't fixup! fixup! anyways

This was fixed in git 1.8.4. See the release notes:

Having multiple "fixup!" on a line in the rebase instruction sheet did not work very well with "git rebase -i --autosquash".

cgwalters · 2016-07-27T14:29:35Z

Regarding static linking - this does seem like an obvious use case for musl. I have absolutely no problem with whatever autoconf bits we need for that.

gatispaeglis · 2016-07-28T11:02:01Z

What other TODO items do I need to add to the list before this can be merged?

I think one more item that needs to be done here to simplify integration with bootloaders includes updating boot/loader/entries/*.conf files. An example of what it currently contains:

initrd /ostree/os-../initramfs-*
options ostree=..
title ..
linux /ostree/os-../vmlinuz-*

When initramfs-* not provided in the tree, assume that booting with ostree-prepare-root as PID1. The options field should be extended to include:

options ostree=.. init=path/to/ostree-prepare-root/in/$ostree/deployment

The current workaround with ostree-grub-generator would be: https://paste.kde.org/pafbskaqk
On u-boot shell this can not be done AFAIK, as the shell is very limited. Appending init= to options would help for bootloader integration code.

Does this sound reasonable?

This supports running ostree on embedded platforms without an initrd. Specificially I'm trying to do bringup on an NVidia Tegra based Jetson TK1 dev board.

When trying to read kernel command-line.

Typically we have our ready made-up up root at `/sysroot/ostree/deploy/.../` (`deploy_path`) and the real rootfs at `/sysroot` (`root_mountpoint`). We want to end up with our made-up root at `/sysroot/` and the real rootfs under `/sysroot/sysroot` as systemd will be responsible for moving `/sysroot` to `/`. We need to do this in 3 moves to avoid trying to move `/sysroot` under itself: 1. `/sysroot/ostree/deploy/...` -> `/sysroot.tmp` 2. `/sysroot` -> `/sysroot.tmp/sysroot` 3. `/sysroot.tmp` -> `/sysroot` This is a refactoring to group all these operations together so I can implement an alternative in terms of `pivot_root`.

...for simplicity. This way we don't need to keep concatenating deploy_path to everything. We can just refer relative to the current working directory. We need to do this after bind-mounting it over itself otherwise our cwd is still on the non-bind-mounted filesystem below.

…d at / This allows ostree-prepare-root outside of the initramfs context where the real rootfs is already mounted at /. We can't use `mount --move` in this case because we would be trying to move / into a subdirectory of itself.

There seemed to be more lower case first letters so I've standardised on that.

cgwalters · 2016-08-02T19:07:21Z

Great, thanks for these patches! Looking forward to iterating from here - I may get some time this week to look at #268

@rh-atomic-bot r+ f3f80d8

rh-atomic-bot · 2016-08-02T19:07:27Z

⌛ Testing commit f3f80d8 with merge 52acb9d...

I'll reuse this for a new ostree-init. Closes: #403 Approved by: cgwalters

This supports running ostree on embedded platforms without an initrd. Specificially I'm trying to do bringup on an NVidia Tegra based Jetson TK1 dev board. Closes: #403 Approved by: cgwalters

When trying to read kernel command-line. Closes: #403 Approved by: cgwalters

Typically we have our ready made-up up root at `/sysroot/ostree/deploy/.../` (`deploy_path`) and the real rootfs at `/sysroot` (`root_mountpoint`). We want to end up with our made-up root at `/sysroot/` and the real rootfs under `/sysroot/sysroot` as systemd will be responsible for moving `/sysroot` to `/`. We need to do this in 3 moves to avoid trying to move `/sysroot` under itself: 1. `/sysroot/ostree/deploy/...` -> `/sysroot.tmp` 2. `/sysroot` -> `/sysroot.tmp/sysroot` 3. `/sysroot.tmp` -> `/sysroot` This is a refactoring to group all these operations together so I can implement an alternative in terms of `pivot_root`. Closes: #403 Approved by: cgwalters

...for simplicity. This way we don't need to keep concatenating deploy_path to everything. We can just refer relative to the current working directory. We need to do this after bind-mounting it over itself otherwise our cwd is still on the non-bind-mounted filesystem below. Closes: #403 Approved by: cgwalters

…d at / This allows ostree-prepare-root outside of the initramfs context where the real rootfs is already mounted at /. We can't use `mount --move` in this case because we would be trying to move / into a subdirectory of itself. Closes: #403 Approved by: cgwalters

There seemed to be more lower case first letters so I've standardised on that. Closes: #403 Approved by: cgwalters

Closes: #403 Approved by: cgwalters

rh-atomic-bot · 2016-08-02T19:25:58Z

☀️ Test successful - status-atomicjenkins
Approved by: cgwalters
Pushing 52acb9d to master...

gatispaeglis · 2016-08-05T12:00:23Z

I've not yet had a play around with the bootloader integration code and I'll want to do a bunch of experimenting to explore the problem space before implementing a proper solution.

@wmanley This is what works well for me. Let me know if you agree with this once your are done with your experimenting. I can do a PR for this, unless you disagree?

From fbfe0e7cae34d192e8c3739bea846ba005f55e48 Mon Sep 17 00:00:00 2001
From: Gatis Paeglis <gatis.paeglis@qt.io>
Date: Thu, 4 Aug 2016 14:11:10 +0200
Subject: [PATCH] Support for booting without initramfs

Upstream status: WIP
---
 src/libostree/ostree-sysroot-deploy.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c
index 913c6e3..58413e2 100644
--- a/src/libostree/ostree-sysroot-deploy.c
+++ b/src/libostree/ostree-sysroot-deploy.c
@@ -1526,20 +1526,31 @@ install_deployment_kernel (OstreeSysroot   *sysroot,
     ostree_bootconfig_parser_set (bootconfig, "linux", boot_relpath);
   }

+  val = ostree_bootconfig_parser_get (bootconfig, "options");
+  kargs = _ostree_kernel_args_from_string (val);
+
   if (dest_initramfs_name)
     {
       g_autofree char * boot_relpath = g_strconcat ("/", bootcsumdir, "/", dest_initramfs_name, NULL);
       ostree_bootconfig_parser_set (bootconfig, "initrd", boot_relpath);
     }
-
-  val = ostree_bootconfig_parser_get (bootconfig, "options");
+  else
+    {
+      g_autofree char *ostree_prepare_root = NULL;
+      /* A proper fix would be to add a define -DLIBSBINDIR='"$(libsbindir)"' and use it here */
+      ostree_prepare_root = g_strdup_printf ("init=/ostree/boot.%d/%s/%s/%d/usr/sbin/ostree-prepare-root /",
+                                             new_bootversion, osname, bootcsum,
+                                             ostree_deployment_get_bootserial (deployment));
+      _ostree_kernel_args_replace_take (kargs, ostree_prepare_root);
+      ostree_prepare_root = NULL;
+    }

   ostree_kernel_arg = g_strdup_printf ("ostree=/ostree/boot.%d/%s/%s/%d",
                                        new_bootversion, osname, bootcsum,
                                        ostree_deployment_get_bootserial (deployment));
-  kargs = _ostree_kernel_args_from_string (val);
   _ostree_kernel_args_replace_take (kargs, ostree_kernel_arg);
   ostree_kernel_arg = NULL;
+
   options_key = _ostree_kernel_args_to_string (kargs);
   ostree_bootconfig_parser_set (bootconfig, "options", options_key);

-- 
2.7.4

wmanley · 2016-08-05T12:59:40Z

@wmanley This is what works well for me. Let me know if you agree with this once your are done with your experimenting. I can do a PR for this, unless you disagree?

@gatispaeglis: Go ahead and raise a PR. I think it would be better to discuss this on it's own PR rather than this one :).

This particular patch won't work for me because of the hard-coded path to /usr/sbin/ostree-prepare-root, as you mention in your comment. I have ostree-prepare-root installed into /opt/ostree/usr/sbin/ostree-prepare-root. The tricky thing here is that using the value of LIBSBINDIR from the current ostree installation may be inappropriate: The image you're deploying may have ostree-prepare-root installed into a different location.

One fix might be to require that ostree-prepare-root exist at $root/usr/sbin/ostree-prepare-root as you have done here. Another would be to install ostree-prepare-root into /boot at deploy time. The former is certainly simpler.

I've built ostree and its deps into /opt because the Ubuntu 14.04 based system I'm trying to use has a glib that's too old to build ostree against. Originally I tried to build ostree as a static binary but gave up on that after getting weird errors, but I didn't try very hard.

gatispaeglis · 2016-08-05T14:11:59Z

Another would be to install ostree-prepare-root into /boot at deploy time. The former is certainly simpler.

Another downside with this approach would be that whenever ostree-prepare-rootis updated, a new /boot/ostree/$os-$bootcsum would need to be created.

Maybe a symlink in/usr/lib/ostree-boot/ostree-prepare-root -> path_to_ostree-prepare-root could work better. Have to think about a better solution, then I can open a new PR.

gatispaeglis · 2016-08-08T15:47:08Z

#442

cgwalters reviewed Jul 22, 2016
View reviewed changes

gatispaeglis reviewed Jul 26, 2016
View reviewed changes

cgwalters added the reviewed label Jul 26, 2016

wmanley reviewed Jul 26, 2016
View reviewed changes

wmanley force-pushed the prepare-root-init branch from 4d0150f to 5cbfc82 Compare July 27, 2016 19:05

wmanley added 8 commits August 1, 2016 16:46

ostree-prepare-root as init: exec init from deployment if run as PID1

cf537b8

This supports running ostree on embedded platforms without an initrd. Specificially I'm trying to do bringup on an NVidia Tegra based Jetson TK1 dev board.

ostree-prepare-root: Cope with /proc not being mounted

de7c99c

When trying to read kernel command-line.

ostree-prepare-root: Make error message capitalisation consistent

62d6e8d

There seemed to be more lower case first letters so I've standardised on that.

ostree-prepare-root: Fix typo in error message

43a543f

ostree-prepare-root: Add test that overlayfs over /usr works.

f3f80d8

wmanley force-pushed the prepare-root-init branch from af967c5 to f3f80d8 Compare August 1, 2016 16:37

rh-atomic-bot pushed a commit that referenced this pull request Aug 2, 2016

ostree-prepare-root: Refactor code to resolve deploy_path

1364e6e

I'll reuse this for a new ostree-init. Closes: #403 Approved by: cgwalters

rh-atomic-bot pushed a commit that referenced this pull request Aug 2, 2016

ostree-prepare-root: Cope with /proc not being mounted

a0a4052

When trying to read kernel command-line. Closes: #403 Approved by: cgwalters

rh-atomic-bot pushed a commit that referenced this pull request Aug 2, 2016

ostree-prepare-root: Make error message capitalisation consistent

6356eda

There seemed to be more lower case first letters so I've standardised on that. Closes: #403 Approved by: cgwalters

rh-atomic-bot pushed a commit that referenced this pull request Aug 2, 2016

ostree-prepare-root: Fix typo in error message

78f99d4

Closes: #403 Approved by: cgwalters

rh-atomic-bot pushed a commit that referenced this pull request Aug 2, 2016

ostree-prepare-root: Add test that overlayfs over /usr works.

52acb9d

Closes: #403 Approved by: cgwalters

rh-atomic-bot changed the title ~~ostree-prepare-root as init~~ [merged] ostree-prepare-root as init Aug 2, 2016

rh-atomic-bot closed this in 94e50ca Aug 2, 2016

wmanley deleted the prepare-root-init branch August 10, 2016 16:27

wmanley mentioned this pull request Aug 30, 2016

[merged] ostree-prepare-root: Allow building statically with musl #477

Closed

[merged] ostree-prepare-root as init #403

[merged] ostree-prepare-root as init #403

Conversation

wmanley commented Jul 19, 2016 • edited Loading

rh-atomic-bot commented Jul 19, 2016

giuseppe commented Jul 20, 2016

gatispaeglis commented Jul 20, 2016

cgwalters Jul 22, 2016

Choose a reason for hiding this comment

wmanley Jul 26, 2016

Choose a reason for hiding this comment

wmanley Jul 26, 2016

Choose a reason for hiding this comment

cgwalters commented Jul 22, 2016

gatispaeglis commented Jul 25, 2016

cgwalters commented Jul 25, 2016

gatispaeglis commented Jul 25, 2016

gatispaeglis commented Jul 26, 2016

gatispaeglis Jul 26, 2016 • edited Loading

Choose a reason for hiding this comment

wmanley Jul 27, 2016

Choose a reason for hiding this comment

gatispaeglis commented Jul 26, 2016

cgwalters commented Jul 26, 2016

wmanley commented Jul 26, 2016

wmanley commented Jul 26, 2016

wmanley commented Jul 26, 2016

wmanley commented Jul 26, 2016

wmanley Jul 26, 2016

Choose a reason for hiding this comment

wmanley Jul 26, 2016

Choose a reason for hiding this comment

wmanley commented Jul 26, 2016

wmanley commented Jul 26, 2016

wmanley commented Jul 26, 2016

cgwalters commented Jul 27, 2016

gatispaeglis commented Jul 27, 2016

wmanley commented Jul 27, 2016

cgwalters commented Jul 27, 2016

gatispaeglis commented Jul 28, 2016 • edited Loading

cgwalters commented Aug 2, 2016

rh-atomic-bot commented Aug 2, 2016

rh-atomic-bot commented Aug 2, 2016

gatispaeglis commented Aug 5, 2016 • edited Loading

wmanley commented Aug 5, 2016 • edited Loading

gatispaeglis commented Aug 5, 2016 • edited Loading

gatispaeglis commented Aug 8, 2016

wmanley commented Jul 19, 2016 •

edited

Loading

gatispaeglis Jul 26, 2016 •

edited

Loading

gatispaeglis commented Jul 28, 2016 •

edited

Loading

gatispaeglis commented Aug 5, 2016 •

edited

Loading

wmanley commented Aug 5, 2016 •

edited

Loading

gatispaeglis commented Aug 5, 2016 •

edited

Loading