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 <jk@ozlabs.org>

diff --git a/discover/yaboot-parser.c b/discover/yaboot-parser.c
index f2f34c10b18e32636998dfa5e0980821e20736e6..81ab83d548b9e8e93979500a6bbce9bc792954f2 100644 (file)
--- a/discover/yaboot-parser.c
+++ b/discover/yaboot-parser.c
@@ -12,13 +12,15 @@
 #include "resource.h"
 
 struct yaboot_state {
 #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;
        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;
        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)
 {
                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->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");
        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;
 }
 
        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;
 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;
 
        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;
        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 */
        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);
        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",
        }
 
        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 : "");
 
                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);
 }
 
        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)
 {
 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);
 
                /* 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;
        }
 
                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 = 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 {
                } 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;
                }
 
                return;


@@ -232,16 +229,12 @@ static void yaboot_process_pair(struct conf_context *conf, const char *name,
        }
 
        /* initrd */
        }
 
        /* initrd */

-

        if (streq(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 */
                return;
        }
 
        /* label */

-

        if (streq(name, "label")) {
                opt->option->id = talloc_asprintf(opt->option, "%s#%s",
                        conf->dc->device->device->id, value);
        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 */
        }
 
        /* 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(
 
        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" },
 
 static struct conf_global_option yaboot_global_options[] = {
        { .name = "root" },

-       { .name = "boot" },
+       { .name = "device" },
+       { .name = "partition" },

        { .name = "initrd" },
        { .name = "initrd_size" },
        { .name = "initrd" },
        { .name = "initrd_size" },

-       { .name = "partition" },

        { .name = "video" },
        { .name = "literal" },
        { .name = "ramdisk" },
        { .name = "video" },
        { .name = "literal" },
        { .name = "ramdisk" },


@@ -329,6 +332,8 @@ static const char *yaboot_known_names[] = {
        "read-only",
        "read-write",
        "root",
        "read-only",
        "read-write",
        "root",

+       "device",
+       "partition",

        NULL
 };
 
        NULL
 };
 

diff --git a/test/parser/Makefile.am b/test/parser/Makefile.am
index 4319705c576163aa48da7fef0b9d18ac1f850a1c..3c6d73beac746eafd09d452ac2d5e344e94f69dc 100644 (file)
--- 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-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
        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 (file)
index 0000000..5db5788
--- /dev/null
+++ b/test/parser/test-yaboot-device-override.c
@@ -0,0 +1,79 @@
+#include "parser-test.h"
+
+#include <talloc/talloc.h>
+
+#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 97d0162697686976cff40ac8d288caf9669d21ac..8966cfd40c2a2942dec1cb5740fbd7017b140182 100644 (file)
--- 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_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");
 
        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);
 
 
        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");
                        "/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 "
                        "/boot/initrd-1.0-20121219-1.img");
 
        check_args(opt, "root=/dev/sdb2 root=/dev/sdb2 ro crashkernel=auto "