[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] [Xend] Fix appending policy module to end of grub's config file
Stefan Berger writes ("[Xen-devel] [PATCH] [Xend] Fix appending policy module to end of grub's config file"): > Index: root/xen-unstable.hg/tools/python/xen/util/bootloader.py > =================================================================== > --- root.orig/xen-unstable.hg/tools/python/xen/util/bootloader.py > +++ root/xen-unstable.hg/tools/python/xen/util/bootloader.py > @@ -64,6 +64,8 @@ def get_kernel_val(index, att): > > def set_boot_policy(title_idx, filename): > boottitles = get_boot_policies() > + for key in boottitles.iterkeys(): > + boottitles[key] += ".bin" > if boottitles.has_key(title_idx): > rm_policy_from_boottitle(title_idx, [ boottitles[title_idx] ]) > rc = add_boot_policy(title_idx, filename) Having investigated this situation I'm not really convinced that this patch is correct. One problem that the various Bootloader classes in bootloader.py seem to disagree about what `title_idx' is in this context ? The Grub class uses line numbers but LatePolicyLoader seems always to use DEFAULT_TITLE (ie, `any') which seems odd. Perhaps I have misread the code. What your change seems to be addressing is a confusion about whether it's a filename (with `.bin') or a stripped version. The relevant stripped version seems to be the one generated in LatePolicyLoader.add_boot_policy, which just strips `.bin', but compare this with Grub.get_boot_policies which strips `.bin' but also various stuff from the front of what I think is the same kind of path. I'm afraid I wasn't able to conclude which of the disagreeing parts the code were right or wrong, or I would have suggested an alternative way to fix the problem. Even if the right answer is just to deal with the discrepancy over `.bin' in this way in set_boot_policy, editing the whole array seems a strange way to go about it. Why not just amend the result of the actual lookup ? eg rm_policy_from_boottitle(title_idx, [ boottitles[title_idx] + '.bin' ]) > @@ -335,6 +337,8 @@ class Grub(Bootloader): > os.write(tmp_fd, line) > > if module_line != "" and not found: > + if ord(line[-1]) not in [ 10 ]: > + os.write(tmp_fd, '\n') > os.write(tmp_fd, module_line) > found = True Why the circumlocutions with `ord' and `in [ ... ]' ? As an aside, bootloader.py currently contains multiple bootfile parsers and editors, containing a lot of very similar code. I make it 4 parsers and 3 parser-mutators. Surely these should be combined as far as possible ? Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |