"Daemon" mode #221

dlespiau · 2016-10-18T14:07:21Z

It'd be nice if we could use hyperstart as a regular daemon instead of just an init system. This allows us to:

Customise the OS we run inside the VM. There are configurations where we want a fuller OS than just an initrd+hyperstart. In those cases, hyperstart becomes a daemon started by systemd.
Something else that could be nice is to have hyperstart started by nsenter or systemd-nspawn so we can debug it on the host side (gdb, strace, ...). I have something like this sort of working and this series is the start of it.

laijs · 2016-10-19T01:52:49Z

src/init.c

+			 NULL) == -1) {
 		perror("mount devpts failed");
 		return -1;
 	}


I think we could move the above code and some init-related code into a function.
and the new function should be called only when argv[0] == "init".

I still consider moving the above code and some init-related code into a function better than introducing system_mount()? @dlespiau what do you think? it might be better in looking.

Oh, sure, I'll have a look.

Currently it's only the system mounts that are causing problems because they are actually trying to remount the already mounted file systems, so might just pull those in a separate function.

the commit for Regrouped the early setup code (system mounts, becoming session leader and setting up a controlling terminal in a single function that is skipped when not running as "init". is missing

laijs · 2016-10-19T01:56:47Z

src/Makefile.am

@@ -1,3 +1,12 @@
 AM_CFLAGS = -Wall -Werror
-bin_PROGRAMS=init
-init_SOURCES=init.c jsmn.c net.c util.c parse.c parson.c container.c exec.c event.c portmapping.c


we can compile it to src/hyperstart only.
and copy src/hyperstart to rootfs/init in the make-initrd.sh

Ooops, pushed the wrong branch, the new update should have the correct commmits.

laijs · 2016-10-19T02:08:09Z

trivis error:

checking for SYSTEMD... no
configure: error: Package requirements (systemd) were not met:
No package 'systemd' found
Consider adjusting the PKG_CONFIG_PATH environment variable if you
installed software in a non-standard prefix.
Alternatively, you may set the environment variables SYSTEMD_CFLAGS
and SYSTEMD_LIBS to avoid the need to call pkg-config.
See the pkg-config man page for more details.

dlespiau · 2016-10-19T12:46:27Z

PR updated:

Changed the binary name to "hyperstart" instead of "init" and change its name
when being copied to the initrd image.
hyperstart now knows if it has been started as an init system or a regular
daemon based on argv[0]
Added a trivial patch to ignore config.h.in~ I had lying around
Fixed the case where we don't have the systemd .pc file
Made sure we don't introduce a build-time dependency on pkg-config

dlespiau · 2016-10-19T16:55:47Z

PR updated:

Remove read_cmdline() that wasn't doing anything.
Regrouped the early setup code (system mounts, becoming session leader and
setting up a controlling terminal in a single function that is skipped when
not running as "init".

laijs · 2016-10-20T13:30:31Z

src/util.c

 #include "container.h"
 #include "../config.h"

-char *read_cmdline(void)


I'm not sure we should remove it, we may pass the debug level from it in future.
#170

What the function does is "return NULL", I'm not really removing anything useful that would need to be rewritten :)

dlespiau · 2016-10-24T10:14:58Z

PR updated:

dropped the commit removing read_cmdline()

dlespiau · 2016-10-27T13:10:08Z

Is there anything else blocking this PR? Thanks!

dlespiau · 2016-11-09T15:35:24Z

ping! :)

devimc · 2016-11-10T15:45:16Z

👍

dlespiau · 2016-12-01T12:13:09Z

Hi,

Can I do anything to help this PR getting merged? Or maybe you do not want to support hyperstart as a regular daemon?

Thanks,

laijs · 2016-12-02T02:08:50Z

src/init.c

 {
-	char *cmdline, *ctl_serial, *tty_serial;
+	if (!is_init)
+		return 0;


cloud you move the test up (outside this hyper_setup_init_process())?

and it seams better to remove this global is_init since there is no other usage.

laijs · 2016-12-02T02:10:36Z

src/init.c

+		return -1;
+	}
+
+	cmdline = read_cmdline();


leave this in the hyper_setup_init_process()

gnawux · 2016-12-02T02:22:47Z

@dlespiau Sorry for the delay, we are willing to enable the daemon mode, and we are working on get this to be merged soon. BTW, could you share some tests/comparison of the daemon mode?

Jimmy-Xu · 2016-12-15T02:54:03Z

Can one of the admins verify this patch?

Jimmy-Xu · 2016-12-15T02:54:04Z

Can one of the admins verify this patch?

laijs · 2016-12-15T03:08:00Z

hello @dlespiau, could you update the pr please?

laijs · 2016-12-22T12:47:12Z

hyperstart uses devtmpfs for containers' /dev, but devtmpfs is the singleton in the whole system,
it is shared with all the containers and the rest of the system. it is Ok when hyperstart is the init process. is it Ok when hyperstart is daemon mode? if not we should convert it to tmpfs as the same as libcontainer.

I have one of the autoconf generated files not being ignored, config.h.in~, it's usual to igore /config.* in one go. Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

We'd like to be able to run hyperstart as a regular daemon as well as the init system and have small behavioural differences between the two. Start by renaming the binary to "hyperstart", it will be copied as "init" when preparing the initrd. Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

This commit adds a systemd service file to start hyperstart when configured in daemon mode. v2: Fix the case where we don't have have the systemd pkgconfig file (Damien) Signed-off-by: Jose Carlos Venegas Munoz <jose.carlos.venegas.munoz@intel.com>

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

We base this detection on the name of the binary: "init" means we've been started as an init system. When not running as an init system, we can skip the basic initialisation as systemd or any other init system would have done that for us. Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

dlespiau · 2017-02-10T12:58:15Z

Updated PR:

Rebased onto master (needed to fix a couple of conflicts)
moved is_init inside main()

dlespiau · 2017-02-10T13:02:06Z

@laijs we haven't seen any problems with the containers' /dev so far.

dlespiau mentioned this pull request Oct 18, 2016

Daemon support for hyperstart intel/cc-oci-runtime#257

Closed

laijs reviewed Oct 19, 2016

View reviewed changes

dlespiau force-pushed the 20161011-daemon branch from 8a8f9a6 to 12bf280 Compare October 19, 2016 11:23

dlespiau force-pushed the 20161011-daemon branch from 12bf280 to 8f3c07b Compare October 19, 2016 16:42

laijs reviewed Oct 20, 2016

View reviewed changes

dlespiau force-pushed the 20161011-daemon branch from 8f3c07b to 12bf280 Compare October 24, 2016 10:13

dlespiau force-pushed the 20161011-daemon branch from 12bf280 to ba71111 Compare October 25, 2016 06:01

laijs reviewed Dec 2, 2016

View reviewed changes

src/init.c

return -1;

}

cmdline = read_cmdline();

Copy link

Contributor

laijs Dec 2, 2016

Choose a reason for hiding this comment

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

leave this in the hyper_setup_init_process()

Damien Lespiau and others added 5 commits February 10, 2017 12:27

gitignore: Ignore /config.*

f67d1df

I have one of the autoconf generated files not being ignored, config.h.in~, it's usual to igore /config.* in one go. Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

init: regroup init process setup code in a single function

bfc5002

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

dlespiau force-pushed the 20161011-daemon branch from ba71111 to 0611644 Compare February 10, 2017 12:45

laijs merged commit a9aa2f7 into hyperhq:master Feb 10, 2017

"Daemon" mode #221

"Daemon" mode #221

Uh oh!

Conversation

dlespiau commented Oct 18, 2016

Uh oh!

laijs Oct 19, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laijs commented Oct 19, 2016

Uh oh!

dlespiau commented Oct 19, 2016

Uh oh!

dlespiau commented Oct 19, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dlespiau commented Oct 24, 2016

Uh oh!

dlespiau commented Oct 27, 2016

Uh oh!

dlespiau commented Nov 9, 2016

Uh oh!

devimc commented Nov 10, 2016

Uh oh!

dlespiau commented Dec 1, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gnawux commented Dec 2, 2016

Uh oh!

Jimmy-Xu commented Dec 15, 2016

Uh oh!

Jimmy-Xu commented Dec 15, 2016

Uh oh!

laijs commented Dec 15, 2016

Uh oh!

laijs commented Dec 22, 2016

Uh oh!

dlespiau commented Feb 10, 2017

Uh oh!

dlespiau commented Feb 10, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

laijs Oct 19, 2016 •

edited

Loading