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