Bug 4061 - libbe should copy all entries in menu.lst matching a BE when creating a new BE based on it.
: libbe should copy all entries in menu.lst matching a BE when creating a new B...
Status: FIXUNDERSTOOD
Product: installer
library
: unspecified
: Other OpenSolaris
: P4 enhancement (vote)
: 2010.1H
Assigned To: evan.layton
:
:
: reviewed-2009.06
:
: 6518
:
  Show dependency treegraph
 
Reported: 2008-10-18 07:26 UTC by Tim Foster
Modified: 2009-04-28 11:08 UTC (History)
5 users (show)

See Also:


Attachments


Note

You need to log in before you can comment on or make changes to this bug.


Description Tim Foster 2008-10-18 07:26:21 UTC
When porting the fix for:

3654 libbe should keep menu.lst customizations when cloning

to xVM Server, we noticed a small problem.

When the system is creating a new boot environment (via beadm or
pkg image-update), it uses the bootfs entry in the menu.lst file
as the key to determine which grub entry to copy.

There may be multiple grub entries for a given dataset that have the same
bootfs though (failsafe, ttya, ttyb, etc.) but the code only clones the first
one it finds, regardless of which you're currently booted from.

So, if we have:

default 1
title Bar
bootfs rpool/ROOT/opensolaris
kernel .... 
module ....

title Bar ttya
bootfs rpool/ROOT/opensolaris
kernel .... -B console=ttya
module ....

title Bar ttyb
etc.

Ideally the system would clone the ttyb entry, but instead, cloning a new BE
will always give us a clone of the first entry - the user needs to fix their
menu.lst to add the ttya option to the newly cloned entry.

This RFE requests the code to first check to see if the dataset we're booted
from happens to match the dataset pointed to by the "default" grub keyword.
If they do match, then we should clone that dataset.  True, the user could have
chosen a different grub menu entry during bootup and we'd end up cloning
an incorrect BE entry anyway, having it always clone the active boot
environment
entry might make more sense.
Comment 1 Tim Foster 2008-10-18 08:32:31 UTC
sorry, typo in my description : "Ideally the system would clone the ttyb
entry", I meant to say "Ideally the system would clone the ttya entry" (it
being the one that the "default" property pointed to)
Comment 2 evan.layton 2008-10-19 12:44:11 UTC
We have no other choice but to use the bootfs entry as the key for searching
menu.lst. The title line is at best unreliable and none of the other lines in
an entry have anything at all to do with a BE. Based on this the original
summary and description are somewhat invalid since use of the bootfs line is
done by design and is all that we really have available to us. 

I believe that what is really being asked for here is that if there are
multiple entries for a BE that all of these are cloned when a new BE is
created. It was also stated that we could use the default entry to determine
which BE we're trying to clone however this won't work for all cases since we
may also be attempting to clone an inactive BE using the beadm -e option. What
may make the most sense here is for all of a BE's entries to be cloned based on
what is in the bootfs line. In other words we would clone each entry in
menu.lst which had a bootfs line matching the BE we're cloning/copying.
Comment 3 John Levon 2008-10-20 08:46:48 UTC
*** Bug 4096 has been marked as a duplicate of this bug. ***
Comment 4 evan.layton 2009-01-12 07:20:49 UTC
*** Bug 6069 has been marked as a duplicate of this bug. ***
Comment 5 evan.layton 2009-02-20 14:11:42 UTC
To fix this we simply need to make sure we continue looking for more entries
for this BE instead of quitting as soon as we hit the first one.

The fix might look similar to the following:

--- a/usr/src/lib/libbe/be_utils.c      Fri Feb 20 12:10:14 2009 -0700
+++ b/usr/src/lib/libbe/be_utils.c      Fri Feb 20 15:21:12 2009 -0700
@@ -373,6 +373,7 @@
        while (fgets(line, BUFSIZ, menu_fp)) {
                char *tok = NULL;

+title_line:
                (void) strlcpy(temp_line, line, BUFSIZ);
                tok = strtok(line, BE_WHITE_SPACE);

@@ -402,8 +403,7 @@
                                break;
                        }
                        if (be_orig_root_ds != NULL &&
-                           strcmp(bootfs, be_orig_root_ds) == 0 &&
-                           !found_orig_be) {
+                           strcmp(bootfs, be_orig_root_ds) == 0) {
                                char str[BUFSIZ];
                                found_orig_be = B_TRUE;
                                num_lines = 0;
@@ -439,14 +439,17 @@
                                while (fgets(line, BUFSIZ, menu_fp)) {
                                        if (strstr(line, BE_GRUB_COMMENT)
                                            != NULL || strstr(line, "BOOTADM")
-                                           != NULL || strncmp(line, "title",
5)
-                                           == 0)
+                                           != NULL)
                                                break;
+                                       if (strlen(line) == 0)
+                                               continue;
+                                       if (strncmp(line, "title", 5) == 0)
+                                               got title_line;
                                        entries[num_lines] = strdup(line);
                                        num_lines++;
                                }
                        }
-               } else if (found_title && !found_orig_be) {
+               } else if (found_title) {
                        tmp_entries[num_tmp_lines] = strdup(temp_line);
                        num_tmp_lines++;
                }
Comment 6 evan.layton 2009-04-07 14:31:48 UTC
Not a blocker bug