From cdaae762f23d137eef7de73f2226f55090ddbec0 Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Wed, 7 Aug 2013 14:00:16 +0800 Subject: [PATCH] discover/yaboot: fix device parsing A couple of fixes for yaboot's device-handling code. Firstly, we need to use 'device=' rather than 'root=', as the latter is purely for ybin, to define where the yaboot binary goes. Secondly, we need to respect global and option-specific device= parameters. To do this, we keep all boot_image and initrd strings in the state, and create the actual resources in yaboot_finish. Add a test for all override cases, and fix the incorrect boot= parsing in the rh8 test. Signed-off-by: Jeremy Kerr --- discover/yaboot-parser.c | 139 +++++++++++----------- test/parser/Makefile.am | 1 + test/parser/test-yaboot-device-override.c | 79 ++++++++++++ test/parser/test-yaboot-rh8-ppc64.c | 11 +- 4 files changed, 154 insertions(+), 76 deletions(-) create mode 100644 test/parser/test-yaboot-device-override.c diff --git a/discover/yaboot-parser.c b/discover/yaboot-parser.c index f2f34c1..81ab83d 100644 --- a/discover/yaboot-parser.c +++ b/discover/yaboot-parser.c @@ -12,13 +12,15 @@ #include "resource.h" struct yaboot_state { - char *desc_image; - char *desc_initrd; int globals_done; const char *const *known_names; /* current option data */ struct discover_boot_option *opt; + const char *device; + const char *partition; + const char *boot_image; + const char *initrd; const char *initrd_size; const char *literal; const char *ramdisk; @@ -31,12 +33,12 @@ static struct discover_boot_option *state_start_new_option( struct conf_context *conf, struct yaboot_state *state) { - state->desc_initrd = NULL; - state->opt = discover_boot_option_create(conf->dc, conf->dc->device); state->opt->option->boot_args = talloc_strdup(state->opt->option, ""); /* old allocated values will get freed with the state */ + state->device = conf_get_global_option(conf, "device"); + state->partition = conf_get_global_option(conf, "partition"); state->initrd_size = conf_get_global_option(conf, "initrd_size"); state->literal = conf_get_global_option(conf, "literal"); state->ramdisk = conf_get_global_option(conf, "ramdisk"); @@ -45,17 +47,48 @@ static struct discover_boot_option *state_start_new_option( return state->opt; } +static struct resource *create_yaboot_devpath_resource( + struct yaboot_state *state, + struct conf_context *conf, + const char *path) +{ + struct discover_boot_option *opt = state->opt; + const char *dev, *part; + struct resource *res; + char *devpath; + + dev = state->device; + part = state->partition; + + if (!dev) + dev = conf_get_global_option(conf, "device"); + if (!part) + part = conf_get_global_option(conf, "partition"); + + if (strchr(path, ':')) { + devpath = talloc_strdup(conf, path); + + } else if (dev && part) { + devpath = talloc_asprintf(conf, + "%s%s:%s", dev, part, path); + } else if (dev) { + devpath = talloc_asprintf(conf, "%s:%s", dev, path); + } else { + devpath = talloc_strdup(conf, path); + } + + res = create_devpath_resource(opt, conf->dc->device, devpath); + + talloc_free(devpath); + + return res; +} + static void yaboot_finish(struct conf_context *conf) { struct yaboot_state *state = conf->parser_info; - struct device *dev = conf->dc->device->device; struct boot_option *opt; - if (!state->desc_image) { - pb_log("%s: %s: no image found\n", __func__, dev->id); - return; - } - assert(state->opt); opt = state->opt->option; @@ -64,6 +97,13 @@ static void yaboot_finish(struct conf_context *conf) assert(opt->boot_args); /* populate the boot option from state data */ + state->opt->boot_image = create_yaboot_devpath_resource(state, + conf, state->boot_image); + if (state->initrd) { + state->opt->initrd = create_yaboot_devpath_resource(state, + conf, state->initrd); + } + if (state->initrd_size) { opt->boot_args = talloc_asprintf(opt, "ramdisk_size=%s %s", state->initrd_size, opt->boot_args); @@ -96,48 +136,16 @@ static void yaboot_finish(struct conf_context *conf) } opt->description = talloc_asprintf(opt, "%s %s %s", - state->desc_image, - (state->desc_initrd ? state->desc_initrd : ""), + state->boot_image, + (state->initrd ? state->initrd : ""), opt->boot_args ? opt->boot_args : ""); - talloc_free(state->desc_initrd); - conf_strip_str(opt->boot_args); conf_strip_str(opt->description); discover_context_add_boot_option(conf->dc, state->opt); } -static struct resource *create_yaboot_devpath_resource( - struct discover_boot_option *opt, - struct conf_context *conf, - const char *path, char **desc_str) -{ - const char *g_boot = conf_get_global_option(conf, "boot"); - const char *g_part = conf_get_global_option(conf, "partition"); - struct resource *res; - char *devpath; - - if (g_boot && g_part) { - devpath = talloc_asprintf(conf, - "%s%s:%s", g_boot, g_part, path); - } else if (g_boot) { - devpath = talloc_asprintf(conf, "%s:%s", g_boot, path); - } else { - devpath = talloc_strdup(conf, path); - } - - res = create_devpath_resource(opt, conf->dc->device, devpath); - - if (desc_str) - *desc_str = devpath; - else - talloc_free(devpath); - - return res; -} - - static void yaboot_process_pair(struct conf_context *conf, const char *name, char *value) { @@ -181,8 +189,7 @@ static void yaboot_process_pair(struct conf_context *conf, const char *name, /* Then start the new image. */ opt = state_start_new_option(conf, state); - opt->boot_image = create_yaboot_devpath_resource(opt, - conf, value, &state->desc_image); + state->boot_image = talloc_strdup(state, value); return; } @@ -205,21 +212,11 @@ static void yaboot_process_pair(struct conf_context *conf, const char *name, opt = state_start_new_option(conf, state); if (*value == '/') { - opt->boot_image = create_yaboot_devpath_resource(opt, - conf, value, &state->desc_image); + state->boot_image = talloc_strdup(state, value); } else { - char *tmp; - - opt->boot_image = create_yaboot_devpath_resource(opt, - conf, suse_fp->image, - &state->desc_image); - - opt->initrd = create_yaboot_devpath_resource(opt, - conf, suse_fp->initrd, &tmp); - - state->desc_initrd = talloc_asprintf(opt, - "initrd=%s", tmp); - talloc_free(tmp); + state->boot_image = talloc_strdup(state, + suse_fp->image); + state->initrd = talloc_strdup(state, suse_fp->initrd); } return; @@ -232,16 +229,12 @@ static void yaboot_process_pair(struct conf_context *conf, const char *name, } /* initrd */ - if (streq(name, "initrd")) { - opt->initrd = create_yaboot_devpath_resource(opt, conf, - value, &state->desc_image); - + state->initrd = talloc_strdup(state, value); return; } /* label */ - if (streq(name, "label")) { opt->option->id = talloc_asprintf(opt->option, "%s#%s", conf->dc->device->device->id, value); @@ -250,6 +243,16 @@ static void yaboot_process_pair(struct conf_context *conf, const char *name, } /* args */ + if (streq(name, "device")) { + printf("option device : %s", value); + state->device = talloc_strdup(state, value); + return; + } + + if (streq(name, "parititon")) { + state->partition = talloc_strdup(state, value); + return; + } if (streq(name, "append")) { opt->option->boot_args = talloc_asprintf_append( @@ -292,10 +295,10 @@ static void yaboot_process_pair(struct conf_context *conf, const char *name, static struct conf_global_option yaboot_global_options[] = { { .name = "root" }, - { .name = "boot" }, + { .name = "device" }, + { .name = "partition" }, { .name = "initrd" }, { .name = "initrd_size" }, - { .name = "partition" }, { .name = "video" }, { .name = "literal" }, { .name = "ramdisk" }, @@ -329,6 +332,8 @@ static const char *yaboot_known_names[] = { "read-only", "read-write", "root", + "device", + "partition", NULL }; diff --git a/test/parser/Makefile.am b/test/parser/Makefile.am index 4319705..3c6d73b 100644 --- a/test/parser/Makefile.am +++ b/test/parser/Makefile.am @@ -36,6 +36,7 @@ TESTS = \ test-yaboot-external \ test-yaboot-root-global \ test-yaboot-root-override \ + test-yaboot-device-override \ test-yaboot-rh8-ppc64 \ test-pxe-single \ test-pxe-initrd-in-append diff --git a/test/parser/test-yaboot-device-override.c b/test/parser/test-yaboot-device-override.c new file mode 100644 index 0000000..5db5788 --- /dev/null +++ b/test/parser/test-yaboot-device-override.c @@ -0,0 +1,79 @@ +#include "parser-test.h" + +#include + +#if 0 /* PARSER_EMBEDDED_CONFIG */ +default= +device=/dev/sda1 + +image=/vmlinux.1 + label=linux.1 + initrd=initrd.1 + +image=/vmlinux.2 + device=/dev/sda2 + label=linux.2 + initrd=initrd.2 + +image=sda3:/vmlinux.3 + device=/dev/sda2 + label=linux.3 + initrd=sda3:initrd.3 + +image=sda4:/vmlinux.4 + device=/dev/sda3 + label=linux.4 + initrd=initrd.4 +#endif + +void run_test(struct parser_test *test) +{ + struct discover_boot_option *opt[4]; + struct discover_device *dev[4]; + struct discover_context *ctx; + char *devname; + int i; + + test_read_conf_embedded(test); + test_run_parser(test, "yaboot"); + + ctx = test->ctx; + + check_boot_option_count(ctx, 4); + + for (i = 0; i < 4; i++) + opt[i] = get_boot_option(ctx, i); + + check_name(opt[0], "linux.1"); + check_unresolved_resource(opt[0]->boot_image); + check_unresolved_resource(opt[0]->initrd); + + check_name(opt[1], "linux.2"); + check_unresolved_resource(opt[1]->boot_image); + check_unresolved_resource(opt[1]->initrd); + + check_name(opt[2], "linux.3"); + check_unresolved_resource(opt[2]->boot_image); + check_unresolved_resource(opt[2]->initrd); + + check_name(opt[3], "linux.4"); + check_unresolved_resource(opt[3]->boot_image); + check_unresolved_resource(opt[3]->initrd); + + /* hotplug all dependent devices */ + for (i = 0; i < 4; i++) { + devname = talloc_asprintf(test, "sda%d", i + 1); + dev[i] = test_create_device(ctx, devname); + test_hotplug_device(test, dev[i]); + } + + check_resolved_local_resource(opt[0]->boot_image, dev[0], "/vmlinux.1"); + check_resolved_local_resource(opt[1]->boot_image, dev[1], "/vmlinux.2"); + check_resolved_local_resource(opt[2]->boot_image, dev[2], "/vmlinux.3"); + check_resolved_local_resource(opt[3]->boot_image, dev[3], "/vmlinux.4"); + + check_resolved_local_resource(opt[0]->initrd, dev[0], "/initrd.1"); + check_resolved_local_resource(opt[1]->initrd, dev[1], "/initrd.2"); + check_resolved_local_resource(opt[2]->initrd, dev[2], "/initrd.3"); + check_resolved_local_resource(opt[3]->initrd, dev[2], "/initrd.4"); +} diff --git a/test/parser/test-yaboot-rh8-ppc64.c b/test/parser/test-yaboot-rh8-ppc64.c index 97d0162..8966cfd 100644 --- a/test/parser/test-yaboot-rh8-ppc64.c +++ b/test/parser/test-yaboot-rh8-ppc64.c @@ -5,7 +5,6 @@ void run_test(struct parser_test *test) { struct discover_boot_option *opt; struct discover_context *ctx; - struct discover_device *dev; test_read_conf_file(test, "yaboot-rh8-ppc64.conf"); test_run_parser(test, "yaboot"); @@ -16,15 +15,9 @@ void run_test(struct parser_test *test) opt = get_boot_option(ctx, 0); - check_unresolved_resource(opt->boot_image); - check_unresolved_resource(opt->initrd); - - dev = test_create_device(ctx, "sdb1"); - test_hotplug_device(test, dev); - - check_resolved_local_resource(opt->boot_image, dev, + check_resolved_local_resource(opt->boot_image, test->ctx->device, "/boot/vmlinuz-1.0-20121219-1"); - check_resolved_local_resource(opt->initrd, dev, + check_resolved_local_resource(opt->initrd, test->ctx->device, "/boot/initrd-1.0-20121219-1.img"); check_args(opt, "root=/dev/sdb2 root=/dev/sdb2 ro crashkernel=auto " -- 2.39.2