Thanks for the precious comments from both David & Chris. I will revise and send out a third version soon.
BTW, can anyone tell me where to find the current compose package list for F16? I need to make sure tboot package is included.
Jimmy
David Lehman wrote on 2011-08-12:
> I have some inline comments.
>
> On Mon, 2011-08-01 at 17:54 +0800, Wei, Gang wrote:
>> diff --git a/pyanaconda/bootloader.py b/pyanaconda/bootloader.py
>> index 6df0107..dc11f90 100644
>> --- a/pyanaconda/bootloader.py
>> +++ b/pyanaconda/bootloader.py
>> @@ -114,6 +114,25 @@ class LinuxBootLoaderImage(BootLoaderImage):
>> filename = "initramfs-%s.img" % self.version
>> return filename
>> +class MultibootLinuxBootLoaderImage(LinuxBootLoaderImage): + def
>> __init__(self, device=None, label=None, short=None, version=None, +
>> multiboot=None, mbargs=None, args=None): +
>> super(MultibootLinuxBootLoaderImage, self).__init__( + device=device,
>> label=label, + short=short, version=version) + self._multiboot =
>> multiboot # filename string + self._mbargs = mbargs
>
> It seems like these two attributes above could just be hardcoded in
> here if they cannot be changed in any case.
>
>> + self._args = args
>> +
>> + @property
>> + def multiboot(self):
>> + return self._multiboot
>> +
>> + def mbargs(self):
>> + return self._mbargs
>> +
>> + def args(self):
>> + return self._args
>>
>> class BootLoader(object):
>> """TODO:
>> @@ -1123,15 +1142,30 @@ class GRUB(BootLoader):
>> grub_root = self.grub_device_name(self.stage2_device)
>> args.update(["ro", "root=%s" %
>> image.device.fstabSpec]) args.update(self.boot_args)
>> - stanza = ("title %(label)s (%(version)s)\n"
>> - "\troot %(grub_root)s\n"
>
>
>> - "\tkernel %(prefix)s/%(kernel)s %(args)s\n"
>> - "\tinitrd %(prefix)s/%(initrd)s\n"
>
> Just by abstracting the above section you could prevent duplication of
> the entire stanza template, eg:
>
> if isinstance(image, MultibootLinuxBootLoaderImage):
> snippet = ("\tkernel %(prefix)s/%(multiboot)s %(mbargs)s\n"
> "\tmodule %(prefix)s/%(kernel)s %(args)s\n"
> "\tmodule %(prefix)s/%(initrd)s\n"
> % {"prefix": self.boot_prefix,
> "multiboot": image.multiboot,
> "mbargs": image.mbargs,
> "kernel": image.kernel, "initrd": image.initrd,
> "args": args})
> else:
> snippet = ("\tkernel %(prefix)s/%(kernel)s %(args)s\n"
> "\tinitrd %(prefix)s/%(initrd)s\n"
> % {"prefix": self.boot_prefix,
> "kernel": image.kernel, "initrd": image.initrd,
> "args": args})
> and then
>
> stanza = ("title %(label)s (%(version)s)\n"
> "\troot %(grub_root)s\n"
> "%(snippet)s"
> % {"label": image.label, "version": image.version,
> "snippet": snippet
> "grub_root": grub_root})
>
>
>> - % {"label": image.label, "version":
>> image.version,
>> - "grub_root": grub_root,
>> - "kernel": image.kernel, "initrd":
>> image.initrd,
>> - "args": args,
>> - "prefix": self.boot_prefix})
>> + if isinstance(image, MultibootLinuxBootLoaderImage):
>> + args.update(image.args)
>> + stanza = ("title %(label)s (%(version)s)\n"
>> + "\troot %(grub_root)s\n"
>> + "\tkernel %(prefix)s/%(multiboot)s
>> %(mbargs)s\n"
>> + "\tmodule %(prefix)s/%(kernel)s
>> %(args)s\n"
>> + "\tmodule %(prefix)s/%(initrd)s\n"
>> + % {"label": image.label, "version":
>> image.version,
>> + "grub_root": grub_root,
>> + "multiboot": image.multiboot,
>> + "mbargs": image.mbargs,
>> + "kernel": image.kernel, "initrd":
>> image.initrd,
>> + "args": args,
>> + "prefix": self.boot_prefix})
>> + else:
>> + stanza = ("title %(label)s (%(version)s)\n"
>> + "\troot %(grub_root)s\n"
>> + "\tkernel %(prefix)s/%(kernel)s
>> %(args)s\n"
>> + "\tinitrd %(prefix)s/%(initrd)s\n"
>> + % {"label": image.label, "version":
>> image.version,
>> + "grub_root": grub_root,
>> + "kernel": image.kernel, "initrd":
>> image.initrd,
>> + "args": args,
>> + "prefix": self.boot_prefix})
>> else:
>> stanza = ("title %(label)s\n"
>> "\trootnoverify %(grub_root)s\n"
>> @@ -1905,6 +1939,8 @@ def writeSysconfigKernel(anaconda,
>> default_kernel):
>> f.write("DEFAULTKERNEL=%s\n" % default_kernel)
>> f.close()
>> +def is_trusted_boot(anaconda):
>> + return anaconda.backend.ayum.isPackageInstalled(name="tboot")
>
> Perhaps this could be written to an attribute of the BootLoader
> instance from inside YumBackend immediately after package installation
> so we don't have yum references in the bootloader module. Then you can
> just check anaconda.bootloader.trusted_boot in writeBootloader.
>
>>
>> def writeBootloader(anaconda):
>> """ Write bootloader configuration to disk.
>> @@ -1971,9 +2007,18 @@ def writeBootloader(anaconda):
>> used.append(nick)
>> label = "%s-%s" % (base_label, nick)
>> short = "%s-%s" % (base_short, nick)
>> - image =
>> LinuxBootLoaderImage(device=anaconda.storage.rootDevice,
>> - version=version,
>> - label=label, short=short)
>> + if is_trusted_boot(anaconda):
>> + image = MultibootLinuxBootLoaderImage(
>> +
>> device=anaconda.storage.rootDevice,
>> + version=version,
>> + label=label, short=short,
>> + multiboot="tboot.gz",
>> +
>> mbargs=["logging=vga,serial,memory"],
>> +
> args=["intel_iommu=on"])
>
> If you're going to hardcode the multiboot and mbargs strings, why not
> do it in the MultibootLinuxBootLoaderImage class instead of here?
>
>> + else:
>> + image =
>> LinuxBootLoaderImage(device=anaconda.storage.rootDevice,
>> + version=version,
>> + label=label, short=short)
>> anaconda.bootloader.add_image(image)
>> # write out /etc/sysconfig/kernel
>
> Dave
>
>> --
>> 1.7.5.1
>>
>>
>>
>
> _______________________________________________
> Anaconda-devel-list mailing list
> Anaconda-devel-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/anaconda-devel-list
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
