The following patches give grubby the ability to manipulate multiboot-capable elilo.conf. Fedora''s elilo already contains the support, these patches simply teach grubby to take advantage of it. These patches are an enabler for using Xen on ia64 with Fedora. I have tested these changes with both grubby and elilo, using both multiboot and non-multiboot configurations. Regards, Aron
Aron Griffis
2006-Jun-07 21:36 UTC
[Fedora-xen] [patch 1/4] elilo multiboot support (grubby: new functions)
This patch adds some generic functions to make extending grubby a little easier, and makes changes throughout the code to take advantage of them. Specifically: - new functions for manipulating singleLine->element arrays: insertElement removeElement - new functions for matching keywords to types: getKeywordByType getTypeByKeyword - new functions for search an entry for lines matching a type: getLineByType getLineByType2 - new function for inserting a line to an entry from a template: addLineTmpl - simple debugging via dprintf, which is normally compiled out - repair one memcpy to use memmove instead, since it was unsafely working with overlapping regions Signed-off-by: Aron Griffis <aron@hp.com> grubby.c | 390 +++++++++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 258 insertions(+), 132 deletions(-) --- grubby.c.0 2006-06-07 15:11:48.000000000 -0400 +++ grubby.c 2006-06-07 15:31:45.000000000 -0400 @@ -31,6 +31,14 @@ #include "block.h" +#define DEBUG 0 + +#if DEBUG +#define dprintf(format, args...) printf(format , ## args) +#else +#define dprintf(format, args...) +#endif + #define _(A) (A) #define CODE_SEG_SIZE 128 /* code segment checked by --bootloader-probe */ @@ -77,7 +85,7 @@ char * key; enum lineType_e type; char nextChar; -} ; +}; struct configFileInfo { char * defaultConfig; @@ -258,7 +266,6 @@ struct configFileInfo * cfi; }; - struct singleEntry * findEntryByIndex(struct grubConfig * cfg, int index); struct singleEntry * findEntryByPath(struct grubConfig * cfg, const char * path, const char * prefix, @@ -271,6 +278,18 @@ static int getNextLine(char ** bufPtr, struct singleLine * line, struct configFileInfo * cfi); static char * getRootSpecifier(char * str); +static void insertElement(struct singleLine * line, + const char * item, int insertHere); +static void removeElement(struct singleLine * line, int removeHere); +static struct keywordTypes * getKeywordByType(enum lineType_e type, + struct configFileInfo * cfi); +static enum lineType_e getTypeByKeyword(char * keyword, + struct configFileInfo * cfi); +static struct singleLine * getLineByType(enum lineType_e type, + struct singleLine * line); +static struct singleLine * getLineByType2(enum lineType_e type1, + enum lineType_e type2, + struct singleLine * line); static char * sdupprintf(const char *format, ...) #ifdef __GNUC__ @@ -305,6 +324,54 @@ return buf; } +static struct keywordTypes * getKeywordByType(enum lineType_e type, + struct configFileInfo * cfi) { + struct keywordTypes * kw; + for (kw = cfi->keywords; kw->key; kw++) { + if (kw->type == type) + return kw; + } + return NULL; +} + +static enum lineType_e getTypeByKeyword(char * keyword, + struct configFileInfo * cfi) { + struct keywordTypes * kw; + for (kw = cfi->keywords; kw->key; kw++) { + if (!strcmp(keyword, kw->key)) + return kw->type; + } + return LT_UNKNOWN; +} + +/* These two (getLineByType and getLineByType2) cover the most common cases + * without going overboard... + */ +static struct singleLine * getLineByType(enum lineType_e type, + struct singleLine * line) { + dprintf("getLineByType(%d): ", type); + for (; line; line = line->next) { + dprintf("%d:%s ", line->type, + line->numElements ? line->elements[0].item : "(empty)"); + if (line->type == type) break; + } + dprintf(line ? "\n" : " (failed)\n"); + return line; +} + +static struct singleLine * getLineByType2(enum lineType_e type1, + enum lineType_e type2, + struct singleLine * line) { + dprintf("getLineByType(%d,%d): ", type1, type2); + for (; line; line = line->next) { + dprintf("%d:%s ", line->type, + line->numElements ? line->elements[0].item : "(empty)"); + if (line->type == type1 || line->type == type2) break; + } + dprintf(line ? "\n" : " (failed)\n"); + return line; +} + static int isBracketedTitle(struct singleLine * line) { if ((*line->elements[0].item == ''['') && (line->numElements == 1)) { int len = strlen(line->elements[0].item); @@ -318,19 +385,10 @@ return 0; } -/* figure out if this is a entry separator */ static int isEntrySeparator(struct singleLine * line, struct configFileInfo * cfi) { - if (line->type == LT_WHITESPACE) - return 0; - if (line->type == cfi->entrySeparator) - return 1; - if (line->type == LT_OTHER) - return 1; - if (cfi->titleBracketed && isBracketedTitle(line)) { - return 1; - } - return 0; + return line->type == cfi->entrySeparator || line->type == LT_OTHER || + (cfi->titleBracketed && isBracketedTitle(line)); } /* extract the title from within brackets (for zipl) */ @@ -425,9 +483,7 @@ char * chptr; int elementsAlloced = 0; struct lineElement * element; - struct keywordTypes * keywords = cfi->keywords; int first = 1; - int i; lineFree(line); @@ -481,14 +537,8 @@ if (!line->numElements) line->type = LT_WHITESPACE; else { - for (i = 0; keywords[i].key; i++) - if (!strcmp(line->elements[0].item, keywords[i].key)) break; - - if (keywords[i].key) { - line->type = keywords[i].type; - } else { - line->type = LT_UNKNOWN; - + line->type = getTypeByKeyword(line->elements[0].item, cfi); + if (line->type == LT_UNKNOWN) { /* zipl does [title] instead of something reasonable like all * the other boot loaders. kind of ugly */ if (cfi->titleBracketed && isBracketedTitle(line)) { @@ -644,8 +694,8 @@ int last, len; if (*line->elements[1].item == ''"'') - memcpy(line->elements[1].item, line->elements[1].item + 1, - strlen(line->elements[1].item + 1) + 1); + memmove(line->elements[1].item, line->elements[1].item + 1, + strlen(line->elements[1].item + 1) + 1); last = line->numElements - 1; len = strlen(line->elements[last].item) - 1; @@ -761,8 +811,7 @@ if (!entry) return; - line = entry->lines; - while (line && line->type != LT_TITLE) line = line->next; + line = getLineByType(LT_TITLE, entry->lines); if (line && line->numElements >= 2) fprintf(out, "%sdefault%s%s\n", indent, separator, @@ -913,12 +962,10 @@ char * dev; char * rootspec; - line = entry->lines; - while (line && line->type != LT_KERNEL) line = line->next; - - if (!line) return 0; if (skipRemoved && entry->skip) return 0; - if (line->numElements < 2) return 0; + + line = getLineByType(LT_KERNEL, entry->lines); + if (!line || line->numElements < 2) return 0; if (flags & GRUBBY_BADIMAGE_OKAY) return 1; @@ -926,8 +973,7 @@ strlen(line->elements[1].item) + 1); rootspec = getRootSpecifier(line->elements[1].item); sprintf(fullName, "%s%s", bootPrefix, - line->elements[1].item + ((rootspec != NULL) ? - strlen(rootspec) : 0)); + line->elements[1].item + (rootspec ? strlen(rootspec) : 0)); if (access(fullName, R_OK)) return 0; for (i = 2; i < line->numElements; i++) @@ -936,19 +982,15 @@ dev = line->elements[i].item + 5; } else { /* look for a lilo style LT_ROOT line */ - line = entry->lines; - while (line && line->type != LT_ROOT) line = line->next; + line = getLineByType(LT_ROOT, entry->lines); if (line && line->numElements >= 2) { dev = line->elements[1].item; } else { - int type; - /* didn''t succeed in finding a LT_ROOT, let''s try LT_KERNELARGS */ - line = entry->lines; - - type = ((entry->multiboot) ? LT_MBMODULE : LT_KERNELARGS); - - while (line && line->type != type) line = line->next; + /* didn''t succeed in finding a LT_ROOT, let''s try LT_KERNELARGS. + * grub+multiboot uses LT_MBMODULE for the args, so check that too. + */ + line = getLineByType2(LT_KERNELARGS, LT_MBMODULE, entry->lines); /* failed to find one */ if (!line) return 0; @@ -1019,10 +1061,7 @@ entry = findEntryByIndex(config, indexVars[i]); if (!entry) return NULL; - line = entry->lines; - while (line && line->type != LT_KERNEL) - line = line->next; - + line = getLineByType(LT_KERNEL, entry->lines); if (!line) return NULL; if (index) *index = indexVars[i]; @@ -1064,10 +1103,8 @@ kernel += 6; } - while ((entry = findEntryByIndex(config, i))) { - line = entry->lines; - while (line && line->type != checkType) line=line->next; - + for (entry = findEntryByIndex(config, i); entry; entry = entry->next, i++) { + line = getLineByType(checkType, entry->lines); if (line && line->numElements >= 2 && !entry->skip) { rootspec = getRootSpecifier(line->elements[1].item); @@ -1076,10 +1113,10 @@ kernel + strlen(prefix))) break; } - + /* have to check multiboot lines too */ if (entry->multiboot) { - while (line && line->type != LT_MBMODULE) line = line->next; + line = getLineByType(LT_MBMODULE, line); if (line && line->numElements >= 2 && !entry->skip) { rootspec = getRootSpecifier(line->elements[1].item); if (!strcmp(line->elements[1].item + @@ -1088,8 +1125,6 @@ break; } } - - i++; } if (index) *index = i; @@ -1099,12 +1134,11 @@ /* make sure this entry has a kernel identifier; this skips non-Linux boot entries (could find netbsd etc, though, which is unfortunate) */ - line = entry->lines; - while (line && line->type != LT_KERNEL) line = line->next; + line = getLineByType(LT_KERNEL, entry->lines); if (!line) { if (!index) index = &i; (*index)++; - return findEntryByPath(config, kernel, prefix, index); + return findEntryByPath(config, kernel, prefix, index); /* recurse */ } return entry; @@ -1271,11 +1305,9 @@ char * root = NULL; int i; - line = entry->lines; - while (line && line->type != LT_KERNEL) line = line->next; - printf("index=%d\n", index); + line = getLineByType(LT_KERNEL, entry->lines); printf("kernel=%s\n", line->elements[1].item); if (line->numElements >= 3) { @@ -1293,9 +1325,7 @@ } printf("\"\n"); } else { - line = entry->lines; - while (line && line->type != LT_KERNELARGS) line=line->next; - + line = getLineByType(LT_KERNELARGS, entry->lines); if (line) { char * s; @@ -1319,9 +1349,7 @@ } if (!root) { - line = entry->lines; - while (line && line->type != LT_ROOT) line = line->next; - + line = getLineByType(LT_ROOT, entry->lines); if (line && line->numElements >= 2) root=line->elements[1].item; } @@ -1336,8 +1364,7 @@ printf("root=%s\n", s); } - line = entry->lines; - while (line && line->type != LT_INITRD) line = line->next; + line = getLineByType(LT_INITRD, entry->lines); if (line && line->numElements >= 2) { printf("initrd=%s", prefix); @@ -1421,14 +1448,12 @@ if (config->cfi == &grubConfigType) { dumpSysconfigGrub(); } else { - line = config->theLines; - while (line && line->type != LT_BOOT) line = line->next; + line = getLineByType(LT_BOOT, config->theLines); if (line && line->numElements >= 1) { printf("boot=%s\n", line->elements[1].item); } - line = config->theLines; - while (line && line->type != LT_LBA) line = line->next; + line = getLineByType(LT_LBA, config->theLines); if (line) printf("lba\n"); } @@ -1443,77 +1468,112 @@ return 0; } +struct singleLine * addLineTmpl(struct singleEntry * entry, + struct singleLine * tmplLine, + struct singleLine * prevLine, + const char * val) { + int i; + struct singleLine * newLine = malloc(sizeof(*newLine)); + + newLine->indent = strdup(tmplLine->indent); + newLine->next = NULL; + newLine->type = tmplLine->type; + newLine->numElements = tmplLine->numElements; + newLine->elements = malloc(sizeof(*newLine->elements) * + newLine->numElements); + + for (i = 0; i < newLine->numElements; i++) { + newLine->elements[i].indent = strdup(tmplLine->elements[i].indent); + newLine->elements[i].item = strdup(tmplLine->elements[i].item); + } + + if (val) { + /* override the inherited value with our own. + * This is a little weak because it only applies to elements[1] + */ + if (newLine->numElements > 1) + removeElement(newLine, 1); + insertElement(newLine, val, 1); + + /* but try to keep the rootspec from the template... sigh */ + if (tmplLine->type == LT_KERNEL || + tmplLine->type == LT_MBMODULE || + tmplLine->type == LT_INITRD) + { + char * rootspec = getRootSpecifier(tmplLine->elements[1].item); + if (rootspec != NULL) { + free(newLine->elements[1].item); + newLine->elements[1].item = + sdupprintf("%s%s", rootspec, val); + } + } + } + + if (!entry->lines) { + /* first one on the list */ + entry->lines = newLine; + } else if (prevLine) { + /* add after prevLine */ + newLine->next = prevLine->next; + prevLine->next = newLine; + } + + return newLine; +} + /* val may be NULL */ struct singleLine * addLine(struct singleEntry * entry, struct configFileInfo * cfi, - enum lineType_e type, const char * defaultIndent, - char * val) { + enum lineType_e type, char * defaultIndent, + const char * val) { struct singleLine * line, * prev; - int i; + struct keywordTypes * kw; + struct singleLine tmpl; - for (i = 0; cfi->keywords[i].key; i++) - if (cfi->keywords[i].type == type) break; - if (type != LT_TITLE || !cfi->titleBracketed) - if (!cfi->keywords[i].key) abort(); + /* NB: This function shouldn''t allocation items on the heap, but rather on + * the stack since it calls addLineTmpl which will make copies. + */ + + if (type == LT_TITLE && cfi->titleBracketed) { + /* we''re doing a bracketed title (zipl) */ + tmpl.type = type; + tmpl.numElements = 1; + tmpl.elements = alloca(sizeof(*tmpl.elements)); + tmpl.elements[0].item = alloca(strlen(val)+3); + sprintf(tmpl.elements[0].item, "[%s]", val); + tmpl.elements[0].indent = ""; + } else { + kw = getKeywordByType(type, cfi); + if (!kw) abort(); + tmpl.type = type; + tmpl.numElements = val ? 2 : 1; + tmpl.elements = alloca(sizeof(*tmpl.elements) * tmpl.numElements); + tmpl.elements[0].item = kw->key; + tmpl.elements[0].indent = alloca(2); + sprintf(tmpl.elements[0].indent, "%c", kw->nextChar); + if (val) { + tmpl.elements[1].item = (char *)val; + tmpl.elements[1].indent = ""; + } + } /* The last non-empty line gives us the indention to us and the line to insert after. Note that comments are considered empty lines, which may not be ideal? If there are no lines or we are looking at the first line, we use defaultIndent (the first line is normally indented differently from the rest) */ - if (entry->lines) { - line = entry->lines; - prev = NULL; - while (line) { - if (line->numElements) prev = line; - line = line->next; - } - if (!prev) { - /* just use the last line */ - prev = entry->lines; - while (prev->next) prev = prev->next; - } - - line = prev->next; - prev->next = malloc(sizeof(*line)); - prev->next->next = line; - line = prev->next; - - if (prev == entry->lines) - line->indent = strdup(defaultIndent); - else - line->indent = strdup(prev->indent); - } else { - line = malloc(sizeof(*line)); - line->indent = strdup(defaultIndent); - line->next = NULL; + for (line = entry->lines, prev = NULL; line; line = line->next) { + if (line->numElements) prev = line; + /* fall back on the last line if prev isn''t otherwise set */ + if (!line->next && !prev) prev = line; } - if (type != LT_TITLE || !cfi->titleBracketed) { - line->type = type; - line->numElements = val ? 2 : 1; - line->elements = malloc(sizeof(*line->elements) * line->numElements); - line->elements[0].item = strdup(cfi->keywords[i].key); - line->elements[0].indent = malloc(2); - line->elements[0].indent[0] = cfi->keywords[i].nextChar; - line->elements[0].indent[1] = ''\0''; - - if (val) { - line->elements[1].item = val; - line->elements[1].indent = strdup(""); - } - } else { - /* we''re doing the title of a bracketed title (zipl) */ - line->type = type; - line->numElements = 1; - line->elements = malloc(sizeof(*line->elements) * line->numElements); - - line->elements[0].item = malloc(strlen(val) + 3); - sprintf(line->elements[0].item, "[%s]", val); - line->elements[0].indent = strdup(""); - } + if (prev == entry->lines) + tmpl.indent = defaultIndent ?: ""; + else + tmpl.indent = prev->indent; - return line; + return addLineTmpl(entry, &tmpl, prev, val); } void removeLine(struct singleEntry * entry, struct singleLine * line) { @@ -1538,6 +1598,73 @@ free(line); } +static void insertElement(struct singleLine * line, + const char * item, int insertHere) { + + /* sanity check */ + if (insertHere > line->numElements) { + dprintf("insertElement() adjusting insertHere from %d to %d\n", + insertHere, line->numElements); + insertHere = line->numElements; + } + + line->elements = realloc(line->elements, (line->numElements + 1) * + sizeof(*line->elements)); + memmove(&line->elements[insertHere+1], + &line->elements[insertHere], + (line->numElements - insertHere) * + sizeof(*line->elements)); + line->elements[insertHere].item = strdup(item); + + if (insertHere > 0 && line->elements[insertHere-1].indent[0] == ''\0'') { + /* move the end-of-line forward */ + line->elements[insertHere].indent = + line->elements[insertHere-1].indent; + line->elements[insertHere-1].indent = strdup(" "); + } else { + /* technically this should honor nextChar from keywordTypes + * when insertHere == 0, but oh well + */ + line->elements[insertHere].indent = + strdup(insertHere == line->numElements ? "" : " "); + } + + line->numElements++; + + dprintf("insertElement(%s, ''%s%s'', %d)\n", + line->elements[0].item, + line->elements[insertHere].item, + line->elements[insertHere].indent, + insertHere); +} + +static void removeElement(struct singleLine * line, int removeHere) { + int i; + + /* sanity check */ + if (removeHere >= line->numElements) return; + + dprintf("removeElement(%s, %d:%s)\n", line->elements[0].item, + removeHere, line->elements[removeHere].item); + + free(line->elements[removeHere].item); + + if (removeHere > 1) { + /* previous argument gets this argument''s post-indentation */ + free(line->elements[removeHere-1].indent); + line->elements[removeHere-1].indent + line->elements[removeHere].indent; + } else { + free(line->elements[removeHere].indent); + } + + /* now collapse the array, but don''t bother to realloc smaller */ + for (i = removeHere; i < line->numElements - 1; i++) + line->elements[i] = line->elements[i + 1]; + + line->numElements--; +} + int argMatch(const char * one, const char * two) { char * first, * second; char * chptr; @@ -2546,8 +2673,7 @@ if (!entry) return 0; if (!suitableImage(entry, bootPrefix, 0, flags)) return 0; - line = entry->lines; - while (line && line->type != LT_KERNEL) line = line->next; + line = getLineByType2(LT_KERNEL, LT_HYPER, entry->lines); if (!line) return 0; rootspec = getRootSpecifier(line->elements[1].item);
Aron Griffis
2006-Jun-07 21:37 UTC
[Fedora-xen] [patch 2/4] elilo multiboot support (grubby: add LT_HYPER)
This patch adds support for the LT_HYPER line type. Whereas grub uses a kernel=hypervisor module=kernel module=initrd, elilo uses vmm=hypervisor image=kernel initrd=initrd. Adding LT_HYPER support, and extending it backward to grub, makes dealing with this a lot easier. configFileInfo->mbHyperFirst is added to differentiate between grub wanting the hypervisor listed first, and elilo wanting the kernel listed first (image= is the entry separator so it''s very important) configFileInfo->mbInitRdIsModule is added to handle grub wanting the initrd listed as a module when multibooting The logic in addNewKernel is considerably rewritten (and cleaned up!) to be more generic and handle elilo alongside grub. Signed-off-by: Aron Griffis <aron@hp.com> grubby.c | 463 ++++++++++++++++++++++++++++++++++++++------------------------- 1 file changed, 285 insertions(+), 178 deletions(-) --- grubby.c.1 2006-06-07 15:31:45.000000000 -0400 +++ grubby.c 2006-06-07 16:14:33.000000000 -0400 @@ -49,9 +49,10 @@ char * indent; }; -enum lineType_e { LT_WHITESPACE, LT_TITLE, LT_KERNEL, LT_INITRD, LT_DEFAULT, - LT_UNKNOWN, LT_ROOT, LT_FALLBACK, LT_KERNELARGS, LT_BOOT, - LT_BOOTROOT, LT_LBA, LT_MBMODULE, LT_OTHER, LT_GENERIC }; +enum lineType_e { LT_WHITESPACE, LT_TITLE, LT_KERNEL, LT_INITRD, LT_HYPER, + LT_DEFAULT, LT_MBMODULE, LT_ROOT, LT_FALLBACK, LT_KERNELARGS, LT_BOOT, + LT_BOOTROOT, LT_LBA, LT_OTHER, LT_GENERIC, LT_UNKNOWN +}; struct singleLine { char * indent; @@ -72,11 +73,12 @@ #define GRUB_CONFIG_NO_DEFAULT (1 << 0) /* don''t write out default=0 */ -#define KERNEL_KERNEL (1 << 0) -#define KERNEL_INITRD (1 << 2) -#define KERNEL_TITLE (1 << 3) -#define KERNEL_ARGS (1 << 4) -#define KERNEL_MB (1 << 5) +/* These defines are (only) used in addNewKernel() */ +#define NEED_KERNEL (1 << 0) +#define NEED_INITRD (1 << 2) +#define NEED_TITLE (1 << 3) +#define NEED_ARGS (1 << 4) +#define NEED_MB (1 << 5) #define MAIN_DEFAULT (1 << 0) #define DEFAULT_SAVED -2 @@ -97,6 +99,8 @@ int argsInQuotes; int maxTitleLength; int titleBracketed; + int mbHyperFirst; + int mbInitRdIsModule; }; struct keywordTypes grubKeywords[] = { @@ -107,6 +111,7 @@ { "kernel", LT_KERNEL, '' '' }, { "initrd", LT_INITRD, '' '' }, { "module", LT_MBMODULE, '' '' }, + { "kernel", LT_HYPER, '' '' }, { NULL, 0, 0 }, }; @@ -120,6 +125,8 @@ 0, /* argsInQuotes */ 0, /* maxTitleLength */ 0, /* titleBracketed */ + 1, /* mbHyperFirst */ + 1, /* mbInitRdIsModule */ }; struct keywordTypes yabootKeywords[] = { @@ -173,6 +180,17 @@ { NULL, 0, 0 }, }; +struct keywordTypes eliloKeywords[] = { + { "label", LT_TITLE, ''='' }, + { "root", LT_ROOT, ''='' }, + { "default", LT_DEFAULT, ''='' }, + { "image", LT_KERNEL, ''='' }, + { "initrd", LT_INITRD, ''='' }, + { "append", LT_KERNELARGS, ''='' }, + { "vmm", LT_HYPER, ''='' }, + { NULL, 0, 0 }, +}; + struct keywordTypes siloKeywords[] = { { "label", LT_TITLE, ''='' }, { "root", LT_ROOT, ''='' }, @@ -196,7 +214,7 @@ struct configFileInfo eliloConfigType = { "/boot/efi/EFI/redhat/elilo.conf", /* defaultConfig */ - liloKeywords, /* keywords */ + eliloKeywords, /* keywords */ 0, /* defaultIsIndex */ 0, /* defaultSupportSaved */ LT_KERNEL, /* entrySeparator */ @@ -204,6 +222,8 @@ 1, /* argsInQuotes */ 0, /* maxTitleLength */ 0, /* titleBracketed */ + 0, /* mbHyperFirst */ + 0, /* mbInitRdIsModule */ }; struct configFileInfo liloConfigType = { @@ -216,6 +236,8 @@ 1, /* argsInQuotes */ 15, /* maxTitleLength */ 0, /* titleBracketed */ + 0, /* mbHyperFirst */ + 0, /* mbInitRdIsModule */ }; struct configFileInfo yabootConfigType = { @@ -228,6 +250,8 @@ 1, /* argsInQuotes */ 15, /* maxTitleLength */ 0, /* titleBracketed */ + 0, /* mbHyperFirst */ + 0, /* mbInitRdIsModule */ }; struct configFileInfo siloConfigType = { @@ -240,6 +264,8 @@ 1, /* argsInQuotes */ 15, /* maxTitleLength */ 0, /* titleBracketed */ + 0, /* mbHyperFirst */ + 0, /* mbInitRdIsModule */ }; struct configFileInfo ziplConfigType = { @@ -252,6 +278,8 @@ 1, /* argsInQuotes */ 15, /* maxTitleLength */ 1, /* titleBracketed */ + 0, /* mbHyperFirst */ + 0, /* mbInitRdIsModule */ }; struct grubConfig { @@ -656,11 +684,46 @@ if (line->type == LT_DEFAULT && line->numElements == 2) { cfg->flags &= ~GRUB_CONFIG_NO_DEFAULT; defaultLine = line; + + } else if (line->type == LT_KERNEL) { + /* if by some freak chance this is multiboot and the "module" + * lines came earlier in the template, make sure to use LT_HYPER + * instead of LT_KERNEL now + */ + if (entry->multiboot) { + struct singleLine * l; + for (l = entry->lines; l; l = l->next) { + if (l->type == LT_MBMODULE) { + line->type = LT_HYPER; /* caught it! */ + break; + } + } + } + } else if (line->type == LT_MBMODULE) { + /* go back and fix the LT_KERNEL line to indicate LT_HYPER + * instead, now that we know this is a multiboot entry. + * This only applies to grub, but that''s the only place we + * should find LT_MBMODULE lines anyway. + */ + struct singleLine * l; + for (l = entry->lines; l; l = l->next) { + if (l->type == LT_HYPER) + break; + else if (l->type == LT_KERNEL) { + l->type = LT_HYPER; + break; + } + } entry->multiboot = 1; + + } else if (line->type == LT_HYPER) { + entry->multiboot = 1; + } else if (line->type == LT_FALLBACK && line->numElements == 2) { cfg->fallbackImage = strtol(line->elements[1].item, &end, 10); if (*end) cfg->fallbackImage = -1; + } else if (line->type == LT_TITLE && line->numElements > 1) { /* make the title a single argument (undoing our parsing) */ len = 0; @@ -685,6 +748,7 @@ line->elements[line->numElements - 1].indent; line->elements[1].item = buf; line->numElements = 2; + } else if (line->type == LT_KERNELARGS && cfi->argsInQuotes) { /* Strip off any " which may be present; they''ll be put back on write. This is one of the few (the only?) places that grubby @@ -702,7 +766,6 @@ if (line->elements[last].item[len] == ''"'') line->elements[last].item[len] = ''\0''; } - } /* If we find a generic config option which should live at the @@ -964,7 +1027,7 @@ if (skipRemoved && entry->skip) return 0; - line = getLineByType(LT_KERNEL, entry->lines); + line = getLineByType2(LT_KERNEL, LT_HYPER, entry->lines); if (!line || line->numElements < 2) return 0; if (flags & GRUBBY_BADIMAGE_OKAY) return 1; @@ -1061,7 +1124,7 @@ entry = findEntryByIndex(config, indexVars[i]); if (!entry) return NULL; - line = getLineByType(LT_KERNEL, entry->lines); + line = getLineByType2(LT_KERNEL, LT_HYPER, entry->lines); if (!line) return NULL; if (index) *index = indexVars[i]; @@ -1104,7 +1167,10 @@ } for (entry = findEntryByIndex(config, i); entry; entry = entry->next, i++) { - line = getLineByType(checkType, entry->lines); + if (checkType == LT_KERNEL) + line = getLineByType2(LT_KERNEL, LT_HYPER, entry->lines); + else + line = getLineByType(checkType, entry->lines); if (line && line->numElements >= 2 && !entry->skip) { rootspec = getRootSpecifier(line->elements[1].item); @@ -1134,7 +1200,7 @@ /* make sure this entry has a kernel identifier; this skips non-Linux boot entries (could find netbsd etc, though, which is unfortunate) */ - line = getLineByType(LT_KERNEL, entry->lines); + line = getLineByType2(LT_KERNEL, LT_HYPER, entry->lines); if (!line) { if (!index) index = &i; (*index)++; @@ -1307,7 +1373,7 @@ printf("index=%d\n", index); - line = getLineByType(LT_KERNEL, entry->lines); + line = getLineByType2(LT_KERNEL, LT_HYPER, entry->lines); printf("kernel=%s\n", line->elements[1].item); if (line->numElements >= 3) { @@ -1496,7 +1562,8 @@ insertElement(newLine, val, 1); /* but try to keep the rootspec from the template... sigh */ - if (tmplLine->type == LT_KERNEL || + if (tmplLine->type == LT_HYPER || + tmplLine->type == LT_KERNEL || tmplLine->type == LT_MBMODULE || tmplLine->type == LT_INITRD) { @@ -2112,13 +2179,9 @@ char * newKernelArgs, char * newKernelInitrd, char * newMBKernel, char * newMBKernelArgs) { struct singleEntry * new; - struct singleLine * newLine = NULL, * tmplLine = NULL, * lastLine = NULL; + struct singleLine * newLine = NULL, * tmplLine = NULL; int needs; - char * indent = NULL; - char * rootspec = NULL; char * chptr; - int i; - enum lineType_e type; if (!newKernelPath) return 0; @@ -2148,66 +2211,113 @@ config->entries = new; /* copy/update from the template */ - needs = KERNEL_KERNEL | KERNEL_INITRD | KERNEL_TITLE; + needs = NEED_KERNEL | NEED_TITLE; + if (newKernelInitrd) + needs |= NEED_INITRD; if (newMBKernel) { - needs |= KERNEL_MB; + needs |= NEED_MB; new->multiboot = 1; } if (template) { for (tmplLine = template->lines; tmplLine; tmplLine = tmplLine->next) { - /* remember the indention level; we may need it for new lines */ - if (tmplLine->numElements) - indent = tmplLine->indent; - /* skip comments */ chptr = tmplLine->indent; while (*chptr && isspace(*chptr)) chptr++; if (*chptr == ''#'') continue; - /* we don''t need an initrd here */ - if (tmplLine->type == LT_INITRD && !newKernelInitrd) continue; - - if (tmplLine->type == LT_KERNEL && - !template->multiboot && (needs & KERNEL_MB)) { - struct singleLine *l; - needs &= ~ KERNEL_MB; - - l = addLine(new, config->cfi, LT_KERNEL, - config->secondaryIndent, - newMBKernel + strlen(prefix)); - - tmplLine = lastLine; - if (!new->lines) { - new->lines = l; - } else { - newLine->next = l; - newLine = l; - } - continue; - } else if (tmplLine->type == LT_KERNEL && - template->multiboot && !new->multiboot) { - continue; /* don''t need multiboot kernel here */ - } + if (tmplLine->type == LT_KERNEL && + tmplLine->numElements >= 2) { + if (!template->multiboot && (needs & NEED_MB)) { + /* it''s not a multiboot template and this is the kernel + * line. Try to be intelligent about inserting the + * hypervisor at the same time. + */ + if (config->cfi->mbHyperFirst) { + /* insert the hypervisor first */ + newLine = addLine(new, config->cfi, LT_HYPER, + tmplLine->indent, + newMBKernel + strlen(prefix)); + /* set up for adding the kernel line */ + free(tmplLine->indent); + tmplLine->indent = strdup(config->secondaryIndent); + needs &= ~NEED_MB; + } + if (needs & NEED_KERNEL) { + /* use addLineTmpl to preserve line elements, + * otherwise we could just call addLine. Unfortunately + * this means making some changes to the template + * such as the indent change above and the type + * change below. + */ + struct keywordTypes * mbm_kw = + getKeywordByType(LT_MBMODULE, config->cfi); + if (mbm_kw) { + tmplLine->type = LT_MBMODULE; + free(tmplLine->elements[0].item); + tmplLine->elements[0].item = strdup(mbm_kw->key); + } + newLine = addLineTmpl(new, tmplLine, newLine, + newKernelPath + strlen(prefix)); + needs &= ~NEED_KERNEL; + } + if (needs & NEED_MB) { /* !mbHyperFirst */ + newLine = addLine(new, config->cfi, LT_HYPER, + config->secondaryIndent, + newMBKernel + strlen(prefix)); + needs &= ~NEED_MB; + } + } else if (needs & NEED_KERNEL) { + newLine = addLineTmpl(new, tmplLine, newLine, + newKernelPath + strlen(prefix)); + needs &= ~NEED_KERNEL; + } - if (!new->lines) { - newLine = malloc(sizeof(*newLine)); - new->lines = newLine; - } else { - newLine->next = malloc(sizeof(*newLine)); - newLine = newLine->next; - } + } else if (tmplLine->type == LT_HYPER && + tmplLine->numElements >= 2) { + if (new->multiboot) { + if (needs & NEED_MB) { + newLine = addLineTmpl(new, tmplLine, newLine, + newMBKernel + strlen(prefix)); + needs &= ~NEED_MB; + } + } else if (needs & NEED_KERNEL) { + /* template is multi but new is not, + * insert the kernel where the hypervisor was before + */ + tmplLine->type = LT_KERNEL; + free(tmplLine->elements[0].item); + tmplLine->elements[0].item = + strdup(getKeywordByType(LT_KERNEL, config->cfi)->key); + newLine = addLineTmpl(new, tmplLine, newLine, + newKernelPath + strlen(prefix)); + needs &= ~NEED_KERNEL; + } + } else if (tmplLine->type == LT_MBMODULE && + tmplLine->numElements >= 2) { + if (new->multiboot) { + if (needs & NEED_KERNEL) { + newLine = addLineTmpl(new, tmplLine, newLine, + newKernelPath + + strlen(prefix)); + needs &= ~NEED_KERNEL; + } else if (config->cfi->mbInitRdIsModule && + (needs & NEED_INITRD)) { + newLine = addLineTmpl(new, tmplLine, newLine, + newKernelInitrd); + needs &= ~NEED_INITRD; + } + } else if (needs & NEED_INITRD) { + /* template is multi but new is not, + * insert the initrd where the module was before + */ + newLine = addLine(new, config->cfi, LT_INITRD, + config->secondaryIndent, + newKernelInitrd + strlen(prefix)); + needs &= ~NEED_INITRD; + } - newLine->indent = strdup(tmplLine->indent); - newLine->next = NULL; - newLine->type = tmplLine->type; - newLine->numElements = tmplLine->numElements; - newLine->elements = malloc(sizeof(*newLine->elements) * - newLine->numElements); - for (i = 0; i < newLine->numElements; i++) { - newLine->elements[i].item = strdup(tmplLine->elements[i].item); - newLine->elements[i].indent = strdup(tmplLine->elements[i].indent); } @@ -2257,127 +2367,124 @@ strlen(prefix)); } } else if (tmplLine->type == LT_INITRD && - tmplLine->numElements >= 2) { - needs &= ~KERNEL_INITRD; - free(newLine->elements[1].item); - if (new->multiboot && !template->multiboot) { - free(newLine->elements[0].item); - newLine->elements[0].item = strdup("module"); - newLine->type = LT_MBMODULE; - } - rootspec = getRootSpecifier(tmplLine->elements[1].item); - if (rootspec != NULL) { - newLine->elements[1].item = sdupprintf("%s%s", - rootspec, - newKernelInitrd + - strlen(prefix)); - } else { - newLine->elements[1].item = strdup(newKernelInitrd + - strlen(prefix)); - } - } else if (tmplLine->type == LT_MBMODULE && - tmplLine->numElements >= 2 && (needs & KERNEL_INITRD)) { - needs &= ~KERNEL_INITRD; - if (!new->multiboot && template->multiboot) { - free(newLine->elements[0].item); - newLine->elements[0].item = strdup("initrd"); - newLine->type = LT_INITRD; - } - free(newLine->elements[1].item); - rootspec = getRootSpecifier(tmplLine->elements[1].item); - if (rootspec != NULL) { - newLine->elements[1].item = sdupprintf("%s%s", - rootspec, - newKernelInitrd + - strlen(prefix)); - } else { - newLine->elements[1].item = strdup(newKernelInitrd + - strlen(prefix)); - } - } else if (tmplLine->type == LT_TITLE && - tmplLine->numElements >= 2) { - needs &= ~KERNEL_TITLE; - - for (i = 1; i < newLine->numElements; i++) { - free(newLine->elements[i].item); - free(newLine->elements[i].indent); + tmplLine->numElements >= 2 && + (needs & NEED_INITRD)) { + if (new->multiboot && !template->multiboot && + config->cfi->mbInitRdIsModule) { + /* make sure we don''t insert the module initrd + * before the module kernel... if we don''t do it here, + * it will be inserted following the template. + */ + if (!needs & NEED_KERNEL) { + newLine = addLine(new, config->cfi, LT_MBMODULE, + config->secondaryIndent, + newKernelInitrd + strlen(prefix)); + needs &= ~NEED_INITRD; + } + } else { + newLine = addLineTmpl(new, tmplLine, newLine, + newKernelInitrd + strlen(prefix)); + needs &= ~NEED_INITRD; } - newLine->elements[1].item = strdup(newKernelTitle); - newLine->elements[1].indent = strdup(""); - newLine->numElements = 2; } else if (tmplLine->type == LT_TITLE && - config->cfi->titleBracketed && - tmplLine->numElements == 1) { - needs &= ~KERNEL_TITLE; - free(newLine->elements[0].item); - free(newLine->elements[0].indent); - newLine->elements = malloc(sizeof(*newLine->elements) * - newLine->numElements); - - newLine->elements[0].item = malloc(strlen(newKernelTitle) + 3); - sprintf(newLine->elements[0].item, "[%s]", newKernelTitle); - newLine->elements[0].indent = strdup(""); - newLine->numElements = 1; - } + (needs & NEED_TITLE)) { + if (tmplLine->numElements >= 2) { + newLine = addLineTmpl(new, tmplLine, newLine, + newKernelTitle); + needs &= ~NEED_TITLE; + } else if (tmplLine->numElements == 1 && + config->cfi->titleBracketed) { + /* addLineTmpl doesn''t handle titleBracketed */ + newLine = addLine(new, config->cfi, LT_TITLE, + tmplLine->indent, newKernelTitle); + needs &= ~NEED_TITLE; + } + + } else { + /* pass through other lines from the template */ + newLine = addLineTmpl(new, tmplLine, newLine, NULL); + } } + } else { - for (i = 0; config->cfi->keywords[i].key; i++) { - if ((config->cfi->keywords[i].type == config->cfi->entrySeparator) || (config->cfi->keywords[i].type == LT_OTHER)) + /* don''t have a template, so start the entry with the + * appropriate starting line + */ + switch (config->cfi->entrySeparator) { + case LT_KERNEL: + if (new->multiboot && config->cfi->mbHyperFirst) { + /* fall through to LT_HYPER */ + } else { + newLine = addLine(new, config->cfi, LT_KERNEL, + config->primaryIndent, + newKernelPath + strlen(prefix)); + needs &= ~NEED_KERNEL; + break; + } + + case LT_HYPER: + newLine = addLine(new, config->cfi, LT_HYPER, + config->primaryIndent, + newMBKernel + strlen(prefix)); + needs &= ~NEED_MB; break; - } - switch (config->cfi->keywords[i].type) { - case LT_KERNEL: needs &= ~KERNEL_KERNEL, - chptr = newKernelPath + strlen(prefix); - type = LT_KERNEL; break; - case LT_TITLE: needs &= ~KERNEL_TITLE, chptr = newKernelTitle; - type = LT_TITLE; break; - default: - /* zipl strikes again */ - if (config->cfi->titleBracketed) { - needs &= ~KERNEL_TITLE; - chptr = newKernelTitle; - type = LT_TITLE; - break; - } else { - abort(); - } - } + case LT_TITLE: + newLine = addLine(new, config->cfi, LT_TITLE, + config->primaryIndent, newKernelTitle); + needs &= ~NEED_TITLE; + break; - newLine = addLine(new, config->cfi, type, config->primaryIndent, chptr); - new->lines = newLine; + default: + abort(); + } } - if (new->multiboot) { - if (needs & KERNEL_MB) - newLine = addLine(new, config->cfi, LT_KERNEL, - config->secondaryIndent, - newMBKernel + strlen(prefix)); - if (needs & KERNEL_KERNEL) - newLine = addLine(new, config->cfi, LT_MBMODULE, - config->secondaryIndent, - newKernelPath + strlen(prefix)); - /* don''t need to check for title as it''s guaranteed to have been - * done as we only do multiboot with grub which uses title as - * a separator */ - if (needs & KERNEL_INITRD && newKernelInitrd) - newLine = addLine(new, config->cfi, LT_MBMODULE, - config->secondaryIndent, - newKernelInitrd + strlen(prefix)); - } else { - if (needs & KERNEL_KERNEL) - newLine = addLine(new, config->cfi, LT_KERNEL, - config->secondaryIndent, - newKernelPath + strlen(prefix)); - if (needs & KERNEL_TITLE) - newLine = addLine(new, config->cfi, LT_TITLE, - config->secondaryIndent, - newKernelTitle); - if (needs & KERNEL_INITRD && newKernelInitrd) - newLine = addLine(new, config->cfi, LT_INITRD, - config->secondaryIndent, - newKernelInitrd + strlen(prefix)); + /* add the remainder of the lines, i.e. those that either + * weren''t present in the template, or in the case of no template, + * all the lines following the entrySeparator. + */ + if (needs & NEED_TITLE) { + newLine = addLine(new, config->cfi, LT_TITLE, + config->secondaryIndent, + newKernelTitle); + needs &= ~NEED_TITLE; + } + if ((needs & NEED_MB) && config->cfi->mbHyperFirst) { + newLine = addLine(new, config->cfi, LT_HYPER, + config->secondaryIndent, + newMBKernel + strlen(prefix)); + needs &= ~NEED_MB; + } + if (needs & NEED_KERNEL) { + newLine = addLine(new, config->cfi, + (new->multiboot && getKeywordByType(LT_MBMODULE, + config->cfi)) ? + LT_MBMODULE : LT_KERNEL, + config->secondaryIndent, + newKernelPath + strlen(prefix)); + needs &= ~NEED_KERNEL; + } + if (needs & NEED_MB) { + newLine = addLine(new, config->cfi, LT_HYPER, + config->secondaryIndent, + newMBKernel + strlen(prefix)); + needs &= ~NEED_MB; + } + if (needs & NEED_INITRD) { + newLine = addLine(new, config->cfi, + (new->multiboot && getKeywordByType(LT_MBMODULE, + config->cfi)) ? + LT_MBMODULE : LT_INITRD, + config->secondaryIndent, + newKernelInitrd + strlen(prefix)); + needs &= ~NEED_INITRD; + } + + if (needs) { + printf(_("grubby: needs=%d, aborting\n"), needs); + abort(); } if (updateImage(config, "0", prefix, newKernelArgs, NULL,
Aron Griffis
2006-Jun-07 21:38 UTC
[Fedora-xen] [patch 3/4] elilo multiboot support (grubby: args)
elilo handles hypervisor args by putting a "--" separator on the append line, like this: append="hyper args -- kernel args" This patch modifies updateActualImage() to handle this situation, including removing the hypervisor args plus separator when a multiboot template is used to construct a non-multiboot entry. Signed-off-by: Aron Griffis <aron@hp.com> grubby.c | 279 ++++++++++++++++++++++++++++----------------------------------- 1 file changed, 128 insertions(+), 151 deletions(-) --- grubby.c.2 2006-06-07 16:14:33.000000000 -0400 +++ grubby.c 2006-06-07 16:16:44.000000000 -0400 @@ -101,6 +101,7 @@ int titleBracketed; int mbHyperFirst; int mbInitRdIsModule; + int mbConcatArgs; }; struct keywordTypes grubKeywords[] = { @@ -127,6 +128,7 @@ 0, /* titleBracketed */ 1, /* mbHyperFirst */ 1, /* mbInitRdIsModule */ + 0, /* mbConcatArgs */ }; struct keywordTypes yabootKeywords[] = { @@ -224,6 +226,7 @@ 0, /* titleBracketed */ 0, /* mbHyperFirst */ 0, /* mbInitRdIsModule */ + 1, /* mbConcatArgs */ }; struct configFileInfo liloConfigType = { @@ -238,6 +241,7 @@ 0, /* titleBracketed */ 0, /* mbHyperFirst */ 0, /* mbInitRdIsModule */ + 0, /* mbConcatArgs */ }; struct configFileInfo yabootConfigType = { @@ -252,6 +256,7 @@ 0, /* titleBracketed */ 0, /* mbHyperFirst */ 0, /* mbInitRdIsModule */ + 0, /* mbConcatArgs */ }; struct configFileInfo siloConfigType = { @@ -266,6 +271,7 @@ 0, /* titleBracketed */ 0, /* mbHyperFirst */ 0, /* mbInitRdIsModule */ + 0, /* mbConcatArgs */ }; struct configFileInfo ziplConfigType = { @@ -280,6 +286,7 @@ 1, /* titleBracketed */ 0, /* mbHyperFirst */ 0, /* mbInitRdIsModule */ + 0, /* mbConcatArgs */ }; struct grubConfig { @@ -1754,14 +1761,13 @@ struct singleEntry * entry; struct singleLine * line, * rootLine; int index = 0; - int i, j, k; + int i, k; const char ** newArgs, ** oldArgs; const char ** arg; - const char * chptr; - int useKernelArgs = 0; - int useRoot = 0; + int useKernelArgs, useRoot; int firstElement; int *usedElements, *usedArgs; + int doreplace; if (!image) return 0; @@ -1788,54 +1794,102 @@ } } - for (i = 0; cfg->cfi->keywords[i].key; i++) - if (cfg->cfi->keywords[i].type == LT_KERNELARGS) break; - if (cfg->cfi->keywords[i].key) - useKernelArgs = 1; + useKernelArgs = (getKeywordByType(LT_KERNELARGS, cfg->cfi) + && (!multibootArgs || cfg->cfi->mbConcatArgs)); - for (i = 0; cfg->cfi->keywords[i].key; i++) - if (cfg->cfi->keywords[i].type == LT_ROOT) break; + useRoot = (getKeywordByType(LT_ROOT, cfg->cfi) + && !multibootArgs); - if (cfg->cfi->keywords[i].key) - useRoot = 1; + for (k = 0, arg = newArgs; *arg; arg++, k++) ; + usedArgs = calloc(k, sizeof(*usedArgs)); - k = 0; - for (arg = newArgs; *arg; arg++) - k++; - usedArgs = calloc(k, sizeof(int)); + for (; (entry = findEntryByPath(cfg, image, prefix, &index)); index++) { - while ((entry = findEntryByPath(cfg, image, prefix, &index))) { - index++; + if (multibootArgs && !entry->multiboot) + continue; - line = entry->lines; - while (line && line->type != LT_KERNEL) line = line->next; - if (!line) continue; - firstElement = 2; - - if (entry->multiboot && !multibootArgs) { - /* first mb module line is the real kernel */ - while (line && line->type != LT_MBMODULE) line = line->next; - firstElement = 2; - } else if (useKernelArgs) { - while (line && line->type != LT_KERNELARGS) line = line->next; + /* Determine where to put the args. If this config supports + * LT_KERNELARGS, use that. Otherwise use + * LT_HYPER/LT_KERNEL/LT_MBMODULE lines. + */ + if (useKernelArgs) { + line = getLineByType(LT_KERNELARGS, entry->lines); + if (!line) { + /* no LT_KERNELARGS, need to add it */ + line = addLine(entry, cfg->cfi, LT_KERNELARGS, + cfg->secondaryIndent, NULL); + } firstElement = 1; + + } else if (multibootArgs) { + line = getLineByType(LT_HYPER, entry->lines); + if (!line) { + /* a multiboot entry without LT_HYPER? */ + continue; + } + firstElement = 2; + + } else { + line = getLineByType2(LT_KERNEL, LT_MBMODULE, entry->lines); + if (!line) { + /* no LT_KERNEL or LT_MBMODULE in this entry? */ + continue; + } + firstElement = 2; } - if (!line && useKernelArgs) { - /* no append in there, need to add it */ - line = addLine(entry, cfg->cfi, LT_KERNELARGS, NULL, NULL); + /* handle the elilo case which does: + * append="hypervisor args -- kernel args" + */ + if (entry->multiboot && cfg->cfi->mbConcatArgs) { + /* this is a multiboot entry, make sure there''s + * -- on the args line + */ + for (i = firstElement; i < line->numElements; i++) { + if (!strcmp(line->elements[i].item, "--")) + break; + } + if (i == line->numElements) { + /* assume all existing args are kernel args, + * prepend -- to make it official + */ + insertElement(line, "--", firstElement); + i = firstElement; + } + if (!multibootArgs) { + /* kernel args start after the -- */ + firstElement = i + 1; + } + } else if (cfg->cfi->mbConcatArgs) { + /* this is a non-multiboot entry, remove hyper args */ + for (i = firstElement; i < line->numElements; i++) { + if (!strcmp(line->elements[i].item, "--")) + break; + } + if (i < line->numElements) { + /* remove args up to -- */ + while (strcmp(line->elements[firstElement].item, "--")) + removeElement(line, firstElement); + /* remove -- */ + removeElement(line, firstElement); + } } - usedElements = calloc(line->numElements, sizeof(int)); - - k = 0; - for (arg = newArgs; *arg; arg++) { - if (usedArgs[k]) { - k++; - continue; - } + usedElements = calloc(line->numElements, sizeof(*usedElements)); + + for (k = 0, arg = newArgs; *arg; arg++, k++) { + if (usedArgs[k]) continue; + + doreplace = 1; for (i = firstElement; i < line->numElements; i++) { + if (multibootArgs && cfg->cfi->mbConcatArgs && + !strcmp(line->elements[i].item, "--")) + { + /* reached the end of hyper args, insert here */ + doreplace = 0; + break; + } if (usedElements[i]) continue; if (!argMatch(line->elements[i].item, *arg)) { @@ -1844,91 +1898,62 @@ break; } } - chptr = strchr(*arg, ''=''); - if (i < line->numElements) { - /* replace */ + if (i < line->numElements && doreplace) { + /* direct replacement */ free(line->elements[i].item); line->elements[i].item = strdup(*arg); - } else if (useRoot && !strncmp(*arg, "root=/dev/", 10) && *chptr) { - rootLine = entry->lines; - while (rootLine && rootLine->type != LT_ROOT) - rootLine = rootLine->next; - if (!rootLine) { - rootLine = addLine(entry, cfg->cfi, LT_ROOT, NULL, NULL); - rootLine->elements = realloc(rootLine->elements, - 2 * sizeof(*rootLine->elements)); - rootLine->numElements++; - rootLine->elements[1].indent = strdup(""); - rootLine->elements[1].item = strdup(""); - } - free(rootLine->elements[1].item); - rootLine->elements[1].item = strdup(chptr + 1); - } else { - /* append */ - line->elements = realloc(line->elements, - (line->numElements + 1) * sizeof(*line->elements)); - line->elements[line->numElements].item = strdup(*arg); - usedElements = realloc(usedElements, - (line->numElements + 1) * sizeof(int)); - usedElements[line->numElements] = 1; - - if (line->numElements > 1) { - /* add to existing list of arguments */ - line->elements[line->numElements].indent = - line->elements[line->numElements - 1].indent; - line->elements[line->numElements - 1].indent = strdup(" "); + } else if (useRoot && !strncmp(*arg, "root=/dev/", 10)) { + /* root= replacement */ + rootLine = getLineByType(LT_ROOT, entry->lines); + if (rootLine) { + free(rootLine->elements[1].item); + rootLine->elements[1].item = strdup(*arg + 5); } else { - /* First thing on this line; treat a bit differently. Note - this is only possible if we''ve added a LT_KERNELARGS - entry */ - line->elements[line->numElements].indent = strdup(""); + rootLine = addLine(entry, cfg->cfi, LT_ROOT, + cfg->secondaryIndent, *arg + 5); } + } - line->numElements++; + else { + /* insert/append */ + insertElement(line, *arg, i); + usedElements = realloc(usedElements, line->numElements * + sizeof(*usedElements)); + memmove(&usedElements[i + 1], &usedElements[i], + line->numElements - i - 1); + usedElements[i] = 1; /* if we updated a root= here even though there is a LT_ROOT available we need to remove the LT_ROOT entry (this will happen if we switch from a device to a label) */ if (useRoot && !strncmp(*arg, "root=", 5)) { - rootLine = entry->lines; - while (rootLine && rootLine->type != LT_ROOT) - rootLine = rootLine->next; - if (rootLine) { + rootLine = getLineByType(LT_ROOT, entry->lines); + if (rootLine) removeLine(entry, rootLine); - } } } - k++; } free(usedElements); - /* no arguments to remove (i.e. no append line) */ - if (!line) continue; - - /* this won''t remove an LT_ROOT item properly (but then again, - who cares? */ for (arg = oldArgs; *arg; arg++) { - for (i = firstElement; i < line->numElements; i++) - if (!argMatch(line->elements[i].item, *arg)) + for (i = firstElement; i < line->numElements; i++) { + if (multibootArgs && cfg->cfi->mbConcatArgs && + !strcmp(line->elements[i].item, "--")) + /* reached the end of hyper args, stop here */ + break; + if (!argMatch(line->elements[i].item, *arg)) { + removeElement(line, i); break; - - if (i < line->numElements) { - /* if this isn''t the first argument the previous argument - gets this arguments post-indention */ - if (i > firstElement) { - free(line->elements[i - 1].indent); - line->elements[i - 1].indent = line->elements[i].indent; } - - free(line->elements[i].item); - - for (j = i + 1; j < line->numElements; j++) - line->elements[j - 1] = line->elements[j]; - - line->numElements--; + } + /* handle removing LT_ROOT line too */ + if (useRoot && !strncmp(*arg, "root=", 5)) { + rootLine = getLineByType(LT_ROOT, entry->lines); + if (rootLine) + removeLine(entry, rootLine); } } @@ -2318,54 +2343,6 @@ needs &= ~NEED_INITRD; } - strdup(tmplLine->elements[i].indent); - } - - lastLine = tmplLine; - if (tmplLine->type == LT_KERNEL && tmplLine->numElements >= 2) { - char * repl; - if (!template->multiboot) { - needs &= ~KERNEL_KERNEL; - repl = newKernelPath; - } else { - needs &= ~KERNEL_MB; - repl = newMBKernel; - } - if (new->multiboot && !template->multiboot) { - free(newLine->elements[0].item); - newLine->elements[0].item = strdup("module"); - newLine->type = LT_MBMODULE; - } - free(newLine->elements[1].item); - rootspec = getRootSpecifier(tmplLine->elements[1].item); - if (rootspec != NULL) { - newLine->elements[1].item = sdupprintf("%s%s", - rootspec, - repl + - strlen(prefix)); - } else { - newLine->elements[1].item = strdup(repl + - strlen(prefix)); - } - } else if (tmplLine->type == LT_MBMODULE && - tmplLine->numElements >= 2 && (needs & KERNEL_KERNEL)) { - needs &= ~KERNEL_KERNEL; - if (!new->multiboot && template->multiboot) { - free(newLine->elements[0].item); - newLine->elements[0].item = strdup("kernel"); - newLine->type = LT_KERNEL; - } - free(newLine->elements[1].item); - rootspec = getRootSpecifier(tmplLine->elements[1].item); - if (rootspec != NULL) { - newLine->elements[1].item = sdupprintf("%s%s", - rootspec, - newKernelPath + - strlen(prefix)); - } else { - newLine->elements[1].item = strdup(newKernelPath + - strlen(prefix)); - } } else if (tmplLine->type == LT_INITRD && tmplLine->numElements >= 2 && (needs & NEED_INITRD)) {
Aron Griffis
2006-Jun-07 21:39 UTC
[Fedora-xen] [patch 4/4] elilo multiboot support (new-kernel-pkg)
This patch allows new-kernel-pkg to call grubby with multiboot arguments for elilo. Additionally it fixes multiple /sbin/grubby instances to use the variable from the top of the script. Signed-off-by: Aron Griffis <aron@hp.com> new-kernel-pkg | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) --- new-kernel-pkg.0 Mon Jun 05 14:04:27 2006 -0400 +++ new-kernel-pkg Wed Jun 07 16:41:34 2006 -0400 @@ -125,7 +125,7 @@ install() { fi fi - if [ -n "$mbkernel" ]; then + if [ -n "$mbkernel" -a -n "$cfgLilo" -a "$liloFlag" != "elilo" ]; then [ -n "$verbose" ] && echo "multiboot specified, not updating lilo.conf" cfgLilo="" fi @@ -140,7 +140,7 @@ install() { else title="Red Hat Linux ($version)" fi - /sbin/grubby --add-kernel=$bootPrefix/$kernelName-$version \ + $grubby --add-kernel=$bootPrefix/$kernelName-$version \ $INITRD --copy-default $makedefault --title "$title" \ ${mbkernel:+--add-multiboot="$mbkernel"} ${mbargs:+--mbargs="$mbargs"} \ --args="root=$rootdevice $kernargs" --remove-kernel="TITLE=$title" @@ -151,11 +151,11 @@ install() { if [ -n "$cfgLilo" ]; then [ -n "$verbose" ] && echo "adding $version to $liloConfig" - /sbin/grubby --add-kernel=$bootPrefix/$kernelName-$version $INITRD \ - --copy-default $makedefault --title $version \ - --args="root=$rootdevice $kernargs" \ - --remove-kernel="TITLE=$version" \ - --$liloFlag + $grubby --add-kernel=$bootPrefix/$kernelName-$version $INITRD + --copy-default $makedefault --title $version \ + ${mbkernel:+--add-multiboot="$mbkernel"} ${mbargs:+--mbargs="$mbargs"} \ + --args="root=$rootdevice $kernargs" --remove-kernel="TITLE=$version" \ + --$liloFlag if [ -n "$runLilo" ]; then [ -n "$verbose" ] && echo "running $lilo" @@ -181,14 +181,14 @@ remove() { if [ -n "$cfgGrub" ]; then [ -n "$verbose" ] && echo "removing $version from $grubConfig" - /sbin/grubby --remove-kernel=$bootPrefix/$kernelName-$version + $grubby --remove-kernel=$bootPrefix/$kernelName-$version else [ -n "$verbose" ] && echo "$grubConfig does not exist, not running grubby" fi if [ -n "$cfgLilo" ]; then [ -n "$verbose" ] && echo "removing $version from $liloConfig" - /sbin/grubby --remove-kernel=$bootPrefix/$kernelName-$version \ + $grubby --remove-kernel=$bootPrefix/$kernelName-$version \ --$liloFlag if [ -n "$runLilo" ]; then @@ -207,7 +207,7 @@ update() { update() { if [ -n "$cfgGrub" ]; then [ -n "$verbose" ] && echo "updating $version from $grubConfig" - /sbin/grubby --update-kernel=$bootPrefix/$kernelName-$version \ + $grubby --update-kernel=$bootPrefix/$kernelName-$version \ ${kernargs:+--args="$kernargs"} \ ${removeargs:+--remove-args="$removeargs"} else @@ -216,7 +216,7 @@ update() { if [ -n "$cfgLilo" ]; then [ -n "$verbose" ] && echo "updating $version from $liloConfig" - /sbin/grubby --update-kernel=$bootPrefix/$kernelName-$version \ + $grubby --update-kernel=$bootPrefix/$kernelName-$version \ ${kernargs:+--args="$kernargs"} \ ${removeargs:+--remove-args="$removeargs"} \ --$liloFlag
Hi Jeremy, I''m still waiting on review of these patches. Is there anything I can do to help out beyond sending this reminder? Thanks, Aron
On Wed, 2006-06-21 at 17:53 -0400, Aron Griffis wrote:> I''m still waiting on review of these patches. Is there anything I can > do to help out beyond sending this reminder?Have a time machine handy? :-) Hopefully things are actually calming down for real today which will then give me some time to look at things like this tomorrow... my whiteboard is full of stuff that I''ve been dropping over the past week or so :-/ Jeremy
Jeremy Katz wrote: [Wed Jun 21 2006, 06:26:38PM EDT]> On Wed, 2006-06-21 at 17:53 -0400, Aron Griffis wrote: > > I''m still waiting on review of these patches. Is there anything I can > > do to help out beyond sending this reminder? > > Have a time machine handy? :-)Unfortunately no... :-|> Hopefully things are actually calming down for real today which will > then give me some time to look at things like this tomorrow... my > whiteboard is full of stuff that I''ve been dropping over the past > week or so :-/Great, thanks. This situation starts to concern me because I''d my changes to get some soak time in rawhide before FC6 test releases. If there are issues, I''m more than willing to follow up with debugging and patches, but it gets more worrisome as the schedule is compressed. Regards, Aron
The new-kernel-pkg patch submitted in this thread contained a typo. This patch fixes that. Signed-off-by: Aron Griffis <aron@hp.com> diff -r f59e085b800f -r 624f49b8c307 grubby/new-kernel-pkg --- a/grubby/new-kernel-pkg Wed Jun 07 16:17:07 2006 -0400 +++ b/grubby/new-kernel-pkg Thu Jun 22 20:16:57 2006 -0400 @@ -151,7 +151,7 @@ install() { if [ -n "$cfgLilo" ]; then [ -n "$verbose" ] && echo "adding $version to $liloConfig" - $grubby --add-kernel=$bootPrefix/$kernelName-$version $INITRD + $grubby --add-kernel=$bootPrefix/$kernelName-$version $INITRD \ --copy-default $makedefault --title $version \ ${mbkernel:+--add-multiboot="$mbkernel"} ${mbargs:+--mbargs="$mbargs"} \ --args="root=$rootdevice $kernargs" --remove-kernel="TITLE=$version" \
(dropping anaconda-devel and pjones from the cc... not generally applicable for anaconda-devel and I''m fielding this for Peter) On Wed, 2006-06-07 at 17:26 -0400, Aron Griffis wrote:> The following patches give grubby the ability to manipulate > multiboot-capable elilo.conf. Fedora''s elilo already contains the > support, these patches simply teach grubby to take advantage of it. > These patches are an enabler for using Xen on ia64 with Fedora. > > I have tested these changes with both grubby and elilo, using both > multiboot and non-multiboot configurations.Aron -- Overall, these look pretty good. A few stylistic-ish things * The patches don''t work individually -- applying them in sequence ends up with some not compiling bits. Was pretty easy to fix up, but worth noting * dprintf is already included in the standard library as a GNU extension, but with a different functionality -- something like dbgprintf would be better just to avoid problems there * Instead of having the getLineByType2 thing, it would probably be a little cleaner to just change the types to be a normal int instead of an enum and then be able to determine matches via bitwise operators In the bigger realm, there seems to be a bug or two lingering. Running the test suite (''make test'' from the toplevel) seems to fail for x86/x86_64 with the patches applied -- it looks like we''re gaining an initrd in cases where it''s unexpected (copy-default shouldn''t be copying the initrd, but it looks like it might be). Also, it looks like kernel arguments might be getting lost in the multiboot case. I''ll try to look at this a little more later in the afternoon, but given how the past two weeks have been "later in the afternoon" could end up being Friday :/ Also, it''d be nice to have some test cases to add to the test suite just so that we can more easily ensure that minor bugfixes don''t cause regressions. Jeremy
Jeremy, Jeremy Katz wrote: [Mon Jun 26 2006, 03:21:18PM EDT]> Overall, these look pretty good.Good to hear. :-)> A few stylistic-ish things > * The patches don''t work individually -- applying them in sequence ends > up with some not compiling bits. Was pretty easy to fix up, but worth > notingThanks, and sorry for the inconvenience.> * dprintf is already included in the standard library as a GNU > extension, but with a different functionality -- something like > dbgprintf would be better just to avoid problems thereOkay> * Instead of having the getLineByType2 thing, it would probably be a > little cleaner to just change the types to be a normal int instead of an > enum and then be able to determine matches via bitwise operatorsHeh, I wanted to do this but was already nervous about the volume of code being changed. I agree it would be better. I''ll change this for the next submission.> In the bigger realm, there seems to be a bug or two lingering. Running > the test suite (''make test'' from the toplevel) seems to fail for > x86/x86_64 with the patches applied -- it looks like we''re gaining an > initrd in cases where it''s unexpected (copy-default shouldn''t be copying > the initrd, but it looks like it might be). Also, it looks like kernel > arguments might be getting lost in the multiboot case. I''ll try to look > at this a little more later in the afternoon, but given how the past two > weeks have been "later in the afternoon" could end up being Friday :/I hadn''t even noticed the existence of the test suite, silly me. :-| My testing had all been manual. I''ll run through the test suite and fix up the errors.> Also, it''d be nice to have some test cases to add to the test suite just > so that we can more easily ensure that minor bugfixes don''t cause > regressions.Will do. Thanks for the review. Regards, Aron
Jeremy Katz wrote: [Mon Jun 26 2006, 03:21:18PM EDT]> * dprintf is already included in the standard library as a GNU > extension, but with a different functionality -- something like > dbgprintf would be better just to avoid problems thererenamed to dbgPrintf to match the style of the rest of the program.> * Instead of having the getLineByType2 thing, it would probably be a > little cleaner to just change the types to be a normal int instead of an > enum and then be able to determine matches via bitwise operatorsI left this an enum with explicit bit values. So you retain whatever advantage you gained using an enum, and only lose it when they''re OR''d together for getLineByType.> In the bigger realm, there seems to be a bug or two lingering. Running > the test suite (''make test'' from the toplevel) seems to fail for > x86/x86_64 with the patches applied -- it looks like we''re gaining an > initrd in cases where it''s unexpected (copy-default shouldn''t be copying > the initrd, but it looks like it might be). Also, it looks like kernel > arguments might be getting lost in the multiboot case. I''ll try to look > at this a little more later in the afternoon, but given how the past two > weeks have been "later in the afternoon" could end up being Friday :/All of the test suite issues are fixed in the patch attached to this message. addNewKernel: - don''t allow fall-through for LT_INITRD - don''t corrupt the template, use a copy instead - don''t forget to remove the prefix from the initrd - when converting a grub multiboot template to a normal entry, use the module template lines to preserve kernel parameters addLineTmpl: - set val=NULL for a bracketed title to prevent elements[1] being set isBracketedTitle: - change order of tests for correctness (not related to a failure) grubby/test/results/updargs/g3.7: - remove trailing spaces, they were only there because previous versions of grubby didn''t quite handle EOL correctly grubby/test.sh: - current arch doesn''t affect what tests can be run, so run them all The only test suite issue I didn''t fix was related to symlink handling since I didn''t touch that code. I think the code is attempting to resolve the symlink and create the temporary file next to the target, but the implementation is completely bogus at the moment (the output of readlink() doesn''t change depending on cwd).> Also, it''d be nice to have some test cases to add to the test suite just > so that we can more easily ensure that minor bugfixes don''t cause > regressions.Not included in this message, but I''ll follow up with these too. Signed-off-by: Aron Griffis <aron@hp.com> diff -r 57dd3b104446 -r 6964eee46f24 grubby/grubby.c --- a/grubby/grubby.c Tue Jun 27 19:09:26 2006 -0400 +++ b/grubby/grubby.c Wed Jun 28 07:44:20 2006 -0400 @@ -34,7 +34,7 @@ #define DEBUG 0 #if DEBUG -#define dbgPrintf(format, args...) printf(format , ## args) +#define dbgPrintf(format, args...) fprintf(stderr, format , ## args) #else #define dbgPrintf(format, args...) #endif @@ -89,10 +89,10 @@ struct singleEntry { /* These defines are (only) used in addNewKernel() */ #define NEED_KERNEL (1 << 0) -#define NEED_INITRD (1 << 2) -#define NEED_TITLE (1 << 3) -#define NEED_ARGS (1 << 4) -#define NEED_MB (1 << 5) +#define NEED_INITRD (1 << 1) +#define NEED_TITLE (1 << 2) +#define NEED_ARGS (1 << 3) +#define NEED_MB (1 << 4) #define MAIN_DEFAULT (1 << 0) #define DEFAULT_SAVED -2 @@ -321,6 +321,7 @@ struct singleEntry * findEntryByPath(str int * index); static int readFile(int fd, char ** bufPtr); static void lineInit(struct singleLine * line); +struct singleLine * lineDup(struct singleLine * line); static void lineFree(struct singleLine * line); static int lineWrite(FILE * out, struct singleLine * line, struct configFileInfo * cfi); @@ -403,7 +404,7 @@ static struct singleLine * getLineByType } static int isBracketedTitle(struct singleLine * line) { - if ((*line->elements[0].item == ''['') && (line->numElements == 1)) { + if (line->numElements == 1 && *line->elements[0].item == ''['') { int len = strlen(line->elements[0].item); if (*(line->elements[0].item + len - 1) == '']'') { /* FIXME: this is a hack... */ @@ -467,6 +468,25 @@ static void lineInit(struct singleLine * line->elements = NULL; line->numElements = 0; line->next = NULL; +} + +struct singleLine * lineDup(struct singleLine * line) { + int i; + struct singleLine * newLine = malloc(sizeof(*newLine)); + + newLine->indent = strdup(line->indent); + newLine->next = NULL; + newLine->type = line->type; + newLine->numElements = line->numElements; + newLine->elements = malloc(sizeof(*newLine->elements) * + newLine->numElements); + + for (i = 0; i < newLine->numElements; i++) { + newLine->elements[i].indent = strdup(line->elements[i].indent); + newLine->elements[i].item = strdup(line->elements[i].item); + } + + return newLine; } static void lineFree(struct singleLine * line) { @@ -692,15 +712,8 @@ static struct grubConfig * readConfig(co * lines came earlier in the template, make sure to use LT_HYPER * instead of LT_KERNEL now */ - if (entry->multiboot) { - struct singleLine * l; - for (l = entry->lines; l; l = l->next) { - if (l->type == LT_MBMODULE) { - line->type = LT_HYPER; /* caught it! */ - break; - } - } - } + if (entry->multiboot) + line->type = LT_HYPER; } else if (line->type == LT_MBMODULE) { /* go back and fix the LT_KERNEL line to indicate LT_HYPER @@ -1540,20 +1553,7 @@ struct singleLine * addLineTmpl(struct s struct singleLine * tmplLine, struct singleLine * prevLine, const char * val) { - int i; - struct singleLine * newLine = malloc(sizeof(*newLine)); - - newLine->indent = strdup(tmplLine->indent); - newLine->next = NULL; - newLine->type = tmplLine->type; - newLine->numElements = tmplLine->numElements; - newLine->elements = malloc(sizeof(*newLine->elements) * - newLine->numElements); - - for (i = 0; i < newLine->numElements; i++) { - newLine->elements[i].indent = strdup(tmplLine->elements[i].indent); - newLine->elements[i].item = strdup(tmplLine->elements[i].item); - } + struct singleLine * newLine = lineDup(tmplLine); if (val) { /* override the inherited value with our own. @@ -1599,8 +1599,8 @@ struct singleLine * addLine(struct sing struct keywordTypes * kw; struct singleLine tmpl; - /* NB: This function shouldn''t allocation items on the heap, but rather on - * the stack since it calls addLineTmpl which will make copies. + /* NB: This function shouldn''t allocate items on the heap, rather on the + * stack since it calls addLineTmpl which will make copies. */ if (type == LT_TITLE && cfi->titleBracketed) { @@ -1611,6 +1611,7 @@ struct singleLine * addLine(struct sing tmpl.elements[0].item = alloca(strlen(val)+3); sprintf(tmpl.elements[0].item, "[%s]", val); tmpl.elements[0].indent = ""; + val = NULL; } else { kw = getKeywordByType(type, cfi); if (!kw) abort(); @@ -2199,7 +2200,7 @@ int addNewKernel(struct grubConfig * con char * newKernelArgs, char * newKernelInitrd, char * newMBKernel, char * newMBKernelArgs) { struct singleEntry * new; - struct singleLine * newLine = NULL, * tmplLine = NULL; + struct singleLine * newLine = NULL, * tmplLine = NULL, * masterLine = NULL; int needs; char * chptr; @@ -2240,7 +2241,10 @@ int addNewKernel(struct grubConfig * con } if (template) { - for (tmplLine = template->lines; tmplLine; tmplLine = tmplLine->next) { + for (masterLine = template->lines; + masterLine && (tmplLine = lineDup(masterLine)); + lineFree(tmplLine), masterLine = masterLine->next) + { /* skip comments */ chptr = tmplLine->indent; while (*chptr && isspace(*chptr)) chptr++; @@ -2295,23 +2299,10 @@ int addNewKernel(struct grubConfig * con } else if (tmplLine->type == LT_HYPER && tmplLine->numElements >= 2) { - if (new->multiboot) { - if (needs & NEED_MB) { - newLine = addLineTmpl(new, tmplLine, newLine, - newMBKernel + strlen(prefix)); - needs &= ~NEED_MB; - } - } else if (needs & NEED_KERNEL) { - /* template is multi but new is not, - * insert the kernel where the hypervisor was before - */ - tmplLine->type = LT_KERNEL; - free(tmplLine->elements[0].item); - tmplLine->elements[0].item = - strdup(getKeywordByType(LT_KERNEL, config->cfi)->key); + if (needs & NEED_MB) { newLine = addLineTmpl(new, tmplLine, newLine, - newKernelPath + strlen(prefix)); - needs &= ~NEED_KERNEL; + newMBKernel + strlen(prefix)); + needs &= ~NEED_MB; } } else if (tmplLine->type == LT_MBMODULE && @@ -2325,23 +2316,38 @@ int addNewKernel(struct grubConfig * con } else if (config->cfi->mbInitRdIsModule && (needs & NEED_INITRD)) { newLine = addLineTmpl(new, tmplLine, newLine, - newKernelInitrd); + newKernelInitrd + + strlen(prefix)); needs &= ~NEED_INITRD; } + } else if (needs & NEED_KERNEL) { + /* template is multi but new is not, + * insert the kernel in the first module slot + */ + tmplLine->type = LT_KERNEL; + free(tmplLine->elements[0].item); + tmplLine->elements[0].item = + strdup(getKeywordByType(LT_KERNEL, config->cfi)->key); + newLine = addLineTmpl(new, tmplLine, newLine, + newKernelPath + strlen(prefix)); + needs &= ~NEED_KERNEL; } else if (needs & NEED_INITRD) { /* template is multi but new is not, - * insert the initrd where the module was before + * insert the initrd in the second module slot */ - newLine = addLine(new, config->cfi, LT_INITRD, - config->secondaryIndent, - newKernelInitrd + strlen(prefix)); + tmplLine->type = LT_INITRD; + free(tmplLine->elements[0].item); + tmplLine->elements[0].item = + strdup(getKeywordByType(LT_INITRD, config->cfi)->key); + newLine = addLineTmpl(new, tmplLine, newLine, + newKernelInitrd + strlen(prefix)); needs &= ~NEED_INITRD; } } else if (tmplLine->type == LT_INITRD && - tmplLine->numElements >= 2 && - (needs & NEED_INITRD)) { - if (new->multiboot && !template->multiboot && + tmplLine->numElements >= 2) { + if (needs & NEED_INITRD && + new->multiboot && !template->multiboot && config->cfi->mbInitRdIsModule) { /* make sure we don''t insert the module initrd * before the module kernel... if we don''t do it here, @@ -2353,7 +2359,7 @@ int addNewKernel(struct grubConfig * con newKernelInitrd + strlen(prefix)); needs &= ~NEED_INITRD; } - } else { + } else if (needs & NEED_INITRD) { newLine = addLineTmpl(new, tmplLine, newLine, newKernelInitrd + strlen(prefix)); needs &= ~NEED_INITRD; diff -r 57dd3b104446 -r 6964eee46f24 grubby/test.sh --- a/grubby/test.sh Tue Jun 27 19:09:26 2006 -0400 +++ b/grubby/test.sh Wed Jun 28 07:44:20 2006 -0400 @@ -1,32 +1,4 @@ #!/bin/bash - -ARCH=$(uname -m) - -elilotest="" -lilotest="" -grubtest="" -zipltest="" -yaboottest="" - -case "$ARCH" in - i?86) - lilotest="yes" - grubtest="yes" - ;; - x86_64) - lilotest="yes" - grubtest="yes" - ;; - ppc*) - yaboottest="yes" - ;; - s390*) - zipltest="yes" - ;; - *) - echo "Not running any tests for $ARCH" - exit 0 -esac export MALLOC_CHECK_=2 @@ -47,35 +19,16 @@ oneTest () { echo -n " \"$arg\"" done echo "" - ./grubby $mode --bad-image-okay -c $cfg -o - "$@" | diff -u $correct -; + ./grubby $mode --bad-image-okay -c $cfg -o - "$@" | diff -u $correct - RESULT=1 fi } -liloTest() { - if [ -z "$lilotest" ]; then echo "skipping LILO test" ; return; fi - oneTest --lilo "$@" -} - -eliloTest() { - if [ -z "$elilotest" ]; then echo "skipping ELILO test" ; return; fi - oneTest --elilo "$@" -} - -grubTest() { - if [ -z "$grubtest" ]; then echo "skipping GRUB test" ; return; fi - oneTest --grub "$@" -} - -yabootTest() { - if [ -z "$yaboottest" ]; then echo "skipping YABOOT test" ; return; fi - oneTest --yaboot "$@" -} - -ziplTest() { - if [ -z "$zipltest" ]; then echo "skipping Z/IPL test" ; return; fi - oneTest --zipl "$@" -} +liloTest() { oneTest --${FUNCNAME%Test} "$@"; } +eliloTest() { oneTest --${FUNCNAME%Test} "$@"; } +grubTest() { oneTest --${FUNCNAME%Test} "$@"; } +yabootTest() { oneTest --${FUNCNAME%Test} "$@"; } +ziplTest() { oneTest --${FUNCNAME%Test} "$@"; } echo "Parse/write comparison..." for n in $(cd test; echo grub.[0-9]*); do @@ -120,7 +73,7 @@ if [ ! -L mytest ]; then if [ ! -L mytest ]; then echo " failed (not a symlink)" fi -target=$(ls -l mytest | awk ''{ print $11 }'') +target=$(readlink mytest) if [ "$target" != grub-test ]; then echo " failed (wrong target)" fi diff -r 57dd3b104446 -r 6964eee46f24 grubby/test/results/updargs/g3.7 --- a/grubby/test/results/updargs/g3.7 Tue Jun 27 19:09:26 2006 -0400 +++ b/grubby/test/results/updargs/g3.7 Wed Jun 28 07:44:20 2006 -0400 @@ -3,11 +3,11 @@ splashimage=(hd0,1)/grub/splash.xpm.gz splashimage=(hd0,1)/grub/splash.xpm.gz title Red Hat Linux (2.4.7-2smp) root (hd0,1) - kernel /vmlinuz-2.4.7-2smp + kernel /vmlinuz-2.4.7-2smp initrd /initrd-2.4.7-2smp.img title Red Hat Linux-up (2.4.7-2) root (hd0,1) - kernel /vmlinuz-2.4.7-2 + kernel /vmlinuz-2.4.7-2 initrd /initrd-2.4.7-2.img title DOS rootnoverify (hd0,0)