Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for booting without initramfs #442

Conversation

gatispaeglis
Copy link
Contributor

@gatispaeglis gatispaeglis commented Aug 8, 2016

WIP

I have tested that this patch works fine with u-boot and grub backends.

What is missing here is an automake rule that would create:

/usr/lib/ostree-boot/ostree-prepare-root-> ../../sbin/ostree-prepare-root

or other way around:

/usr/sbin/ostree-prepare-root-> ../lib/ostree-boot/ostree-prepare-root

@cgwalters You mentioned once that you want to moveostree-prepare-rootand ostree-remount from sbin to libexecdir. I am thinking that maybe /usr/lib/ostree-boot is a better location? As this path would be the same on all systems and seems like a more natural place for all these "boot related" files (including ostree-grub-generator). Having this well known path would guarantee that this always works:

init=/ostree/boot.%d/%s/%s/%d/usr/lib/ostree-boot/ostree-prepare-root /

Related change: 54168bc#diff-f0c889fb056dc4a713f8a87702fdd27eR1452
When copying /usr/lib/ostree-boot, exclude ostree-{grub-generator, remount, prepare-root}

@rh-atomic-bot
Copy link

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

@jlebon
Copy link
Member

jlebon commented Aug 8, 2016

bot, retest this please

new_bootversion, osname, bootcsum,
ostree_deployment_get_bootserial (deployment));
_ostree_kernel_args_replace_take (kargs, ostree_prepare_root);
ostree_prepare_root = NULL;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor but this can now be: _ostree_kernel_args_replace_take (kargs, g_steal_pointer (&ostree_prepare_root));

@cgwalters
Copy link
Member

I think it'd be fine to just move the binaries unconditionally - systemd execs them via the .service files so the indirection should be OK.

else
{
g_autofree char *ostree_prepare_root = NULL;
ostree_prepare_root = g_strdup_printf ("init=/ostree/boot.%d/%s/%s/%d/usr/lib/ostree-boot/ostree-prepare-root /",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will only work though if statically linked...which makes me think this should be part of an #ifdef BUILDOPT_NO_INITRD or something. (And then fatally error out if we do find an initrd in that case?)

Copy link
Contributor Author

@gatispaeglis gatispaeglis Aug 9, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a better backward compatibility I suggest: #ifdef BUILDOPT_STATIC_PREPARE_ROOT

Now when ostree is build with static ostree-prepare-root users

  • can still use initramfs code path, if initramfs-* found (by using the static version inside of initramfs)
  • when initramfs-* not found, appendinit= kernel arg.

and if there are any users out there that were using OSTree without initramfs already (by hacking kernel sources to include the ostree-prepare-root logic), they can continue doing that by not compiling ostree-prepare-root statically (BUILDOPT_STATIC_PREPARE_ROOT not defined and therefore "init=" not appended to the kernel args)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's set aside the prepare-root-in-kernel case since I'm having enough trouble keeping the support matrix in my head 😄 and I doubt anyone is doing it. The overhead of an additional exec in the startup path is small enough, and avoids having to carry kernel patches, etc.

So, I wouldn't disagree that statically linked users can still use an initramfs, but it'd be odd to have a static switchroot inside an initramfs. I guess if it's small enough, I could imagine "general purpose" distros like Debian/CentOS always building it statically and allowing people to make initrd-less systems that use ostree, even if it's not going to be the default. In my world (CentOS/Fedora) though we're not really focused on the embedded cases, so we basically always have an initrd.

Anyways, I'm ok with renaming the build option to #ifdef BUILDOPT_STATIC_PREPARE_ROOT which is I guess what all this boils down to 😄

So...aside from minor bits, are you thinking we merge this patch as is, or is it more of an RFC and we'll update it with build options later?

@cgwalters
Copy link
Member

Just some minor comments. I think the next important step here is adding the build machinery to statically link the switchroot binary, right?

@gatispaeglis
Copy link
Contributor Author

I think the next important step here is adding the build machinery to statically link the switchroot binary, right?

@wmanley any success with the 'musl' patch?

@gatispaeglis
Copy link
Contributor Author

Let's set aside the prepare-root-in-kernel case since I'm having enough trouble keeping the support matrix in my head

Good :)

So...aside from minor bits, are you thinking we merge this patch as is, or is it more of an RFC and we'll update it with build options later?

I pushed it here for the discussion purposes. I want to update this patch to include the configure switch: --with-static-prepare-root. Only my autotools knowledge is limited, so I will keep it simple. The "musl" version can be added as an improvement later on.

@cgwalters cgwalters added the WIP label Aug 9, 2016
@wmanley
Copy link
Member

wmanley commented Aug 10, 2016

@wmanley any success with the 'musl' patch?

Not yet. I had patch to build manually with musl. I haven't yet worked on the autotools bits. I don't think it will be too difficult but I don't think I'm going to have a chance to work on it this week.

@gatispaeglis gatispaeglis force-pushed the no-initramfs-bootloader-support branch from a957bc1 to c34de2b Compare August 10, 2016 14:01
@gatispaeglis
Copy link
Contributor Author

This patch is still Work-In-Progress. There are few more issues that I need to resolve.

@gatispaeglis gatispaeglis force-pushed the no-initramfs-bootloader-support branch from e374c0e to d0fe11e Compare August 15, 2016 09:29
@gatispaeglis gatispaeglis mentioned this pull request Aug 18, 2016
@gatispaeglis gatispaeglis force-pushed the no-initramfs-bootloader-support branch 2 times, most recently from daeb57d to 5e16918 Compare August 26, 2016 10:30
@gatispaeglis
Copy link
Contributor Author

Squashed the autotest into the relevant commit.

@rh-atomic-bot
Copy link

☔ The latest upstream changes (presumably 42dab85) made this pull request unmergeable. Please resolve the merge conflicts.

@wmanley
Copy link
Member

wmanley commented Aug 30, 2016

@wmanley any success with the 'musl' patch?

musl patch merged as part of #477.

@gatispaeglis gatispaeglis force-pushed the no-initramfs-bootloader-support branch from 5e16918 to 3172d58 Compare August 31, 2016 11:02
@rh-atomic-bot
Copy link

☔ The latest upstream changes (presumably 661e463) made this pull request unmergeable. Please resolve the merge conflicts.

@wmanley
Copy link
Member

wmanley commented Aug 31, 2016

musl patch merged as part of #477.

Also further musl fixes in #485

With the current approach, when ostree-prepare-root is used
on the kernel command line as init=, it always assumes that
the next value in the argument list is a path to the sysroot.
The code for falling back to a default path (if none is provided),
would only work if init= is the last arg in the argument list.
We can not rely on that and have to explicitly provide the
path to the sysroot. Which defeats the purpose of a default
path selection code.

To keep command line neater assume that sysroot is on / when
using ostree-prepare-root as init. This probably is what most
people want anyways. Also _ostree_kernel_args* API assumes
that args are space separated list. Which is problematic for:
"init=${ostree}/usr/lib/ostree/ostree-prepare-root /" as it
gets split in two.
If the current deployment has "rootwait root=/dev/sda2",
but the new deployment does not need "rootwait" anymore,
there is no way to clear this arg at the moment (as opposed
to "karg=root=", which overrides any earlier argument with
the same name). With "--karg-none" users can now clear all
the previous args and set new "root=":

ostree admin deploy --karg-none --karg=root=LABEL=rootfs
Previously when initramfs-* was not found in a deployment's
boot directory, it was assumed that rootfs is prepared for
ostree booting by a kernel patch.

With this patch, the behaviour changes to be - if initramfs-*
is not found, assume that system is using a static
ostree-prepare-root as init process. Booting without initramfs
is a common use case on embedded systems. This approach is
also more convenient, than having to patch the kernel.
@gatispaeglis gatispaeglis force-pushed the no-initramfs-bootloader-support branch from 3172d58 to 4fcdca7 Compare September 2, 2016 09:21
@gatispaeglis
Copy link
Contributor Author

FYI this is how I am using --karg-none : https://codereview.qt-project.org/#/c/174145/3/src/lib/qotaclientasync.cpp

@cgwalters
Copy link
Member

Is this still WIP, or is it ready for review?

@wmanley
Copy link
Member

wmanley commented Feb 17, 2017

@gatispaeglis: I was hoping to start using this soon. What's your feeling on it?

@gatispaeglis
Copy link
Contributor Author

@cgwalters, The more accurate status of this PR would be "it is up from grabs" :) The patch in general is in good shape. I have been using it for a while. There is one issue that I noticed, but haven't had time to fix. The issue is in deployment_bootconfigs_equal. It would need to special case the "init=" command line argument (as it is already doing for "ostree=" ). Otherwise the bootloader configs are unnecessary rewritten, even when "inti=" is the same.

The latest snapshot of the documentation how I used this is in http://doc-snapshots.qt.io/qtota/index.html

@wmanley The above should answer your question too.

My focus recently has changed to other projects, but I can still help with reviewing.

@cgwalters cgwalters removed the WIP label Feb 24, 2017
@cgwalters cgwalters self-assigned this Feb 24, 2017
@cgwalters cgwalters added this to the 2017.3 milestone Feb 24, 2017
@cgwalters
Copy link
Member

Ok cool, removing WIP. Will take a look soon.

@cgwalters
Copy link
Member

bot, retest this please

@rh-atomic-bot
Copy link

☔ The latest upstream changes (presumably 305db98) made this pull request unmergeable. Please resolve the merge conflicts.

@cgwalters
Copy link
Member

This was merged in #1401

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants