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

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

Closed
praveenkumar opened this issue Nov 8, 2016 · 23 comments
Closed

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

praveenkumar opened this issue Nov 8, 2016 · 23 comments

Comments

@praveenkumar
Copy link
Collaborator

@praveenkumar praveenkumar commented Nov 8, 2016

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
@praveenkumar praveenkumar changed the title Extract kernel options from the syslinux file if exist. Extract kernel options from the syslinux.conf file if exist. Nov 8, 2016
@zchee

This comment has been minimized.

Copy link
Member

@zchee zchee commented Nov 8, 2016

@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

This comment has been minimized.

Copy link
Collaborator Author

@praveenkumar praveenkumar commented Nov 8, 2016

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

This comment has been minimized.

Copy link
Member

@zchee zchee commented Nov 8, 2016

@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

This comment has been minimized.

Copy link
Collaborator Author

@praveenkumar praveenkumar commented Nov 8, 2016

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

yes, that's right.

@LalatenduMohanty

This comment has been minimized.

Copy link
Member

@LalatenduMohanty LalatenduMohanty commented Nov 8, 2016

@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

This comment has been minimized.

Copy link
Member

@LalatenduMohanty LalatenduMohanty commented Nov 8, 2016

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

@praveenkumar

This comment has been minimized.

Copy link
Collaborator Author

@praveenkumar praveenkumar commented Nov 8, 2016

@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

This comment has been minimized.

Copy link
Collaborator Author

@praveenkumar praveenkumar commented Nov 8, 2016

@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

This comment has been minimized.

Copy link
Member

@zchee zchee commented Nov 8, 2016

@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 moby/hyperkit#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

This comment has been minimized.

Copy link
Member

@zchee zchee commented Nov 8, 2016

@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

This comment has been minimized.

Copy link
Collaborator Author

@praveenkumar praveenkumar commented Nov 10, 2016

@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

This comment has been minimized.

Copy link
Collaborator Author

@praveenkumar praveenkumar commented Nov 11, 2016

@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

This comment has been minimized.

Copy link
Member

@zchee zchee commented Nov 12, 2016

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

@zchee

This comment has been minimized.

Copy link
Member

@zchee zchee commented Nov 12, 2016

@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

This comment has been minimized.

Copy link
Member

@zchee zchee commented Nov 12, 2016

@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

This comment has been minimized.

Copy link
Collaborator Author

@praveenkumar praveenkumar commented Nov 13, 2016

@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

This comment has been minimized.

Copy link
Member

@zchee zchee commented Nov 14, 2016

@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

This comment has been minimized.

Copy link
Collaborator Author

@praveenkumar praveenkumar commented Nov 15, 2016

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

This comment has been minimized.

Copy link
Member

@zchee zchee commented Nov 15, 2016

@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

This comment has been minimized.

Copy link
Collaborator Author

@praveenkumar praveenkumar commented Nov 15, 2016

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

This comment has been minimized.

Copy link
Member

@zchee zchee commented Nov 15, 2016

@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

This comment has been minimized.

Copy link
Collaborator Author

@praveenkumar praveenkumar commented Nov 15, 2016

Do you have any pull request for it?

I will create a PR soon, at max tomorrow.

@zchee

This comment has been minimized.

Copy link
Member

@zchee zchee commented Nov 15, 2016

@praveenkumar

I will create a PR soon, at max tomorrow.

Thanks :)

I am grateful for your cooperation. Thanks again.

praveenkumar added a commit to praveenkumar/docker-machine-driver-xhyve that referenced this issue Nov 15, 2016
praveenkumar added a commit to praveenkumar/docker-machine-driver-xhyve that referenced this issue Nov 18, 2016
praveenkumar added a commit to praveenkumar/docker-machine-driver-xhyve that referenced this issue Nov 18, 2016
@zchee zchee closed this in e51c8fc Nov 21, 2016
zchee added a commit that referenced this issue Nov 21, 2016
Fix #150 Extract kernel options from the syslinux.conf file if exist
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.