Extract kernel options from the syslinux.conf file if exist.

praveenkumar commented 7 years ago

Most Linux distro contain kernel options in the syslinux.cfg file which can be directly used if not provided by user and distro contain it. Implementing this approach will help get rid of hard-coded kernel options and also provide debug message to user if it unable to get.

syslinux.conf file which exist in boot2docker

$ cat /Volumes/b2d-v1.12.2/boot/isolinux/isolinux.cfg 
serial 0 9600
display boot.msg
default boot2docker
label boot2docker
    kernel /boot/vmlinuz64 com1=9600,8n1
    initrd /boot/initrd.img
    append loglevel=3 user=docker console=ttyS0 console=tty0 noembed nomodeset norestore waitusb=10:LABEL=boot2docker-data base

# see http://www.syslinux.org/wiki/index.php/SYSLINUX

# If flag_val is 0, do not load a kernel image unless it has been explicitly named in a LABEL statement. The default is 1.
implicit 0

# If flag_val is 0, display the boot: prompt only if the Shift or Alt key is pressed, or Caps Lock or Scroll lock is set (this is the default). If flag_val is 1, always display the boot: prompt.
prompt 1

# Indicates how long to pause at the boot: prompt until booting automatically, in units of 1/10 s. The timeout is cancelled when any key is pressed, the assumption being the user will complete the command line. A timeout of zero will disable the timeout completely. The default is 0.
timeout 1

# Displays the indicated file on the screen when a function key is pressed at the boot: prompt. This can be used to implement pre-boot online help (presumably for the kernel command line options).
F1 boot.msg
F2 f2
F3 f3
F4 f4

zchee commented 7 years ago

@praveenkumar Originally this was exclusively for boot2docker. But Ideally, we should parse the file, and change the "generic" driver from boot2docker only. SGTM. Do you have any proof-of-concept pull request?

praveenkumar commented 7 years ago

change the "generic" driver from boot2docker only.

Yes agree.

Do you have any proof-of-concept pull request?

@zchee Right now no but I will work on that and soon send a PR for review.

zchee commented 7 years ago

@praveenkumar

Right now no but I will work on that and soon send a PR for review.

Thanks :) minikube-iso also have append on isolinux.cfg, This feature seems to be compatible with minikube.

praveenkumar commented 7 years ago

minikube-iso also have append on isolinux.cfg, This feature seems to be compatible with minikube.

yes, that's right.

LalatenduMohanty commented 7 years ago

@praveenkumar just for the record , you are saying Arch, Ubuntu, Debian , CentOS uses syslinux.cfg, right? ISOLinux and SysLinux are related and ISOLinux is part of SysLinux as per my understanding.

LalatenduMohanty commented 7 years ago

Let me re-phrase, So it has to to be either syslinux.cfg or isolinux.cfg , right?

praveenkumar commented 7 years ago

@LalatenduMohanty I am just going to locate isolinux.cfg file and then parse it to look for append and then use that value if it failed to get it then should have err and notify user to pass the kernel option with --xhyve-kernel-cmd.

praveenkumar commented 7 years ago

@zchee Looks like xhyve doesn't make out from all the options passed to it and it not even able to start the vm. I tried default kernel options which available for centos image as default => root=live:CDLABEL=live-centos rootfstype=auto ro rd.live.image quiet no_timer_check console=tty0 console=ttyS0,115200 net.ifnames=0 biosdevname=0 rhgb rd.luks=0 rd.md=0 rd.dm=0 and it failed to start the VM. Do you have any idea what all kernel options xhyve can support?

zchee commented 7 years ago

@praveenkumar Ah, yeah. In short reply(will comment more detailed), xhyve(but now, We used hyperkit) seems to do not support the full kernel option, such as mem.(IIRC, I was found when https://github.com/docker/hyperkit/pull/66)

See docker/hyperkit/src/lib/firmware/kexec.c and several bootloader diffs. For example, I was referenced qemu/qemu/hw/i386/pc.c for fix kexec.c.

Now I do not yet understand all kernel option, but If you saw strange(ignored) flag behavior, we need fix the hyperkit internal. Or alternatively, use bootrom boot. It was implemented by docker team, but it may more support some flags than kexec boot.

zchee commented 7 years ago

@praveenkumar FYI, We used static vendoring hyperkit source on libhyperkit. So if you want to support some kernel option, It is possible to merge before hyperkit team code review.

praveenkumar commented 7 years ago

@zchee Looks like it will take more time to add required options support to xhyve and I am not very good in internal system implementation :( Meanwhile is it feasible to implement for boot2docker kind of iso which have supported kernel-options for xhyve driver?

praveenkumar commented 7 years ago

@zchee I was able to do some implementation but again that only going to work with boot2docker because we don't know other distro enabled same kernel options.

# cat options_test.go
package main

import (
    "bufio"
    "fmt"
    "os"
    "regexp"
    "strings"
)

func readLine(path string) []string {
    lines := []string{}
    inFile, err := os.Open(path)
    if err != nil {
        fmt.Printf("Issue with file open")
    }
    defer inFile.Close()
    scanner := bufio.NewScanner(inFile)
    scanner.Split(bufio.ScanLines)
    for scanner.Scan() {
        lines = append(lines, scanner.Text())
    }
    return lines
}

func main() {
    lines := readLine("/tmp/isolinux.cfg")
    kernelOptionRegexp := regexp.MustCompile(`\s*append`)
    for _, line := range lines {
        if kernelOptionRegexp.MatchString(line) {
            options := strings.Join(strings.Fields(line)[1:], " ")
            fmt.Printf("%#v", options)
        }
    }
}

# go run options_test.go
"loglevel=3 user=docker console=ttyS0 console=tty0 noembed nomodeset norestore waitusb=10:LABEL=boot2docker-data base"

zchee commented 7 years ago

@praveenkumar Sorry for the delay. I'll comment later.

zchee commented 7 years ago

@praveenkumar I have tested your example .go file. but, If target isolinux.cfg have many append option, will gets multiple option such as

"initrd=initrd.img inst.stage2=hd:LABEL=CentOS\\x207\\x20x86_64 quiet""initrd=initrd.img inst.stage2=hd:LABEL=CentOS\\x207\\x20x86_64 rd.live.check quiet""initrd=initrd.img inst.stage2=hd:LABEL=CentOS\\x207\\x20x86_64 xdriver=vesa nomodeset quiet""initrd=initrd.img inst.stage2=hd:LABEL=CentOS\\x207\\x20x86_64 rescue quiet"

using http://mirror.centos.org/centos/7/os/x86_64/isolinux/isolinux.cfg (Note that I do not know whether it is actually used or not)

What do you think about it?

Edit: I don't isolinux file syntax, Is that situation impossible(not occur)?

zchee commented 7 years ago

@praveenkumar Anyway, boot2docker and minikube only have one append option. As a result, this feature will for power users who want to use alternative iso.

So, We can assume the users already known isolinux syntax and content. I think e.g.

--xhyve-use-isolinux-kernel-option
- bool
- or, change to the use default? If so, disable if set --xhyve-boot-cmd
--xhyve-isolinux-label
- string
- optional flag for --xhyve-use-isolinux-kernel-option
- By deafult, using first founded append option
- If might be have multiple append, sets the label name to this flag

WDYT?

praveenkumar commented 7 years ago

@zchee Actually there will be only single append of kernel options in case of centos and others append options for memtest, rescue mode etc. what we actually need is only first append options which labeled as linux. We have to add break logic once first append find.

--xhyve-boot-cmd and --xhyve-isolinux-label should be same. I mean we don't actually require multiple append and only to pass kernel options.

My suggestion is only have --xhyve-isolinux-label options and don't put hard coded defaults if we can detect otherwise put debug message to use to pass using this options.

WDYT?

zchee commented 7 years ago

@praveenkumar

Actually there will be only single append of kernel options in case of centos and others append options for memtest, rescue mode etc. what we actually need is only first append options which labeled as linux. We have to add break logic once first append find.

agree.

--xhyve-boot-cmd and --xhyve-isolinux-label should be same. I mean we don't actually require multiple append and only to pass kernel options. My suggestion is only have --xhyve-isolinux-label options and don't put hard coded defaults if we can detect otherwise put debug message to use to pass using this options.

Ah, sorry if my understanding is wrong, but it means like this?

Add automatically parses isolinux.cfg logic
Add --xhyve-isolinux-label: string
- for parses and break conditions if have multiple label on isolinux.cfg
Remove hardcoded boot2docker kernel option
Remove --xhyve-boot-cmd
- Instead of use isolinux.cfg data

Or not remove --xhyve-boot-cmd, change to for additional debug option such as debug, bootmem_debug or ignore_loglevel, use isolinux.cfg option even if set --xhyve-boot-cmd ?

Edit: like:

op := d.parseIsoLinuxConfig(label)
if d.BootCmd != "" {
    op = append(op, d.BootCmd) // append "debug", "bootmem_debug", "ignore_loglevel" if set
}

praveenkumar commented 7 years ago

Add --xhyve-isolinux-label: string

you mean like have default value for this option will be append and user can define which option to parse?

Or not remove --xhyve-boot-cmd, change to for additional debug option such as debug, bootmem_debug or ignore_loglevel, use isolinux.cfg option even if set --xhyve-boot-cmd ?

That sound good, actually --xhyve-boot-cmd should be available to pass different kernel options. I am in support to first check isolinux.cfg options and if nothing found then tell user to use --xhyve-boot-cmd argument to pass kernel option which will make sure we are not iterating isolinux.cfg any more and respect users provided kernel parameters.

zchee commented 7 years ago

@praveenkumar

Add --xhyve-isolinux-label: string

you mean like have default value for this option will be append and user can define which option to parse?

Ah. In generally, does isolinux.cfg have a default section? (I learned it now). At the first, I still don't understand all of isolinux.cfg file syntax.

However, if use this, http://mirror.centos.org/centos/7/os/x86_64/isolinux/isolinux.cfg We should parse the append kernel option in label linux section, but there are default vesamenu.c32 in the first line. In this case,

parse default -> search label -> get append

is impossible. Another approach as a fallback,

parse default -> search label -> not found -> use the first found label and append

will use the linux label. But if default boot label is not at the top, such as sample isolinux.cfg, I think --xhyve-isolinux-label flags is necessary. If the default boot label is always at the top(by the isolinux.cfg specification), --xhyve-isolinux-label flag is not necessary.

Edit: Also, if the default boot label is determined for each distribution, it is not necessary.

That sound good, actually --xhyve-boot-cmd should be available to pass different kernel options. I am in support to first check isolinux.cfg options and if nothing found then tell user to use --xhyve-boot-cmd argument to pass kernel option which will make sure we are not iterating isolinux.cfg any more and respect users provided kernel parameters.

I understand. SGTM too.

BTW, I do not know which iso you want to use. Could you tell me centos isolinux.cfg url or gist?

Thank you for being a long time for discussing.

praveenkumar commented 7 years ago

If the default boot label is always at the top, --xhyve-isolinux-label flag is not necessary.

Usually a bootable iso which uses isolinux can have different way to put those kernel options. like in ubuntu if you check then isolinux.cfg is looks like where it include menu.cfg which have further include section and finally isolinux/txt.cfg have details. What I propose is our parser would be as simple as possible which just parse one single file and if not able to get append then put debug message. I am not sure if we can make it so generic then works for each distro.

$ cat /Volumes/Ubuntu\ 16.04.1\ L/isolinux/isolinux.cfg
# D-I config version 2.0
# search path for the c32 support libraries (libcom32, libutil etc.)
path 
include menu.cfg
default vesamenu.c32
prompt 0
timeout 50
ui gfxboot bootlogo

Could you tell me centos isolinux.cfg url or gist?

https://paste.fedoraproject.org/481342/91948141/

zchee commented 7 years ago

@praveenkumar

Usually a bootable iso which uses isolinux can have different way to put those kernel options. like in ubuntu if you check then isolinux.cfg is looks like where it include menu.cfg which have further include section and finally isolinux/txt.cfg have details. What I propose is our parser would be as simple as possible which just parse one single file and if not able to get append then put debug message. I am not sure if we can make it so generic then works for each distro.

Understand, Thanks. that's can include ...

What I propose is our parser would be as simple as possible which just parse one single file and if not able to get append then put debug message.

Sound good to me. It cannot be a complete parsing, but a simple is better.

And, I agree with first issue topic. We should use the isolinux.cfg kernel option if exists. Do you have any pull request for it?

praveenkumar commented 7 years ago

Do you have any pull request for it?

I will create a PR soon, at max tomorrow.

zchee commented 7 years ago

@praveenkumar

I will create a PR soon, at max tomorrow.

Thanks :)

I am grateful for your cooperation. Thanks again.

machine-drivers / docker-machine-driver-xhyve

Extract kernel options from the syslinux.conf file if exist. #150